Re: [robust-futex-3] futex: robust futex support

[Ulrich Drepper <drepper@gmail.com>]

And another thing: semaphores are on their way out.  So, in
futex_deregister and in futex_head, shouldn't you use mutexes?  I
don't see that you realy need semaphores.

In futex_register, you define mm and initialize it with current->mm. 
That's OK.  But why then are you using

+       down_read(&current->mm->mmap_sem);

just a few lines below?

And finally (for now): in get_futex_key the VMA containing the futex
is determined.  And yet, in futex_register you have an identical
find_extend_vma call.  I don't know how expensive this function is. 
But I would assume that at least the error handling in futex_register
can go away since the VMA cannot be torn down while mmap_sem is taken,
right?  But perhaps this just points to more inconsistencies.  Why is
the list/sem lookup in get_futex_key?  Only to share the code with
futex_deregister.  But is that really worth it?  The majority of calls
to get_futex_key come from all the other call sites so the code you
added is only a cost without any gain.  Especially since you could in
futex_register do the whole thing without any additional cost and
because most of the new tests in get_futex_key are again tested in
futex_register (to determined shared vs non-shared) and do not have to
be tested in futex_deregister (we know the futex is in shared memory).

I suggest that if find_extend_vma is sufficiently expensive, pass a
pointer to a variable of that type to get_futex_key.  If it is cheap,
don't do anything.  Pull the new code in get_futex_key into
futex_register and futex_deregister, optimize out unnecessary code,
and merge with the rest of the functions.  It'll be much less
invasive.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/