Re: [patch 1/6] fs: icache RCU free inodes

[Nick Piggin <npiggin@kernel.dk>]

On Tue, Nov 09, 2010 at 11:21:31AM -0500, Christoph Hellwig wrote:
> On Tue, Nov 09, 2010 at 08:02:38AM -0800, Linus Torvalds wrote:
> > Remind me why it wasn't sufficient to just use SLAB_DESTROY_BY_RCU?
> 
> Dave sent a patch for it, which looks much better to me.

It's horribly complex and why even do any of that for icache by
itself? What is going on there really? Do you think RCU inodes
are really important for inode hash lookup?

>  Nick thinks
> it doesn't work for his store free path walk, but I haven't seen an
> explanation why exactly.

I posted explanations several times, and the code is there to
look at. I haven't seen an explanation from you about how it should
be used with rcu-walk, and why we shouldn't go with the simpler
RCU approach first and then evaulate an incremental patch.

> 
> > The only thing we care about is the pathname walk - there are no other
> > inode operations that are common enough to worry about. And the only
> > thing _that_ needs is the ability to look at the inode under RCU, and
> > SLAB_DESTROY_BY_RCU should be entirely sufficient for that.
> 
> It might be worth it for inode lookup.  While it's shadowed by the
> dcache hash we still hit it a lot, especially for NFS serving.  

No, you would never make inode RCU for that case alone. That's
crazy. _If_ NFS serving really hits that bottleneck, then fine
grained hash locking makes it go away. So it's not a justification
for anything.