stable.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* FAILED: patch "[PATCH] autofs: use dentry flags to block walks during expire" failed to apply to 4.4-stable tree
@ 2016-09-26  7:56 gregkh
  2016-09-27  6:54 ` Ian Kent
  0 siblings, 1 reply; 4+ messages in thread
From: gregkh @ 2016-09-26  7:56 UTC (permalink / raw)
  To: raven, akpm, neilb, stable, tiwai, torvalds, viro; +Cc: stable


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@vger.kernel.org>.

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);


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: FAILED: patch "[PATCH] autofs: use dentry flags to block walks during expire" failed to apply to 4.4-stable tree
  2016-09-26  7:56 FAILED: patch "[PATCH] autofs: use dentry flags to block walks during expire" failed to apply to 4.4-stable tree gregkh
@ 2016-09-27  6:54 ` Ian Kent
  2016-09-27  7:05   ` Greg KH
  0 siblings, 1 reply; 4+ messages in thread
From: Ian Kent @ 2016-09-27  6:54 UTC (permalink / raw)
  To: gregkh; +Cc: akpm, neilb, stable, tiwai, torvalds, viro, raven

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);
> 

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: FAILED: patch "[PATCH] autofs: use dentry flags to block walks during expire" failed to apply to 4.4-stable tree
  2016-09-27  6:54 ` Ian Kent
@ 2016-09-27  7:05   ` Greg KH
  2016-09-27  7:32     ` Ian Kent
  0 siblings, 1 reply; 4+ messages in thread
From: Greg KH @ 2016-09-27  7:05 UTC (permalink / raw)
  To: Ian Kent; +Cc: akpm, neilb, stable, tiwai, torvalds, viro

On Tue, Sep 27, 2016 at 02:54:30PM +0800, Ian Kent wrote:
> 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?

Sure, or just give me the git commit id of the patch and I will queue it
up as well before I apply this one.

> 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>?

Well, the correct address would be the good thing to use :)

Other than that, either way is fine.

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: FAILED: patch "[PATCH] autofs: use dentry flags to block walks during expire" failed to apply to 4.4-stable tree
  2016-09-27  7:05   ` Greg KH
@ 2016-09-27  7:32     ` Ian Kent
  0 siblings, 0 replies; 4+ messages in thread
From: Ian Kent @ 2016-09-27  7:32 UTC (permalink / raw)
  To: Greg KH; +Cc: akpm, neilb, stable, tiwai, torvalds, viro

On Tue, 2016-09-27 at 09:05 +0200, Greg KH wrote:
> On Tue, Sep 27, 2016 at 02:54:30PM +0800, Ian Kent wrote:
> > 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?
> 
> Sure, or just give me the git commit id of the patch and I will queue it
> up as well before I apply this one.

There were some other changes to log prints as well, so they both need to be
back ported.

> 
> > 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>?
> 
> Well, the correct address would be the good thing to use :)

LOL, oh, didn't notice that ...

> 
> Other than that, either way is fine.

OK, I presume including the Linux tree commit id in each patch description is
sufficient, unless there's some specific format you need?

> 
> thanks,
> 
> greg k-h

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2016-09-27  7:32 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-09-26  7:56 FAILED: patch "[PATCH] autofs: use dentry flags to block walks during expire" failed to apply to 4.4-stable tree gregkh
2016-09-27  6:54 ` Ian Kent
2016-09-27  7:05   ` Greg KH
2016-09-27  7:32     ` Ian Kent

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).