lkml.org 
[lkml]   [2003]   [Sep]   [8]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH] Re: today's futex changes
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
-
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/

\
 
 \ /
  Last update: 2005-03-22 13:48    [W:0.144 / U:0.088 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site