2023-11-17 02:22:26

by NeilBrown

Subject: [PATCH 5/9] nfsd: allow admin-revoked state to appear in /proc/fs/nfsd/clients/*/states

Change the "show" functions to show some content even if a file cannot
be found.
This is primarily useful for debugging - to ensure states are being
removed eventually.

Signed-off-by: NeilBrown <[email protected]>
---
fs/nfsd/nfs4state.c | 81 +++++++++++++++++++++++----------------------
1 file changed, 41 insertions(+), 40 deletions(-)

diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index a2b3320a6ba8..8debd148840f 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -2680,17 +2680,10 @@ static int nfs4_show_open(struct seq_file *s, struct nfs4_stid *st)
struct nfs4_stateowner *oo;
unsigned int access, deny;

- if (st->sc_type != NFS4_OPEN_STID && st->sc_type != NFS4_LOCK_STID)
- return 0; /* XXX: or SEQ_SKIP? */
ols = openlockstateid(st);
oo = ols->st_stateowner;
nf = st->sc_file;

- spin_lock(&nf->fi_lock);
- file = find_any_file_locked(nf);
- if (!file)
- goto out;
-
seq_printf(s, "- ");
nfs4_show_stateid(s, &st->sc_stateid);
seq_printf(s, ": { type: open, ");
@@ -2705,14 +2698,19 @@ static int nfs4_show_open(struct seq_file *s, struct nfs4_stid *st)
deny & NFS4_SHARE_ACCESS_READ ? "r" : "-",
deny & NFS4_SHARE_ACCESS_WRITE ? "w" : "-");

- nfs4_show_superblock(s, file);
- seq_printf(s, ", ");
- nfs4_show_fname(s, file);
- seq_printf(s, ", ");
+ spin_lock(&nf->fi_lock);
+ file = find_any_file_locked(nf);
+ if (file) {
+ nfs4_show_superblock(s, file);
+ seq_puts(s, ", ");
+ nfs4_show_fname(s, file);
+ seq_puts(s, ", ");
+ }
+ spin_unlock(&nf->fi_lock);
nfs4_show_owner(s, oo);
+ if (st->sc_status & NFS4_STID_ADMIN_REVOKED)
+ seq_puts(s, ", admin-revoked");
seq_printf(s, " }\n");
-out:
- spin_unlock(&nf->fi_lock);
return 0;
}

@@ -2726,30 +2724,31 @@ static int nfs4_show_lock(struct seq_file *s, struct nfs4_stid *st)
ols = openlockstateid(st);
oo = ols->st_stateowner;
nf = st->sc_file;
- spin_lock(&nf->fi_lock);
- file = find_any_file_locked(nf);
- if (!file)
- goto out;

seq_printf(s, "- ");
nfs4_show_stateid(s, &st->sc_stateid);
seq_printf(s, ": { type: lock, ");

- /*
- * Note: a lock stateid isn't really the same thing as a lock,
- * it's the locking state held by one owner on a file, and there
- * may be multiple (or no) lock ranges associated with it.
- * (Same for the matter is true of open stateids.)
- */
+ spin_lock(&nf->fi_lock);
+ file = find_any_file_locked(nf);
+ if (file) {
+ /*
+ * Note: a lock stateid isn't really the same thing as a lock,
+ * it's the locking state held by one owner on a file, and there
+ * may be multiple (or no) lock ranges associated with it.
+ * (Same for the matter is true of open stateids.)
+ */

- nfs4_show_superblock(s, file);
- /* XXX: open stateid? */
- seq_printf(s, ", ");
- nfs4_show_fname(s, file);
- seq_printf(s, ", ");
+ nfs4_show_superblock(s, file);
+ /* XXX: open stateid? */
+ seq_puts(s, ", ");
+ nfs4_show_fname(s, file);
+ seq_puts(s, ", ");
+ }
nfs4_show_owner(s, oo);
+ if (st->sc_status & NFS4_STID_ADMIN_REVOKED)
+ seq_puts(s, ", admin-revoked");
seq_printf(s, " }\n");
-out:
spin_unlock(&nf->fi_lock);
return 0;
}
@@ -2762,27 +2761,29 @@ static int nfs4_show_deleg(struct seq_file *s, struct nfs4_stid *st)

ds = delegstateid(st);
nf = st->sc_file;
- spin_lock(&nf->fi_lock);
- file = nf->fi_deleg_file;
- if (!file)
- goto out;

seq_printf(s, "- ");
nfs4_show_stateid(s, &st->sc_stateid);
seq_printf(s, ": { type: deleg, ");

/* Kinda dead code as long as we only support read delegs: */
- seq_printf(s, "access: %s, ",
- ds->dl_type == NFS4_OPEN_DELEGATE_READ ? "r" : "w");
+ seq_printf(s, "access: %s",
+ ds->dl_type == NFS4_OPEN_DELEGATE_READ ? "r" : "w");

/* XXX: lease time, whether it's being recalled. */

- nfs4_show_superblock(s, file);
- seq_printf(s, ", ");
- nfs4_show_fname(s, file);
- seq_printf(s, " }\n");
-out:
+ spin_lock(&nf->fi_lock);
+ file = nf->fi_deleg_file;
+ if (file) {
+ seq_puts(s, ", ");
+ nfs4_show_superblock(s, file);
+ seq_puts(s, ", ");
+ nfs4_show_fname(s, file);
+ }
spin_unlock(&nf->fi_lock);
+ if (st->sc_status & NFS4_STID_ADMIN_REVOKED)
+ seq_puts(s, ", admin-revoked");
+ seq_puts(s, " }\n");
return 0;
}

--
2.42.0

2023-11-17 11:27:38

by Jeffrey Layton

Subject: Re: [PATCH 5/9] nfsd: allow admin-revoked state to appear in /proc/fs/nfsd/clients/*/states

On Fri, 2023-11-17 at 13:18 +1100, NeilBrown wrote:
> Change the "show" functions to show some content even if a file cannot
> be found.
> This is primarily useful for debugging - to ensure states are being
> removed eventually.
>
> Signed-off-by: NeilBrown <[email protected]>
> ---
> fs/nfsd/nfs4state.c | 81 +++++++++++++++++++++++----------------------
> 1 file changed, 41 insertions(+), 40 deletions(-)
>
> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
> index a2b3320a6ba8..8debd148840f 100644
> --- a/fs/nfsd/nfs4state.c
> +++ b/fs/nfsd/nfs4state.c
> @@ -2680,17 +2680,10 @@ static int nfs4_show_open(struct seq_file *s, struct nfs4_stid *st)
> struct nfs4_stateowner *oo;
> unsigned int access, deny;
>
> - if (st->sc_type != NFS4_OPEN_STID && st->sc_type != NFS4_LOCK_STID)
> - return 0; /* XXX: or SEQ_SKIP? */
> ols = openlockstateid(st);
> oo = ols->st_stateowner;
> nf = st->sc_file;
>
> - spin_lock(&nf->fi_lock);
> - file = find_any_file_locked(nf);
> - if (!file)
> - goto out;
> -
> seq_printf(s, "- ");
> nfs4_show_stateid(s, &st->sc_stateid);
> seq_printf(s, ": { type: open, ");
> @@ -2705,14 +2698,19 @@ static int nfs4_show_open(struct seq_file *s, struct nfs4_stid *st)
> deny & NFS4_SHARE_ACCESS_READ ? "r" : "-",
> deny & NFS4_SHARE_ACCESS_WRITE ? "w" : "-");
>
> - nfs4_show_superblock(s, file);
> - seq_printf(s, ", ");
> - nfs4_show_fname(s, file);
> - seq_printf(s, ", ");
> + spin_lock(&nf->fi_lock);
> + file = find_any_file_locked(nf);
> + if (file) {
> + nfs4_show_superblock(s, file);
> + seq_puts(s, ", ");
> + nfs4_show_fname(s, file);
> + seq_puts(s, ", ");
> + }
> + spin_unlock(&nf->fi_lock);
> nfs4_show_owner(s, oo);
> + if (st->sc_status & NFS4_STID_ADMIN_REVOKED)
> + seq_puts(s, ", admin-revoked");
> seq_printf(s, " }\n");
> -out:
> - spin_unlock(&nf->fi_lock);
> return 0;
> }
>
> @@ -2726,30 +2724,31 @@ static int nfs4_show_lock(struct seq_file *s, struct nfs4_stid *st)
> ols = openlockstateid(st);
> oo = ols->st_stateowner;
> nf = st->sc_file;
> - spin_lock(&nf->fi_lock);
> - file = find_any_file_locked(nf);
> - if (!file)
> - goto out;
>
> seq_printf(s, "- ");
> nfs4_show_stateid(s, &st->sc_stateid);
> seq_printf(s, ": { type: lock, ");
>
> - /*
> - * Note: a lock stateid isn't really the same thing as a lock,
> - * it's the locking state held by one owner on a file, and there
> - * may be multiple (or no) lock ranges associated with it.
> - * (Same for the matter is true of open stateids.)
> - */
> + spin_lock(&nf->fi_lock);
> + file = find_any_file_locked(nf);
> + if (file) {
> + /*
> + * Note: a lock stateid isn't really the same thing as a lock,
> + * it's the locking state held by one owner on a file, and there
> + * may be multiple (or no) lock ranges associated with it.
> + * (Same for the matter is true of open stateids.)
> + */
>
> - nfs4_show_superblock(s, file);
> - /* XXX: open stateid? */
> - seq_printf(s, ", ");
> - nfs4_show_fname(s, file);
> - seq_printf(s, ", ");
> + nfs4_show_superblock(s, file);
> + /* XXX: open stateid? */
> + seq_puts(s, ", ");
> + nfs4_show_fname(s, file);
> + seq_puts(s, ", ");
> + }
> nfs4_show_owner(s, oo);
> + if (st->sc_status & NFS4_STID_ADMIN_REVOKED)
> + seq_puts(s, ", admin-revoked");
> seq_printf(s, " }\n");
> -out:
> spin_unlock(&nf->fi_lock);
> return 0;
> }
> @@ -2762,27 +2761,29 @@ static int nfs4_show_deleg(struct seq_file *s, struct nfs4_stid *st)
>
> ds = delegstateid(st);
> nf = st->sc_file;
> - spin_lock(&nf->fi_lock);
> - file = nf->fi_deleg_file;
> - if (!file)
> - goto out;
>
> seq_printf(s, "- ");
> nfs4_show_stateid(s, &st->sc_stateid);
> seq_printf(s, ": { type: deleg, ");
>
> /* Kinda dead code as long as we only support read delegs: */

nit: You can probably remove the above comment since we now support
write delegations.

> - seq_printf(s, "access: %s, ",
> - ds->dl_type == NFS4_OPEN_DELEGATE_READ ? "r" : "w");
> + seq_printf(s, "access: %s",
> + ds->dl_type == NFS4_OPEN_DELEGATE_READ ? "r" : "w");
>
> /* XXX: lease time, whether it's being recalled. */
>
> - nfs4_show_superblock(s, file);
> - seq_printf(s, ", ");
> - nfs4_show_fname(s, file);
> - seq_printf(s, " }\n");
> -out:
> + spin_lock(&nf->fi_lock);
> + file = nf->fi_deleg_file;
> + if (file) {
> + seq_puts(s, ", ");
> + nfs4_show_superblock(s, file);
> + seq_puts(s, ", ");
> + nfs4_show_fname(s, file);
> + }
> spin_unlock(&nf->fi_lock);
> + if (st->sc_status & NFS4_STID_ADMIN_REVOKED)
> + seq_puts(s, ", admin-revoked");
> + seq_puts(s, " }\n");
> return 0;
> }
>

--
Jeff Layton <[email protected]>

2023-11-19 22:46:03

by NeilBrown

Subject: Re: [PATCH 5/9] nfsd: allow admin-revoked state to appear in /proc/fs/nfsd/clients/*/states

On Fri, 17 Nov 2023, Jeff Layton wrote:
> On Fri, 2023-11-17 at 13:18 +1100, NeilBrown wrote:
> > Change the "show" functions to show some content even if a file cannot
> > be found.
> > This is primarily useful for debugging - to ensure states are being
> > removed eventually.
> >
> > Signed-off-by: NeilBrown <[email protected]>
> > ---
> > fs/nfsd/nfs4state.c | 81 +++++++++++++++++++++++----------------------
> > 1 file changed, 41 insertions(+), 40 deletions(-)
> >
> > diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
> > index a2b3320a6ba8..8debd148840f 100644
> > --- a/fs/nfsd/nfs4state.c
> > +++ b/fs/nfsd/nfs4state.c
> > @@ -2680,17 +2680,10 @@ static int nfs4_show_open(struct seq_file *s, struct nfs4_stid *st)
> > struct nfs4_stateowner *oo;
> > unsigned int access, deny;
> >
> > - if (st->sc_type != NFS4_OPEN_STID && st->sc_type != NFS4_LOCK_STID)
> > - return 0; /* XXX: or SEQ_SKIP? */
> > ols = openlockstateid(st);
> > oo = ols->st_stateowner;
> > nf = st->sc_file;
> >
> > - spin_lock(&nf->fi_lock);
> > - file = find_any_file_locked(nf);
> > - if (!file)
> > - goto out;
> > -
> > seq_printf(s, "- ");
> > nfs4_show_stateid(s, &st->sc_stateid);
> > seq_printf(s, ": { type: open, ");
> > @@ -2705,14 +2698,19 @@ static int nfs4_show_open(struct seq_file *s, struct nfs4_stid *st)
> > deny & NFS4_SHARE_ACCESS_READ ? "r" : "-",
> > deny & NFS4_SHARE_ACCESS_WRITE ? "w" : "-");
> >
> > - nfs4_show_superblock(s, file);
> > - seq_printf(s, ", ");
> > - nfs4_show_fname(s, file);
> > - seq_printf(s, ", ");
> > + spin_lock(&nf->fi_lock);
> > + file = find_any_file_locked(nf);
> > + if (file) {
> > + nfs4_show_superblock(s, file);
> > + seq_puts(s, ", ");
> > + nfs4_show_fname(s, file);
> > + seq_puts(s, ", ");
> > + }
> > + spin_unlock(&nf->fi_lock);
> > nfs4_show_owner(s, oo);
> > + if (st->sc_status & NFS4_STID_ADMIN_REVOKED)
> > + seq_puts(s, ", admin-revoked");
> > seq_printf(s, " }\n");
> > -out:
> > - spin_unlock(&nf->fi_lock);
> > return 0;
> > }
> >
> > @@ -2726,30 +2724,31 @@ static int nfs4_show_lock(struct seq_file *s, struct nfs4_stid *st)
> > ols = openlockstateid(st);
> > oo = ols->st_stateowner;
> > nf = st->sc_file;
> > - spin_lock(&nf->fi_lock);
> > - file = find_any_file_locked(nf);
> > - if (!file)
> > - goto out;
> >
> > seq_printf(s, "- ");
> > nfs4_show_stateid(s, &st->sc_stateid);
> > seq_printf(s, ": { type: lock, ");
> >
> > - /*
> > - * Note: a lock stateid isn't really the same thing as a lock,
> > - * it's the locking state held by one owner on a file, and there
> > - * may be multiple (or no) lock ranges associated with it.
> > - * (Same for the matter is true of open stateids.)
> > - */
> > + spin_lock(&nf->fi_lock);
> > + file = find_any_file_locked(nf);
> > + if (file) {
> > + /*
> > + * Note: a lock stateid isn't really the same thing as a lock,
> > + * it's the locking state held by one owner on a file, and there
> > + * may be multiple (or no) lock ranges associated with it.
> > + * (Same for the matter is true of open stateids.)
> > + */
> >
> > - nfs4_show_superblock(s, file);
> > - /* XXX: open stateid? */
> > - seq_printf(s, ", ");
> > - nfs4_show_fname(s, file);
> > - seq_printf(s, ", ");
> > + nfs4_show_superblock(s, file);
> > + /* XXX: open stateid? */
> > + seq_puts(s, ", ");
> > + nfs4_show_fname(s, file);
> > + seq_puts(s, ", ");
> > + }
> > nfs4_show_owner(s, oo);
> > + if (st->sc_status & NFS4_STID_ADMIN_REVOKED)
> > + seq_puts(s, ", admin-revoked");
> > seq_printf(s, " }\n");
> > -out:
> > spin_unlock(&nf->fi_lock);
> > return 0;
> > }
> > @@ -2762,27 +2761,29 @@ static int nfs4_show_deleg(struct seq_file *s, struct nfs4_stid *st)
> >
> > ds = delegstateid(st);
> > nf = st->sc_file;
> > - spin_lock(&nf->fi_lock);
> > - file = nf->fi_deleg_file;
> > - if (!file)
> > - goto out;
> >
> > seq_printf(s, "- ");
> > nfs4_show_stateid(s, &st->sc_stateid);
> > seq_printf(s, ": { type: deleg, ");
> >
> > /* Kinda dead code as long as we only support read delegs: */
>
> nit: You can probably remove the above comment since we now support
> write delegations.

Done. Thanks for the suggestion.

NeilBrown


>
> > - seq_printf(s, "access: %s, ",
> > - ds->dl_type == NFS4_OPEN_DELEGATE_READ ? "r" : "w");
> > + seq_printf(s, "access: %s",
> > + ds->dl_type == NFS4_OPEN_DELEGATE_READ ? "r" : "w");
> >
> > /* XXX: lease time, whether it's being recalled. */
> >
> > - nfs4_show_superblock(s, file);
> > - seq_printf(s, ", ");
> > - nfs4_show_fname(s, file);
> > - seq_printf(s, " }\n");
> > -out:
> > + spin_lock(&nf->fi_lock);
> > + file = nf->fi_deleg_file;
> > + if (file) {
> > + seq_puts(s, ", ");
> > + nfs4_show_superblock(s, file);
> > + seq_puts(s, ", ");
> > + nfs4_show_fname(s, file);
> > + }
> > spin_unlock(&nf->fi_lock);
> > + if (st->sc_status & NFS4_STID_ADMIN_REVOKED)
> > + seq_puts(s, ", admin-revoked");
> > + seq_puts(s, " }\n");
> > return 0;
> > }
> >
>
> --
> Jeff Layton <[email protected]>
>