On Wed, Oct 20, 2010 at 01:46:31PM +0100, Al Viro wrote: > On Tue, Oct 19, 2010 at 02:42:35PM +1100, npiggin@xxxxxxxxx wrote: > > + /* > > + * We can walk the per-sb list of inodes here without worrying about > > + * its consistency, because the list must not change during umount > > + * anymore, and because iprune_sem keeps shrink_icache_memory() away. > > + */ > > fsnotify_unmount_inodes(&sb->s_inodes); > > OK, explain to me why is that safe. Note that fsnotify_destroy_mark() > _can_ race with umount, dropping the last reference to inode before > fsnotify_unmount_inodes() would get to it and kill it (along with the mark). > With the current code it's just fine - we walk the list under lock and > iput() won't mess with that list until it acquires the damn lock. And > no matter who gets there first, the mark will be destroyed and reference > to inode will be dropped. > > With your change, AFAICS, removal from the list can happen while we walk > it. With obvious results. Ah, tricky. So after fsnotify_unmount_inodes runs, the invalidate_list is now safe because no more marks can put the inode at that point? OK I can concede that point and drop the patch, thanks. Where can fsnotify_destroy_mark run concurrently at umount time, can you explain? I haven't spotted it yet. -- 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
