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

From: Ulrich Drepper
Date: Tue Jan 17 2006 - 12:49:14 EST


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@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/