| Date | Thu, 21 Oct 2010 00:03:45 +1100 | From | Nick Piggin <> | Subject | Re: [patch 19/35] fs: icache remove redundant i_sb_list umount locking |
| |
On Wed, Oct 20, 2010 at 01:46:31PM +0100, Al Viro wrote: > On Tue, Oct 19, 2010 at 02:42:35PM +1100, npiggin@kernel.dk 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.
|