Re: [PATCH 11/18] fs: Introduce per-bucket inode hash locks

[Christoph Hellwig]

On Sat, Oct 16, 2010 at 06:57:03PM +1100, Nick Piggin wrote:
> > duplicating these helpers in the dcache code aswell.  IMHO they
> > should simple operate directly on the hlist_bl_head, as that's
> > what it was designed for.  I also don't really see any point in
> > wrapping the hlist_bl_head as inode_hash_bucket.  If the bucket naming
> > is important we could rename the hlist_bl stuff to bl_hash, and the
> > hlist_bl_head could become bl_hash_bucket.
> 
> It was done because someone, like -rt, might want more than one bit of
> memory to implement a lock. They would have to make a few other
> changes, granted, but this helps reduce a lot of churn.
> 
> I didn't see the point of a layer of dumb wrappers for hlist_bl_head
> locking. Just reproducing bit spin and wait locks in wrappers when we
> already have good functions for them.

With the changes Dave implemented based on my suggestions we now have
an abstract locked hash list data type.  It has the normal hash list
operations plus lock/unlock operations.  So if e.g. the -rt folks need
real locks in there there is one single place they need to touch
instead of every user.  Similarly if we want to add lockdep support
there is just one place to touch.

Nevermind the cast of the list pointer to unsigned long is ugly enough
that it really needs to be hidden in a helper belonging to the list
implementation that is properly commented.

--
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/