> Date: Sat, 2 Jun 2018 12:42:14 +0200 (CEST)
> From: Mark Kettenis <mark.kette...@xs4all.nl>
> 
> > Date: Sat, 2 Jun 2018 12:04:16 +0200
> > From: Martin Pieuchot <m...@openbsd.org>
> > 
> > On 01/06/18(Fri) 19:13, Mark Kettenis wrote:
> > > The diff below changes our futex implementation to allow futexes that
> > > are shared between processes.  Such futexes are a little bit tricky
> > > since there is no guarantee that a futex is mapped at the same address
> > > in each process.  To solve tis issue I adopted the strategy chosen by
> > > FreeBSD.  Shared memory objects have a uvm object associated with
> > > them.  So instead of using the virtual address I can use the offset
> > > into that object.  Together with the a pointer to the object itself
> > > that should form a unique ID for the futex.
> > 
> > Nice.
> > 
> > > In order to establish that we're dealing with shared memory we need to
> > > lookup the mapping corresponding to the userspace address of the
> > > futex.  That involves locking the map.  If we are worried about the
> > > additional overhead we could introduce shared versions of the futex
> > > operations and skip the lookup for non-shared (process-private) futexes.
> > > 
> > > These futexes could be used to implement support for the POSIX Thread
> > > Process-Shared Synchronization option, i.e. the PTHREAD_PROCESS_SHARED
> > > attribute for mutexes.  But I need these to make libxshmfence work on
> > > OpenBSD, which is in turn needed to get DRI3 support working.
> > > 
> > > Making DRI3 support work is somewhat important.  We're currently using
> > > DRI2 but Linux has moved to DRI3 and some of the DRI2-specific
> > > codepaths in X are starting to rot.  DRI3 is also supposed to be more
> > > secure than DRI2.  With DRI2, applications that can guess a 32-bit
> > > handle can in principle access graphiucs buffers from other
> > > applications.  With DRI3 graphics buffers are shared using file
> > > descriptor passing which means that only processes that were passed
> > > the file descriptor can access the graphics buffer.
> > > 
> > > Another feature of DRI3 is that graphics buffers can be shared between
> > > graphics devices.  The idea there is that a GPU can render into a
> > > buffer which is then shared with the Intel CPU graphics which can then
> > > display it on the screen.  But my initial implementation won't support
> > > that.
> > > 
> > > Thoughts?  Especially on whether we want to distinguish between
> > > process-shared and process-private futexes!
> > 
> > I'd prefer if we could do the distinction.  Would it be enough to pass
> > a flag to futex_get()?
> 
> More or less.  The question is what what we should do if someone asks
> for a process-shared futex in memory that isn't shared.  Should we
> automatically create a process-private futex?  Or should this result
> in an error?
> 
> I suspect that people expect PTHREAD_PROCESS_SHARED mutexes to work
> even if they're not created in shared memory.  So a failure to create
> a process-shared futex should probably fall through into the
> process-private case.
> 
> Turns out Linux already has a FUTEX_PRIVATE_FLAG and
> FUTEX_WAIT_PRIVATE, FUTEX_WAKE_PRIVATE and FUTEX_REQUEUE_PRIVATE
> operations.  So I'll just stick to that API.

So here is an implementation of that APU.  After this gets committed
we can change libc to use the _PRIVATE versions of the mutex
operations.  I already verified that that works.

ok?


Index: kern/sys_futex.c
===================================================================
RCS file: /cvs/src/sys/kern/sys_futex.c,v
retrieving revision 1.7
diff -u -p -r1.7 sys_futex.c
--- kern/sys_futex.c    24 Apr 2018 17:19:35 -0000      1.7
+++ kern/sys_futex.c    2 Jun 2018 16:36:31 -0000
@@ -31,6 +31,8 @@
 #include <sys/ktrace.h>
 #endif
 
+#include <uvm/uvm.h>
+
 /*
  * Atomicity is only needed on MULTIPROCESSOR kernels.  Fall back on
  * copyin(9) until non-MULTIPROCESSOR architectures have a copyin32(9)
@@ -46,21 +48,23 @@
 struct futex {
        LIST_ENTRY(futex)        ft_list;       /* list of all futexes */
        TAILQ_HEAD(, proc)       ft_threads;    /* sleeping queue */
-       uint32_t                *ft_uaddr;      /* userspace address */
+       struct uvm_object       *ft_obj;        /* UVM object */
+       voff_t                   ft_off;        /* UVM offset */
        pid_t                    ft_pid;        /* process identifier */
        unsigned int             ft_refcnt;     /* # of references */
 };
 
 /* Syscall helpers. */
-int             futex_wait(uint32_t *, uint32_t, const struct timespec *);
-int             futex_wake(uint32_t *, uint32_t);
-int             futex_requeue(uint32_t *, uint32_t, uint32_t *, uint32_t);
+int     futex_wait(uint32_t *, uint32_t, const struct timespec *, int);
+int     futex_wake(uint32_t *, uint32_t, int);
+int     futex_requeue(uint32_t *, uint32_t, uint32_t *, uint32_t, int);
 
 /* Flags for futex_get(). */
 #define FT_CREATE      0x1     /* Create a futex if it doesn't exist. */
+#define FT_PRIVATE     0x2     /* Futex is process-private. */
 
-struct futex   *futex_get(uint32_t *, int);
-void            futex_put(struct futex *);
+struct futex *futex_get(uint32_t *, int);
+void    futex_put(struct futex *);
 
 /*
  * The global futex lock serialize futex(2) calls such that no wakeup
@@ -94,23 +98,30 @@ sys_futex(struct proc *p, void *v, regis
        uint32_t val = SCARG(uap, val);
        const struct timespec *timeout = SCARG(uap, timeout);
        void *g = SCARG(uap, g);
+       int flags = 0;
+
+       if (op & FUTEX_PRIVATE_FLAG)
+               flags |= FT_PRIVATE;
 
        switch (op) {
        case FUTEX_WAIT:
+       case FUTEX_WAIT_PRIVATE:
                KERNEL_LOCK();
                rw_enter_write(&ftlock);
-               *retval = futex_wait(uaddr, val, timeout);
+               *retval = futex_wait(uaddr, val, timeout, flags);
                rw_exit_write(&ftlock);
                KERNEL_UNLOCK();
                break;
        case FUTEX_WAKE:
+       case FUTEX_WAKE_PRIVATE:
                rw_enter_write(&ftlock);
-               *retval = futex_wake(uaddr, val);
+               *retval = futex_wake(uaddr, val, flags);
                rw_exit_write(&ftlock);
                break;
        case FUTEX_REQUEUE:
+       case FUTEX_REQUEUE_PRIVATE:
                rw_enter_write(&ftlock);
-               *retval = futex_requeue(uaddr, val, g, (unsigned long)timeout);
+               *retval = futex_requeue(uaddr, val, g, (u_long)timeout, flags);
                rw_exit_write(&ftlock);
                break;
        default:
@@ -127,29 +138,47 @@ sys_futex(struct proc *p, void *v, regis
  * If such futex does not exist and FT_CREATE is given, create it.
  */
 struct futex *
-futex_get(uint32_t *uaddr, int flag)
+futex_get(uint32_t *uaddr, int flags)
 {
        struct proc *p = curproc;
+       vm_map_t map = &p->p_vmspace->vm_map;
+       vm_map_entry_t entry;
+       struct uvm_object *obj = NULL;
+       voff_t off = (vaddr_t)uaddr;
+       pid_t pid = p->p_p->ps_pid;
        struct futex *f;
 
        rw_assert_wrlock(&ftlock);
 
+       if (!(flags & FT_PRIVATE)) {
+               vm_map_lock_read(map);
+               if (uvm_map_lookup_entry(map, (vaddr_t)uaddr, &entry) &&
+                   UVM_ET_ISOBJ(entry) && entry->object.uvm_obj &&
+                   entry->inheritance == MAP_INHERIT_SHARE) {
+                       obj = entry->object.uvm_obj;
+                       off = entry->offset + ((vaddr_t)uaddr - entry->start);
+                       pid = 0;
+               }
+               vm_map_unlock_read(map);
+       }
+
        LIST_FOREACH(f, &ftlist, ft_list) {
-               if (f->ft_uaddr == uaddr && f->ft_pid == p->p_p->ps_pid) {
+               if (f->ft_obj == obj && f->ft_off == off && f->ft_pid == pid) {
                        f->ft_refcnt++;
                        break;
                }
        }
 
-       if ((f == NULL) && (flag & FT_CREATE)) {
+       if ((f == NULL) && (flags & FT_CREATE)) {
                /*
                 * We rely on the rwlock to ensure that no other thread
                 * create the same futex.
                 */
                f = pool_get(&ftpool, PR_WAITOK);
                TAILQ_INIT(&f->ft_threads);
-               f->ft_uaddr = uaddr;
-               f->ft_pid = p->p_p->ps_pid;
+               f->ft_obj = obj;
+               f->ft_off = off;
+               f->ft_pid = pid;
                f->ft_refcnt = 1;
                LIST_INSERT_HEAD(&ftlist, f, ft_list);
        }
@@ -181,7 +210,8 @@ futex_put(struct futex *f)
  * indefinitly if the argument is NULL.
  */
 int
-futex_wait(uint32_t *uaddr, uint32_t val, const struct timespec *timeout)
+futex_wait(uint32_t *uaddr, uint32_t val, const struct timespec *timeout,
+    int flags)
 {
        struct proc *p = curproc;
        struct futex *f;
@@ -220,7 +250,7 @@ futex_wait(uint32_t *uaddr, uint32_t val
                        to_ticks = INT_MAX;
        }
 
-       f = futex_get(uaddr, FT_CREATE);
+       f = futex_get(uaddr, flags | FT_CREATE);
        TAILQ_INSERT_TAIL(&f->ft_threads, p, p_fut_link);
        p->p_futex = f;
 
@@ -251,7 +281,8 @@ futex_wait(uint32_t *uaddr, uint32_t val
  * address ``uaddr2''.
  */
 int
-futex_requeue(uint32_t *uaddr, uint32_t n, uint32_t *uaddr2, uint32_t m)
+futex_requeue(uint32_t *uaddr, uint32_t n, uint32_t *uaddr2, uint32_t m,
+    int flags)
 {
        struct futex *f, *g;
        struct proc *p;
@@ -259,7 +290,7 @@ futex_requeue(uint32_t *uaddr, uint32_t 
 
        rw_assert_wrlock(&ftlock);
 
-       f = futex_get(uaddr, 0);
+       f = futex_get(uaddr, flags);
        if (f == NULL)
                return 0;
 
@@ -288,7 +319,7 @@ futex_requeue(uint32_t *uaddr, uint32_t 
  * ``uaddr''.
  */
 int
-futex_wake(uint32_t *uaddr, uint32_t n)
+futex_wake(uint32_t *uaddr, uint32_t n, int flags)
 {
-       return futex_requeue(uaddr, n, NULL, 0);
+       return futex_requeue(uaddr, n, NULL, 0, flags);
 }
Index: sys/futex.h
===================================================================
RCS file: /cvs/src/sys/sys/futex.h,v
retrieving revision 1.1
diff -u -p -r1.1 futex.h
--- sys/futex.h 28 Apr 2017 13:50:55 -0000      1.1
+++ sys/futex.h 2 Jun 2018 16:36:32 -0000
@@ -32,4 +32,10 @@ __END_DECLS
 #define        FUTEX_WAKE              2
 #define        FUTEX_REQUEUE           3
 
+#define FUTEX_PRIVATE_FLAG     128
+
+#define FUTEX_WAIT_PRIVATE     (FUTEX_WAIT | FUTEX_PRIVATE_FLAG)
+#define FUTEX_WAKE_PRIVATE     (FUTEX_WAKE | FUTEX_PRIVATE_FLAG)
+#define FUTEX_REQUEUE_PRIVATE  (FUTEX_REQUEUE | FUTEX_PRIVATE_FLAG)
+
 #endif /* _SYS_FUTEX_H_ */

Reply via email to