Re: [PATCH v6] hashtable: introduce a small and naive hashtable

[Mathieu Desnoyers]

* Sasha Levin (levinsasha928@xxxxxxxxx) wrote:
> On 09/27/2012 10:25 AM, David Laight wrote:
> >>>> And even then, if we would do:
> >>>>
> >>>>  	for (i = 0; i < HASH_SIZE(hashtable); i++)
> >>>>  		if (!hlist_empty(&hashtable[i]))
> >>>>  			break;
> >>>>
> >>>>  	return i >= HASH_SIZE(hashtable);
> >>>>
> >>>> What happens if the last entry of the table is non-empty ?
> >>>
> >>> It still works, as 'i' is not incremented due to the break. And i will
> >>> still be less than HASH_SIZE(hashtable). Did you have *your* cup of
> >>> coffee today? ;-)
> >>
> >> Ahh, right! Actually I had it already ;-)
> > 
> > I tend to dislike the repeated test, gcc might be able to optimise
> > it away, but the code is cleaner written as:
> > 
> > 	for (i = 0; i < HASH_SIZE(hashtable); i++)
> > 		if (!hlist_empty(&hashtable[i]))
> > 			return false;
> > 	return true;
> 
> Right, the flag thing in the macro was there just to make it work
> properly as a macro.
> 
> >> Agreed that the flags should be removed. Moving to define + static
> >> inline is still important though.
> > 
> > Not sure I'd bother making the function inline.
> 
> I usually never make anything 'inline', I just let gcc do it's own
> thing when it compiles the code. If there are any objections
> please let me know before I send the new version.

AFAIK, gcc nowadays use "inline" only as a hint, because programmers
were using it everywhere, even where it should not have been used. This
is where the attribute always_inline becomes useful, if you really,
really want to inline. However, for kernel coding style consistency, it
might be better to use "static inline", because it is used everywhere
else in kernel headers. Or maybe are there some headers that do not use
"inline" I am not aware of ?

Moreover, if your thinking is that we do not need a static inline
function replicated at every caller, maybe we should introduce a
lib/hashtable.c that implements those 2 functions.

Thanks,

Mathieu

> 
> 
> Thanks,
> Sasha
> 

-- 
Mathieu Desnoyers
Operating System Efficiency R&D Consultant
EfficiOS Inc.
http://www.efficios.com
--
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/