Re: FAILED: patch "[PATCH] autofs: use dentry flags to block walks during expire" failed to apply to 4.4-stable tree

[Ian Kent <raven@themaw.net>]

On Mon, 2016-09-26 at 09:56 +0200, gregkh@linuxfoundation.org wrote:

Hi Greg,

> The patch below does not apply to the 4.4-stable tree.
> If someone wants it applied there, or to any other stable or longterm
> tree, then please email the backport, including the original git commit
> id to <stable@vgegr.kernel.org>.

There is another bug fix from Al Viro upon which this path depends which has not
been included in the stable tree.

I recommend that I backport both patches.
Is that acceptable?

Should I include a cover description for the two patches explaining the need for
the additional patch or just post the two patches to <stable@vgegr.kernel.org>?

Ian

> 
> thanks,
> 
> greg k-h
> 
> ------------------ original commit in Linus's tree ------------------
> 
> From 7cbdb4a286a60c5d519cb9223fe2134d26870d39 Mon Sep 17 00:00:00 2001
> From: Ian Kent <raven@themaw.net>
> Date: Mon, 19 Sep 2016 14:44:12 -0700
> Subject: [PATCH] autofs: use dentry flags to block walks during expire
> 
> Somewhere along the way the autofs expire operation has changed to hold
> a spin lock over expired dentry selection.  The autofs indirect mount
> expired dentry selection is complicated and quite lengthy so it isn't
> appropriate to hold a spin lock over the operation.
> 
> Commit 47be61845c77 ("fs/dcache.c: avoid soft-lockup in dput()") added a
> might_sleep() to dput() causing a WARN_ONCE() about this usage to be
> issued.
> 
> But the spin lock doesn't need to be held over this check, the autofs
> dentry info.  flags are enough to block walks into dentrys during the
> expire.
> 
> I've left the direct mount expire as it is (for now) because it is much
> simpler and quicker than the indirect mount expire and adding spin lock
> release and re-aquires would do nothing more than add overhead.
> 
> Fixes: 47be61845c77 ("fs/dcache.c: avoid soft-lockup in dput()")
> Link: 
> http://lkml.kernel.org/r/20160912014017.1773.73060.stgit@pluto.themaw.net
> Signed-off-by: Ian Kent <raven@themaw.net>
> Reported-by: Takashi Iwai <tiwai@suse.de>
> Tested-by: Takashi Iwai <tiwai@suse.de>
> Cc: Takashi Iwai <tiwai@suse.de>
> Cc: NeilBrown <neilb@suse.com>
> Cc: Al Viro <viro@zeniv.linux.org.uk>
> Cc: <stable@vger.kernel.org>
> Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
> Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
> 
> diff --git a/fs/autofs4/expire.c b/fs/autofs4/expire.c
> index b493909e7492..d8e6d421c27f 100644
> --- a/fs/autofs4/expire.c
> +++ b/fs/autofs4/expire.c
> @@ -417,6 +417,7 @@ static struct dentry *should_expire(struct dentry *dentry,
>  	}
>  	return NULL;
>  }
> +
>  /*
>   * Find an eligible tree to time-out
>   * A tree is eligible if :-
> @@ -432,6 +433,7 @@ struct dentry *autofs4_expire_indirect(struct super_block
> *sb,
>  	struct dentry *root = sb->s_root;
>  	struct dentry *dentry;
>  	struct dentry *expired;
> +	struct dentry *found;
>  	struct autofs_info *ino;
>  
>  	if (!root)
> @@ -442,31 +444,46 @@ struct dentry *autofs4_expire_indirect(struct
> super_block *sb,
>  
>  	dentry = NULL;
>  	while ((dentry = get_next_positive_subdir(dentry, root))) {
> +		int flags = how;
> +
>  		spin_lock(&sbi->fs_lock);
>  		ino = autofs4_dentry_ino(dentry);
> -		if (ino->flags & AUTOFS_INF_WANT_EXPIRE)
> -			expired = NULL;
> -		else
> -			expired = should_expire(dentry, mnt, timeout, how);
> -		if (!expired) {
> +		if (ino->flags & AUTOFS_INF_WANT_EXPIRE) {
>  			spin_unlock(&sbi->fs_lock);
>  			continue;
>  		}
> +		spin_unlock(&sbi->fs_lock);
> +
> +		expired = should_expire(dentry, mnt, timeout, flags);
> +		if (!expired)
> +			continue;
> +
> +		spin_lock(&sbi->fs_lock);
>  		ino = autofs4_dentry_ino(expired);
>  		ino->flags |= AUTOFS_INF_WANT_EXPIRE;
>  		spin_unlock(&sbi->fs_lock);
>  		synchronize_rcu();
> -		spin_lock(&sbi->fs_lock);
> -		if (should_expire(expired, mnt, timeout, how)) {
> -			if (expired != dentry)
> -				dput(dentry);
> -			goto found;
> -		}
>  
> +		/* Make sure a reference is not taken on found if
> +		 * things have changed.
> +		 */
> +		flags &= ~AUTOFS_EXP_LEAVES;
> +		found = should_expire(expired, mnt, timeout, how);
> +		if (!found || found != expired)
> +			/* Something has changed, continue */
> +			goto next;
> +
> +		if (expired != dentry)
> +			dput(dentry);
> +
> +		spin_lock(&sbi->fs_lock);
> +		goto found;
> +next:
> +		spin_lock(&sbi->fs_lock);
>  		ino->flags &= ~AUTOFS_INF_WANT_EXPIRE;
> +		spin_unlock(&sbi->fs_lock);
>  		if (expired != dentry)
>  			dput(expired);
> -		spin_unlock(&sbi->fs_lock);
>  	}
>  	return NULL;
>  
> @@ -483,6 +500,7 @@ int autofs4_expire_wait(struct dentry *dentry, int
> rcu_walk)
>  	struct autofs_sb_info *sbi = autofs4_sbi(dentry->d_sb);
>  	struct autofs_info *ino = autofs4_dentry_ino(dentry);
>  	int status;
> +	int state;
>  
>  	/* Block on any pending expire */
>  	if (!(ino->flags & AUTOFS_INF_WANT_EXPIRE))
> @@ -490,8 +508,19 @@ int autofs4_expire_wait(struct dentry *dentry, int
> rcu_walk)
>  	if (rcu_walk)
>  		return -ECHILD;
>  
> +retry:
>  	spin_lock(&sbi->fs_lock);
> -	if (ino->flags & AUTOFS_INF_EXPIRING) {
> +	state = ino->flags & (AUTOFS_INF_WANT_EXPIRE | AUTOFS_INF_EXPIRING);
> +	if (state == AUTOFS_INF_WANT_EXPIRE) {
> +		spin_unlock(&sbi->fs_lock);
> +		/*
> +		 * Possibly being selected for expire, wait until
> +		 * it's selected or not.
> +		 */
> +		schedule_timeout_uninterruptible(HZ/10);
> +		goto retry;
> +	}
> +	if (state & AUTOFS_INF_EXPIRING) {
>  		spin_unlock(&sbi->fs_lock);
>  
>  		pr_debug("waiting for expire %p name=%pd\n", dentry, dentry);
>