From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932730AbcILKGc (ORCPT ); Mon, 12 Sep 2016 06:06:32 -0400 Received: from mx2.suse.de ([195.135.220.15]:49858 "EHLO mx2.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932199AbcILKGT (ORCPT ); Mon, 12 Sep 2016 06:06:19 -0400 Date: Mon, 12 Sep 2016 12:06:14 +0200 Message-ID: From: Takashi Iwai To: Ian Kent Cc: Al Viro , autofs mailing list , NeilBrown , Kernel Mailing List , linux-fsdevel , Andrew Morton Subject: Re: [PATCH] autofs - use dentry flags to block walks during expire In-Reply-To: <20160912014017.1773.73060.stgit@pluto.themaw.net> References: <20160912014017.1773.73060.stgit@pluto.themaw.net> User-Agent: Wanderlust/2.15.9 (Almost Unreal) SEMI/1.14.6 (Maruoka) FLIM/1.14.9 (=?UTF-8?B?R29qxY0=?=) APEL/10.8 Emacs/24.5 (x86_64-suse-linux-gnu) MULE/6.0 (HANACHIRUSATO) MIME-Version: 1.0 (generated by SEMI 1.14.6 - "Maruoka") Content-Type: text/plain; charset=US-ASCII Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Mon, 12 Sep 2016 03:40:17 +0200, Ian Kent wrote: > > 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 47be6184 added a might_sleep() to dput() causing a BUG() > 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) becuase 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()") > Signed-off-by: Ian Kent > Cc: Takashi Iwai Reported-and-tested-by: Takashi Iwai thanks, Takashi > Cc: Andrew Morton > Cc: NeilBrown > --- > fs/autofs4/expire.c | 55 +++++++++++++++++++++++++++++++++++++++------------ > 1 file changed, 42 insertions(+), 13 deletions(-) > > diff --git a/fs/autofs4/expire.c b/fs/autofs4/expire.c > index b493909..d8e6d42 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); >