Re: RFC: Lightweight synchronization mechanism for gnumach v2.0

[Agustina Arzille]

Hello, Samuel.

On 03/30/2016 08:07 PM, Samuel Thibault wrote:
> > For shared gsync objects, I'm using vm_map_lookup to find out the
> > vm object and offset. The problem is that said function expects the map
> > to be unlocked
> Really?
>
> It starts with locking it, and finishes with unlocking it, and the lock
> is recursive, so it's fine to take it several times.

Yes, but there are parts where it tries to upgrade from read to write,
and viceversa as well, and from what I understand, that requires the
lock recursion count to reach zero, which cannot happen if we locked
the VM map before calling the function.

The comments preceding the implementation say that "the map should
not be locked", so I don't know. Perhaps it's only under a specific
circumstance that it needs to upgrade/downgrade the lock. Like I said,
I don't know much about the VM system, but I'd rather err on the side of
cautiousness.

> > A simple workaround is
> > to do valid_access_p *again* after vm_map_lookup returns (It should be
> > faster since the vm hint should be set by then).
> But there's still room between that call and the dereference. We really
> need to have the map locked between the access check and the actual
> dereference.

This I don't see. What I propose to do is basically:

unlock_read (task->map)
vm_map_lookup (...)

lock_read (task->map)
if (!valid_access_p (...))
  error

That is, we release the lock before calling vm_map_lookup, only to
reacquire it after it returns, and checking once again if the address
is still valid to access for the calling task.

Actually, now that I think about it, perhaps we could delegate the
locking to gsync_fill_waiter. For shared objects, we could perform the
vm_map_lookup first, *then* grabbing the read lock and checking if
the address is accessible, instead of doing the check for both private
and shared objects on entry.

What do you think?

> > Slow. I sent the letter about 20 days ago, still no answer.
> Don't hesitate to ping the people at FSF from times to times, to make
> sure it's not stuck somewhere.
>
> Samuel

Will do. Thanks for the tip.