Re: [PATCH] Re: today's futex changes

Jamie Lokier
2003-09-08T17:52:25Z

Rusty Russell wrote:
> + u32 hash = jhash2((u32*)&key->both.word,

Have you checked the code size?

That does more work and has more code than is needed, especially on
32-bit archs. On 32-bit, jhash_3words() is much better because it
reduces to a single call to __jhash_mix(), instead of the two done by
jhash2 (only one is required for good hashing afaict).

It is probably worth adding a jhash_3longs() to jhash.h, which does
one call to __hash_mix() on 32-bit, two calls on 64-bit, and avoids
the loop in both cases.

[ Aside: For hashing individual integers, I prefer to use Thomas Wang's:

http://www.concentric.net/~Ttwang/tech/inthash.htm

He mentions Jenkin's function, and derived an integer mixing function
from correspondence with Jenkins.

For hashing 3 words together, Jenkins' hash does seem a bit more
compact - if you don't call __jhash_mix() multiple times that is! ]

> - if (unlikely((vma->vm_flags & (VM_IO|VM_READ)) != VM_READ))
> - return (vma->vm_flags & VM_IO) ? -EPERM : -EACCES;
> + if (unlikely(vma->vm_flags & VM_IO))
> + return -EPERM;
> + if (unlikely(vma->vm_flags & (VM_READ|VM_WRITE)) != (VM_READ|VM_WRITE))
> + return -EACCES;

Is there a good reason to disallow read-only waiters?
I agree with Hugh that it seems like a regression.

> + /* A spurious wakeup. Should never happen. */
> + BUG();

:)

The rest of your changes seem fine. I particularly appreciate your
grammatical improvements to my comment :)

-- Jamie