> On Oct 28, 2022, at 8:13 AM, Jeff Layton <jlayton@xxxxxxxxxx> wrote:
>
> We had a report of this:
>
> BUG: sleeping function called from invalid context at fs/nfsd/filecache.c:440
>
> ...with a stack trace showing nfsd_file_put being called from
> nfs4_show_open. This code has always tried to call fput while holding a
> spinlock, but we recently changed this to use the filecache, and that
> started triggering the might_sleep() in nfsd_file_put.
>
> states_start takes and holds the cl_lock while iterating over the
> client's states, and we can't sleep with that held.
>
> Have the various nfs4_show_* functions instead hold the fi_lock instead
> of taking a nfsd_file reference.
>
> Fixes: 78599c42ae3c ("nfsd4: add file to display list of client's opens")
> Link: https://bugzilla.redhat.com/show_bug.cgi?id=2138357
> Reported-by: Zhi Li <yieli@xxxxxxxxxx>
> Signed-off-by: Jeff Layton <jlayton@xxxxxxxxxx>
> ---
> fs/nfsd/nfs4state.c | 51 +++++++++++++++++++++++++++++----------------
> 1 file changed, 33 insertions(+), 18 deletions(-)
Applied to nfsd's for-next (internally for now; will push later).
> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
> index 1ded89235111..dde621debeb2 100644
> --- a/fs/nfsd/nfs4state.c
> +++ b/fs/nfsd/nfs4state.c
> @@ -675,15 +675,26 @@ find_any_file(struct nfs4_file *f)
> return ret;
> }
>
> -static struct nfsd_file *find_deleg_file(struct nfs4_file *f)
> +static struct nfsd_file *find_any_file_locked(struct nfs4_file *f)
> {
> - struct nfsd_file *ret = NULL;
> + lockdep_assert_held(&f->fi_lock);
> +
> + if (f->fi_fds[O_RDWR])
> + return f->fi_fds[O_RDWR];
> + if (f->fi_fds[O_WRONLY])
> + return f->fi_fds[O_WRONLY];
> + if (f->fi_fds[O_RDONLY])
> + return f->fi_fds[O_RDONLY];
> + return NULL;
> +}
> +
> +static struct nfsd_file *find_deleg_file_locked(struct nfs4_file *f)
> +{
> + lockdep_assert_held(&f->fi_lock);
>
> - spin_lock(&f->fi_lock);
> if (f->fi_deleg_file)
> - ret = nfsd_file_get(f->fi_deleg_file);
> - spin_unlock(&f->fi_lock);
> - return ret;
> + return f->fi_deleg_file;
> + return NULL;
> }
>
> static atomic_long_t num_delegations;
> @@ -2616,9 +2627,11 @@ static int nfs4_show_open(struct seq_file *s, struct nfs4_stid *st)
> ols = openlockstateid(st);
> oo = ols->st_stateowner;
> nf = st->sc_file;
> - file = find_any_file(nf);
> +
> + spin_lock(&nf->fi_lock);
> + file = find_any_file_locked(nf);
> if (!file)
> - return 0;
> + goto out;
>
> seq_printf(s, "- ");
> nfs4_show_stateid(s, &st->sc_stateid);
> @@ -2640,8 +2653,8 @@ static int nfs4_show_open(struct seq_file *s, struct nfs4_stid *st)
> seq_printf(s, ", ");
> nfs4_show_owner(s, oo);
> seq_printf(s, " }\n");
> - nfsd_file_put(file);
> -
> +out:
> + spin_unlock(&nf->fi_lock);
> return 0;
> }
>
> @@ -2655,9 +2668,10 @@ static int nfs4_show_lock(struct seq_file *s, struct nfs4_stid *st)
> ols = openlockstateid(st);
> oo = ols->st_stateowner;
> nf = st->sc_file;
> - file = find_any_file(nf);
> + spin_lock(&nf->fi_lock);
> + file = find_any_file_locked(nf);
> if (!file)
> - return 0;
> + goto out;
>
> seq_printf(s, "- ");
> nfs4_show_stateid(s, &st->sc_stateid);
> @@ -2677,8 +2691,8 @@ static int nfs4_show_lock(struct seq_file *s, struct nfs4_stid *st)
> seq_printf(s, ", ");
> nfs4_show_owner(s, oo);
> seq_printf(s, " }\n");
> - nfsd_file_put(file);
> -
> +out:
> + spin_unlock(&nf->fi_lock);
> return 0;
> }
>
> @@ -2690,9 +2704,10 @@ static int nfs4_show_deleg(struct seq_file *s, struct nfs4_stid *st)
>
> ds = delegstateid(st);
> nf = st->sc_file;
> - file = find_deleg_file(nf);
> + spin_lock(&nf->fi_lock);
> + file = find_deleg_file_locked(nf);
> if (!file)
> - return 0;
> + goto out;
>
> seq_printf(s, "- ");
> nfs4_show_stateid(s, &st->sc_stateid);
> @@ -2708,8 +2723,8 @@ static int nfs4_show_deleg(struct seq_file *s, struct nfs4_stid *st)
> seq_printf(s, ", ");
> nfs4_show_fname(s, file);
> seq_printf(s, " }\n");
> - nfsd_file_put(file);
> -
> +out:
> + spin_unlock(&nf->fi_lock);
> return 0;
> }
>
> --
> 2.37.3
>
--
Chuck Lever
