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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <50D4DBC8.2020008@oracle.com>
Date:	Fri, 21 Dec 2012 16:59:36 -0500
From:	Sasha Levin <sasha.levin@...cle.com>
To:	Andrew Morton <akpm@...ux-foundation.org>
CC:	Hugh Dickins <hughd@...gle.com>, Michal Hocko <mhocko@...e.cz>,
	Konstantin Khlebnikov <khlebnikov@...nvz.org>,
	Mel Gorman <mgorman@...e.de>, linux-mm@...ck.org,
	linux-kernel@...r.kernel.org, Sasha Levin <levinsasha928@...il.com>
Subject: Re: [PATCH v2] mm,ksm: use new hashtable implementation

On 12/21/2012 04:36 PM, Andrew Morton wrote:
> On Fri, 21 Dec 2012 12:46:50 -0500
> Sasha Levin <sasha.levin@...cle.com> 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@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/