Re: [dm-devel] [PATCH v7 01/16] hashtable: introduce a small and naive hashtable

[Sasha Levin <levinsasha928 gmail com>]

On Mon, Oct 29, 2012 at 12:14 PM, Mathieu Desnoyers
<mathieu desnoyers efficios com> wrote:
> * Sasha Levin (levinsasha928 gmail com) wrote:
>> On Mon, Oct 29, 2012 at 7:29 AM, Mathieu Desnoyers
>> <mathieu desnoyers efficios com> wrote:
>> > * Sasha Levin (levinsasha928 gmail com) wrote:
>> >> +
>> >> +     for (i = 0; i < sz; i++)
>> >> +             INIT_HLIST_HEAD(&ht[sz]);
>> >
>> > ouch. How did this work ? Has it been tested at all ?
>> >
>> > sz -> i
>>
>> Funny enough, it works perfectly. Generally as a test I boot the
>> kernel in a VM and let it fuzz with trinity for a bit, doing that with
>> the code above worked flawlessly.
>>
>> While it works, it's obviously wrong. Why does it work though? Usually
>> there's a list op happening pretty soon after that which brings the
>> list into proper state.
>>
>> I've been playing with a patch that adds a magic value into list_head
>> if CONFIG_DEBUG_LIST is set, and checks that magic in the list debug
>> code in lib/list_debug.c.
>>
>> Does it sound like something useful? If so I'll send that patch out.
>
> Most of the calls to this initialization function apply it on zeroed
> memory (static/kzalloc'd...), which makes it useless. I'd actually be in
> favor of removing those redundant calls (as I pointed out in another
> email), and document that zeroed memory don't need to be explicitly
> initialized.

Why would that make it useless? The idea is that the init functions
will set the magic field to something random, like:

.magic = 0xBADBEEF0;

And have list_add() and friends WARN(.magic != 0xBADBEEF0, "Using an
uninitialized list\n");

This way we'll catch all places that don't go through list initialization code.


Thanks,
Sasha