lkml.org 
[lkml]   [2010]   [Oct]   [13]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH 16/18] fs: Reduce inode I_FREEING and factor inode disposal
    >  /*
    > * Locking rules.
    > *
    > + * inode->i_lock is *always* the innermost lock.
    > + *

    shouldn't this be added in an earlier patch?

    > @@ -48,8 +50,15 @@
    > *
    > * sb inode lock
    > * inode_lru_lock
    > - * wb->b_lock
    > - * inode->i_lock
    > + * wb->b_lock
    > + * inode->i_lock
    > + *
    > + * wb->b_lock
    > + * sb_lock (pin sb for writeback)
    > + * inode->i_lock
    > + *
    > + * inode_lru
    > + * inode->i_lock

    This doesn't seem to be new in this patch either. Maybe just have
    a separate patch to introduce the lock order protection comment in
    it's final form instead of the various updates?

    > - int busy;
    > LIST_HEAD(throw_away);
    > + int busy;
    >
    > down_write(&iprune_sem);
    > spin_lock(&sb->s_inodes_lock);
    > fsnotify_unmount_inodes(&sb->s_inodes);
    > busy = invalidate_list(sb, &sb->s_inodes, &throw_away);
    > spin_unlock(&sb->s_inodes_lock);
    > + up_write(&iprune_sem);
    >
    > dispose_list(&throw_away);
    > - up_write(&iprune_sem);

    I first though this was unsafe. But in the end the lock doesn't
    actually need to protect anything here. If we're getting here
    from generic_shutdown_super the filesystem is dead already and
    thus other calls to invalidate_inodes which need a reference to
    the superblock won't arrive here. prune_icache could arrive
    here, but I_FREEING will make it skip the inode. So it looks
    like the shorter hold time is fine. In fact just cycling through
    iprune_sem here would probably be enough.

    Even better would be getting rid of the gem by simply doing
    per-superblock inode LRUs which require to have a reference on
    the superblock and thus avoid reclaim reacing with unmount.
    Time to ressurect your patch for it once the lock split up is done.

    Otherwise looks good to me.


    \
     
     \ /
      Last update: 2010-10-13 15:53    [W:4.085 / U:0.156 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site