Re: [PATCH] Decrease hash table memory overhead

From: Linus Torvalds (torvalds@transmeta.com)
Date: Mon Jul 31 2000 - 16:40:41 EST


On Mon, 31 Jul 2000, Andi Kleen wrote:
>
> Or it might. It is just too much work for me, because it is takes too long
> to do right.

Fair enough.

> Anyways, hlists are already used all over the kernel (e.g. try grep pprev
> net/ipv4/*), just everybody is reinventing the wheel on them all the
> time. I did that myself several times. It would be nicer to use list_*()
> macros the time, just without the bloat of the list_* list heads.

Noe that THIS is a valid argument that I can find no holes in.

The argument of "inode.c could be speeded up/shrunk/xxxx" doesn't strike
me as being a very good argument especially just before 2.4.x.

The argument that "lots of code already does this, except they aren't very
clean about it and do it by hand", is an argument I can buy into.

You might consider just going about it a different way: pick the places
that _already_ use this kind of list, and clean them up using a generic
list package. I still don't like "hlists" as a name, because I still don't
see the "hash" in them conceptually, but I would certainyl consider any
cleanup a good thing.

And once you come from that direction, it's going to be a lot easier
convincing me to eventually potentially switch over some of the current
lists.h users to a new implementation.

> I suspect that would either end up with lots of pseudo functional function
> pointer (do_foo(hash_list, void (*foo_functor)(void *, void *)) and other slow
> horrors) or a disgusting macro mess like the older lists.h that was
> recently removed.

I'm not convinced. The wnew list.h in my opinion does really well, and
_without_ having a lot of ugly macros. It's strange to people using the
BSD ones, but it has, in my opinion, a much cleaner interface. I think the
same approach could be extended to cover the needs of a nice hash table.

For example, the current lists have the list_for_each() thing that walks
the list. The "hash_find(list)" thing wouldn't need to be all that
different from that one - let people supply their own hash comparison
functions etc not by giving them as arguments, but by creating nice
constructs together with the helper functions.

Something as simple as "list_for_each()" can be a HUGE simplifier, and
make code a lot more readable. My favourite example is the PCI suspend
code, which is just a lot of grotty code walking the trees this way and
that, but that with some good organization resulted in functions that look
really simple and obvious. The "list_for_each()" macro is part of that.

I bet that all the existing hlist-like code is really for walking
hash-chains, no? I really think there is opportunity for creating more of
a _nice_ infrastructure for doing these hash-chains, rather than just
doing the list handling.

Basically, even if it only ends up being a simple list implementation, I'd
prefer to call it "hash_xxxx()" just to avoid confusion with the "normal"
lists. And I think it could be more.

And I think we'd be better off converting existing users than try to
convert code that already looks pretty ;)

                Linus

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.rutgers.edu
Please read the FAQ at http://www.tux.org/lkml/



This archive was generated by hypermail 2b29 : Mon Jul 31 2000 - 21:00:34 EST