Re: [PATCH v2] mm,ksm: use new hashtable implementation

[Sasha Levin]

On 12/21/2012 04:36 PM, Andrew Morton wrote:
> On Fri, 21 Dec 2012 12:46:50 -0500
> Sasha Levin <sasha.levin@xxxxxxxxxx> wrote:
> 
>> Switch ksm to use the new hashtable implementation. This reduces the amount of
>> generic unrelated code in the ksm module.
> 
> hm, include/linux/hashtable.h:hash_min() is rather dangerous - it
> returns different values depending on the size of the first argument. 
> So if the calling code mixes up its ints and longs (and boy we do that
> a lot), the result will work on 32-bit and fail on 64-bit.

The reason for doing so is because hashing 32 bits is much faster than
hashing 64 bits.

I'd really prefer to fix the code the mixes up ints and longs instead
of removing optimizations. Not only because of the optimizations themselves
but because these mixups will be rather obvious with the hashtable as
opposed to all the other places that just misbehave silently.

> Also, is there ever likely to be a situation where the first arg to
> hash_min() is *not* a pointer?  Perhaps it would be better to concede
> to reality: rename `key' to `ptr' and remove all those typcasts you
> just added.

There actually are several. This is the reason for hash_min really - several
places that used 32bit keys would have been slowed down by switch to
hash_long(), which is why hash_min() was introduced.

The first places that come to mind are userns, 9p and tracepoints, I guess
there are a few more which I don't remember.


Thanks,
Sasha
--
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/