On Fri, May 02, 2014 at 11:08:13PM +0200, Miklos Szeredi wrote: > There's more of the "delete from shrink list not owned by us" in select parent. > Proposed patch appended. While it certainly looks like dentry_lru_del() should die, I really wonder if "let's pretend that dentry isn't there if it's on some other shrink list" is the right approach. You've already noticed one problem (check-and-drop giving false busy indications), but shrink_dcache_parent() has similar issues. How about incrementing data->found instead? That'll end up rescanning the tree in case if it's not ours; so what? If it's another process doing the same shrinking in a subtree, we want to let it do its job anyway. Sure, somebody doing mount -o remount in a loop might be able to stall the living hell out of us, for as long as new non-busy dentries are being added in our subtree, but the second part in itself is sufficient; we will keep picking those new non-busy dentries as long as they keep coming. And if they do not, eventually they will be all taken out by us or by those other shrinkers and we'll be done. > And I'm not sure what umount_collect() is supposed to do. Can other shrinkers > still be active at that point? That would present other problems, no? No other shrinkers - prune_dcache_sb() and shrink_dcache_sb() are also serialized by ->s_umount, shrink_dcache_parent() and check_submounts_and_drop() are called only when an active reference is held, which obviously prevents fs shutdown. > Also somewhat related is the question: how check_submounts_and_drop() could be > guaranteed correctness (timely removal of all unsed dentries) in the presence of > other shrinkers? Another interesting question is what the hell are shrink_dcache_parent() callers expecting. E.g. what's going on in fuse_reverse_inval_entry()? And what is nilfs_tree_is_busy() about? FWIW, I'm not at all sure that vfs_rmdir() and vfs_rename() have any reason to call it these days, and dentry_unhash() one simply ought to die - it's used only by hpfs unlink() in case it wants to truncate in order to work around -ENOSPC. And _that_ won't have any subdirectories to deal with anyway, so shrink_dcache_parent() there is a no-op, even if we keep the damn helper alive. The rest of callers also look dubious... -- 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
