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()?
> 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 1 Jun 2018 15:50:17 -0000
> @@ -31,6 +31,8 @@
> #include <sys/ktrace.h>
> #endif
>
> +#include <uvm/uvm_extern.h>
> +
> /*
> * Atomicity is only needed on MULTIPROCESSOR kernels. Fall back on
> * copyin(9) until non-MULTIPROCESSOR architectures have a copyin32(9)
> @@ -46,6 +48,8 @@
> struct futex {
> LIST_ENTRY(futex) ft_list; /* list of all futexes */
> TAILQ_HEAD(, proc) ft_threads; /* sleeping queue */
> + struct uvm_object *ft_obj;
> + voff_t ft_offset;
> uint32_t *ft_uaddr; /* userspace address */
> pid_t ft_pid; /* process identifier */
> unsigned int ft_refcnt; /* # of references */
> @@ -130,8 +134,22 @@ struct futex *
> futex_get(uint32_t *uaddr, int flag)
> {
> 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 offset = 0;
> struct futex *f;
>
> + vm_map_lock_read(map);
> + if (uvm_map_lookup_entry(map, (vaddr_t)uaddr, &entry)) {
> + if (entry->object.uvm_obj &&
> + entry->inheritance == MAP_INHERIT_SHARE) {
> + obj = entry->object.uvm_obj;
> + offset = entry->offset + ((vaddr_t)uaddr -
> entry->start);
> + }
> + }
> + vm_map_unlock_read(map);
> +
> rw_assert_wrlock(&ftlock);
>
> LIST_FOREACH(f, &ftlist, ft_list) {
> @@ -139,6 +157,10 @@ futex_get(uint32_t *uaddr, int flag)
> f->ft_refcnt++;
> break;
> }
> + if (f->ft_obj == obj && f->ft_offset == offset) {
> + f->ft_refcnt++;
> + break;
> + }
> }
>
> if ((f == NULL) && (flag & FT_CREATE)) {
> @@ -146,10 +168,15 @@ futex_get(uint32_t *uaddr, int flag)
> * We rely on the rwlock to ensure that no other thread
> * create the same futex.
> */
> - f = pool_get(&ftpool, PR_WAITOK);
> + f = pool_get(&ftpool, PR_WAITOK | PR_ZERO);
> TAILQ_INIT(&f->ft_threads);
> - f->ft_uaddr = uaddr;
> - f->ft_pid = p->p_p->ps_pid;
> + if (obj) {
> + f->ft_obj = obj;
> + f->ft_offset = offset;
> + } else {
> + f->ft_uaddr = uaddr;
> + f->ft_pid = p->p_p->ps_pid;
> + }
> f->ft_refcnt = 1;
> LIST_INSERT_HEAD(&ftlist, f, ft_list);
> }
>
