On Tue, Apr 29, 2014 at 08:10:15PM +0100, Al Viro wrote: > On Tue, Apr 29, 2014 at 07:16:10PM +0100, Al Viro wrote: > > On Tue, Apr 29, 2014 at 08:03:24PM +0200, Miklos Szeredi wrote: > > > > > Introducing a new per-sb lock should be OK. > > > > > > Another idea, which could have subtler effects, is simply not to kill > > > a dentry that is on the shrink list (indicated by > > > DCACHE_SHRINK_LIST), since it's bound to get killed anyway. But > > > that's a change in behaviour... > > > > Umm... You mean, if final dput() finds dentry already on shrink list, > > just leave it there and return? Might get really painful - the code > > that knows it's holding the last reference to already unhashed dentry > > might get a nasty surprise when dput() returns before it's killed off. > > I wonder if we could have dput() side of thinks check if we are on the > shrink list, mark it "I'll be killing it anyway" and go ahead without > removal from the shrink list and instead of freeing mark it "I'm done > with it". With shrink_dentry_list(), on the other hand, checking for those > marks, treating the former as "just move it to private list and keep > going". After the list of victims is dealt with, keep picking dentries > from the second list, wait for them to get the second mark and do actual > freeing. That ought to avoid any extra locks and preserve all ordering, > except for that of kmem_cache_free(), AFAICS... > > Comments? Seems like it would work, but it seems fragile to me - I'm wondering how we can ensure that the private shrink list manipulations can be kept private. We have a similar situation with the inode cache (private shrink list) but the I_FREEING flag is set the entire time the inode is on the shrink list. Any new hash lookup or attempt to grab the inode that occurs while I_FREEING is set fails, so perhaps dentries also need a well defined "being torn down and freed" state where new references cannot be taken even though the dentry can still be found... Cheers, Dave. -- Dave Chinner david@xxxxxxxxxxxxx -- 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
