> /* > * 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. -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
