On 08/23/2012 10:04 PM, Tejun Heo wrote:

Hello, Sasha.

On Thu, Aug 23, 2012 at 02:24:32AM +0200, Sasha Levin wrote:

quoted

quoted

I think the almost trivial nature of hlist hashtables makes this a bit
tricky and I'm not very sure but having this combinatory explosion is
a bit dazzling when the same functionality can be achieved by simply
combining operations which are already defined and named considering
hashtable.  I'm not feeling too strong about this tho.  What do others
think?

I'm thinking that this hashtable API will have 2 purposes: First, it would
prevent the excessive duplication of hashtable implementations all around the code.

Second, it will allow more easily interchangeable hashtable implementations to
find their way into the kernel. There are several maintainers who would be happy
to see dynamically sized RCU hashtable, and I'm guessing that several more
variants could be added based on needs in specific modules.

The second reason is why several things you've mentioned look the way they are:

 - No DEFINE_HASHTABLE(): I wanted to force the use of hash_init() since
initialization for other hashtables may be more complicated than the static
initialization for this implementation, which means that any place that used
DEFINE_HASHTABLE() and didn't do hash_init() will be buggy.

I think this is problematic.  It looks exactly like other existing
DEFINE macros yet what its semantics is different.  I don't think
that's a good idea.

I can switch that to be DECLARE_HASHTABLE() if the issue is semantics.

quoted

I'm actually tempted in hiding hlist completely from hashtable users, probably
by simply defining a hash_head/hash_node on top of the hlist_ counterparts.

I think that it would be best to keep this one simple & obvious, which
already has enough in-kernel users to justify its existence.  There
are significant benefits in being trivially understandable and
expectable.  If we want more advanced ones - say resizing, hybrid or
what not, let's make that a separate one.  No need to complicate the
common straight-forward case for that.

So, I think it would be best to keep this one as straight-forward and
trivial as possible.  Helper macros to help its users are fine but
let's please not go for full encapsulation.

What if we cut off the dynamic allocated (but not resizable) hashtable out for
the moment, and focus on the most common statically allocated hashtable case?

The benefits would be:

 - Getting rid of all the _size() macros, which will make the amount of helpers
here reasonable.
 - Dynamically allocated hashtable can be easily added as a separate
implementation using the same API. We already have some of those in the kernel...
 - When that's ready, I feel it's a shame to lose full encapsulation just due to
hash_hashed().


Thanks,
Sasha

Thanks.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>