Re: [PATCH] Destroy the dentries contributed by a superblock on unmounting

[David Howells <dhowells@redhat.com>]

Andrew Morton <akpm@osdl.org> wrote:

> Is the cond_resched_lock() here safe?  Once we've dropped that lock, the
> list cursor `loop' is invalidated?

Yes.  Nothing else is permitted to modify the d_subdirs list.  Not the VFS,
not the filesystem, not the memory pressure shrinker.

> If all lookup paths to all entries on this list are removed at this time
> then OK - but these dentries are still on the LRU.

Ah... but the LRU shrinker may not touch dentries belonging to a filesystem
that's got s_umount writelocked (eg: one that's in the process of being
unmounted).  See prune_dcache():

		/*
		 * ...otherwise we need to be sure this filesystem isn't being
		 * unmounted, otherwise we could race with
		 * generic_shutdown_super(), and end up holding a reference to
		 * an inode while the filesystem is unmounted.
		 * So we try to get s_umount, and make sure s_root isn't NULL.
		 * (Take a local copy of s_umount to avoid a use-after-free of
		 * `dentry').
		 */
		s_umount = &dentry->d_sb->s_umount;
		if (down_read_trylock(s_umount)) {
			if (dentry->d_sb->s_root != NULL) {
				prune_one_dentry(dentry);
				up_read(s_umount);
				continue;
			}
			up_read(s_umount);
		}

		spin_unlock(&dentry->d_lock);

> (An answer-via-comment-patch would suit ;))

It is commented already:

/*
 * destroy the dentries attached to a superblock on unmounting
 * - we don't need to use dentry->d_lock, and only need dcache_lock when
 *   removing the dentry from the system lists and hashes because:
 *   - the superblock is detached from all mountings and open files, so the
 *     dentry trees will not be rearranged by the VFS
 *   - s_umount is write-locked, so the memory pressure shrinker will ignore
 *     any dentries belonging to this superblock that it comes across
 *   - the filesystem itself is no longer permitted to rearrange the dentries
 *     in this superblock
 */
void shrink_dcache_for_umount(struct super_block *sb)
{


Note the point about the memory pressure shrinker.

David