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 <ra...@themaw.net>
> Cc: Takashi Iwai <ti...@suse.de>
Reported-and-tested-by: Takashi Iwai <ti...@suse.de>
thanks,
Takashi
> Cc: Andrew Morton <a...@linux-foundation.org>
> Cc: NeilBrown <ne...@suse.com>
> ---
> 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);
>
