diff mbox series

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

Message ID 20231117022121.23310-6-neilb@suse.de (mailing list archive)
State New, archived
Headers show
Series support admin-revocation of v4 state | expand

Commit Message

NeilBrown Nov. 17, 2023, 2:18 a.m. UTC 
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 <neilb@suse.de>
---
 fs/nfsd/nfs4state.c | 81 +++++++++++++++++++++++----------------------
 1 file changed, 41 insertions(+), 40 deletions(-)

Comments

Jeff Layton Nov. 17, 2023, 11:27 a.m. UTC | #1 
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 <neilb@suse.de>
> ---
>  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;
>  }
>
NeilBrown Nov. 19, 2023, 10:36 p.m. UTC | #2 
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 <neilb@suse.de>
> > ---
> >  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 <jlayton@kernel.org>
>
diff mbox series

Patch

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