Re: [PATCH 17/19] fs: Reduce inode I_FREEING and factor inodedisposal

[Dave Chinner]

On Sun, Oct 17, 2010 at 04:13:10PM +1100, Nick Piggin wrote:
> On Sun, Oct 17, 2010 at 03:35:14PM +1100, Nick Piggin wrote:
> > On Sun, Oct 17, 2010 at 03:13:13PM +1100, Dave Chinner wrote:
> > > On Sun, Oct 17, 2010 at 01:49:23PM +1100, Nick Piggin wrote:
> > > > On Sat, Oct 16, 2010 at 09:30:47PM -0400, Christoph Hellwig wrote:
> > > > > >   * inode->i_lock is *always* the innermost lock.
> > > > > >   *
> > > > > > + * inode->i_lock is *always* the innermost lock.
> > > > > > + *
> > > > > 
> > > > > No need to repeat, we got it..
> > > > 
> > > > Except that I didn't see where you fixed all the places where it is
> > > > *not* the innermost lock. Like for example places that take dcache_lock
> > > > inside i_lock.
> > > 
> > > I can't find any code outside of ceph where the dcache_lock is used
> > > within 200 lines of code of the inode->i_lock. The ceph code is not
> > > nesting them, though.
> > 
> > You mustn't have looked very hard? From ceph:
> > 
> >        spin_unlock(&dcache_lock);
> >        spin_unlock(&inode->i_lock);
> > 
> > (and yes, acquisition side does go in i_lock->dcache_lock order)

Sorry, easy to miss with a quick grep when the locks are taken in
different functions.

Anyway, this one looks difficult to fix without knowing something
about Ceph and wtf it is doing there. It's one to punt to the
maintainer to solve as it's not critical to this patch set.

> A really quick grep reveals cifs is using GlobalSMBSeslock inside i_lock
> too.

I'm having a grep-fail day. Where is that one?

Cheers,

Dave.
-- 
Dave Chinner
david@xxxxxxxxxxxxx
--
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/