The NFSv4 protocol allows state to be revoked by the admin and has error
codes which allow this to be communicated to the client.
This patch
- introduces a new state-id status NFS4_STID_ADMIN_REVOKE
which can be set on open, lock, or delegation state.
- reports NFS4ERR_ADMIN_REVOKED when these are accessed
- introduces a per-client counter of these states and returns
SEQ4_STATUS_ADMIN_STATE_REVOKED when the counter is not zero.
Decrements this when freeing any admin-revoked state.
- introduces stub code to find all interesting states for a given
superblock so they can be revoked via the 'unlock_filesystem'
file in /proc/fs/nfsd/
No actual states are handled yet.
Signed-off-by: NeilBrown <[email protected]>
---
fs/nfsd/nfs4state.c | 71 ++++++++++++++++++++++++++++++++++++++++++++-
fs/nfsd/nfsctl.c | 1 +
fs/nfsd/nfsd.h | 1 +
fs/nfsd/state.h | 10 +++++++
fs/nfsd/trace.h | 3 +-
5 files changed, 84 insertions(+), 2 deletions(-)
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index b9239f2ebc79..477a9e9aebbd 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -1215,6 +1215,8 @@ nfs4_put_stid(struct nfs4_stid *s)
return;
}
idr_remove(&clp->cl_stateids, s->sc_stateid.si_opaque.so_id);
+ if (s->sc_status & NFS4_STID_ADMIN_REVOKED)
+ atomic_dec(&s->sc_client->cl_admin_revoked);
nfs4_free_cpntf_statelist(clp->net, s);
spin_unlock(&clp->cl_lock);
s->sc_free(s);
@@ -1534,6 +1536,8 @@ static void put_ol_stateid_locked(struct nfs4_ol_stateid *stp,
}
idr_remove(&clp->cl_stateids, s->sc_stateid.si_opaque.so_id);
+ if (s->sc_status & NFS4_STID_ADMIN_REVOKED)
+ atomic_dec(&s->sc_client->cl_admin_revoked);
list_add(&stp->st_locks, reaplist);
}
@@ -1679,6 +1683,54 @@ static void release_openowner(struct nfs4_openowner *oo)
nfs4_put_stateowner(&oo->oo_owner);
}
+static struct nfs4_stid *find_one_sb_stid(struct nfs4_client *clp,
+ struct super_block *sb,
+ unsigned int sc_types)
+{
+ unsigned long id, tmp;
+ struct nfs4_stid *stid;
+
+ spin_lock(&clp->cl_lock);
+ idr_for_each_entry_ul(&clp->cl_stateids, stid, tmp, id)
+ if ((stid->sc_type & sc_types) &&
+ stid->sc_status == 0 &&
+ stid->sc_file->fi_inode->i_sb == sb) {
+ refcount_inc(&stid->sc_count);
+ break;
+ }
+ spin_unlock(&clp->cl_lock);
+ return stid;
+}
+
+void nfsd4_revoke_states(struct net *net, struct super_block *sb)
+{
+ struct nfsd_net *nn = net_generic(net, nfsd_net_id);
+ unsigned int idhashval;
+ unsigned int sc_types;
+
+ sc_types = 0;
+
+ spin_lock(&nn->client_lock);
+ for (idhashval = 0; idhashval < CLIENT_HASH_MASK; idhashval++) {
+ struct list_head *head = &nn->conf_id_hashtbl[idhashval];
+ struct nfs4_client *clp;
+ retry:
+ list_for_each_entry(clp, head, cl_idhash) {
+ struct nfs4_stid *stid = find_one_sb_stid(clp, sb,
+ sc_types);
+ if (stid) {
+ spin_unlock(&nn->client_lock);
+ switch (stid->sc_type) {
+ }
+ nfs4_put_stid(stid);
+ spin_lock(&nn->client_lock);
+ goto retry;
+ }
+ }
+ }
+ spin_unlock(&nn->client_lock);
+}
+
static inline int
hash_sessionid(struct nfs4_sessionid *sessionid)
{
@@ -2550,6 +2602,8 @@ static int client_info_show(struct seq_file *m, void *v)
}
seq_printf(m, "callback state: %s\n", cb_state2str(clp->cl_cb_state));
seq_printf(m, "callback address: %pISpc\n", &clp->cl_cb_conn.cb_addr);
+ seq_printf(m, "admin-revoked states: %d\n",
+ atomic_read(&clp->cl_admin_revoked));
drop_client(clp);
return 0;
@@ -4109,6 +4163,8 @@ nfsd4_sequence(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
}
if (!list_empty(&clp->cl_revoked))
seq->status_flags |= SEQ4_STATUS_RECALLABLE_STATE_REVOKED;
+ if (atomic_read(&clp->cl_admin_revoked))
+ seq->status_flags |= SEQ4_STATUS_ADMIN_STATE_REVOKED;
out_no_session:
if (conn)
free_conn(conn);
@@ -4597,7 +4653,9 @@ nfsd4_verify_open_stid(struct nfs4_stid *s)
{
__be32 ret = nfs_ok;
- if (s->sc_status & NFS4_STID_REVOKED)
+ if (s->sc_status & NFS4_STID_ADMIN_REVOKED)
+ ret = nfserr_admin_revoked;
+ else if (s->sc_status & NFS4_STID_REVOKED)
ret = nfserr_deleg_revoked;
else if (s->sc_status & NFS4_STID_CLOSED)
ret = nfserr_bad_stateid;
@@ -5188,6 +5246,11 @@ nfs4_check_deleg(struct nfs4_client *cl, struct nfsd4_open *open,
deleg = find_deleg_stateid(cl, &open->op_delegate_stateid);
if (deleg == NULL)
goto out;
+ if (deleg->dl_stid.sc_status & NFS4_STID_ADMIN_REVOKED) {
+ nfs4_put_stid(&deleg->dl_stid);
+ status = nfserr_admin_revoked;
+ goto out;
+ }
if (deleg->dl_stid.sc_status & NFS4_STID_REVOKED) {
nfs4_put_stid(&deleg->dl_stid);
status = nfserr_deleg_revoked;
@@ -6508,6 +6571,8 @@ nfsd4_lookup_stateid(struct nfsd4_compound_state *cstate,
*/
statusmask |= NFS4_STID_REVOKED;
+ statusmask |= NFS4_STID_ADMIN_REVOKED;
+
if (ZERO_STATEID(stateid) || ONE_STATEID(stateid) ||
CLOSE_STATEID(stateid))
return nfserr_bad_stateid;
@@ -6526,6 +6591,10 @@ nfsd4_lookup_stateid(struct nfsd4_compound_state *cstate,
nfs4_put_stid(stid);
return nfserr_deleg_revoked;
}
+ if (stid->sc_type & NFS4_STID_ADMIN_REVOKED) {
+ nfs4_put_stid(stid);
+ return nfserr_admin_revoked;
+ }
*s = stid;
return nfs_ok;
}
diff --git a/fs/nfsd/nfsctl.c b/fs/nfsd/nfsctl.c
index d6eeee149370..a622d773f428 100644
--- a/fs/nfsd/nfsctl.c
+++ b/fs/nfsd/nfsctl.c
@@ -285,6 +285,7 @@ static ssize_t write_unlock_fs(struct file *file, char *buf, size_t size)
* 3. Is that directory the root of an exported file system?
*/
error = nlmsvc_unlock_all_by_sb(path.dentry->d_sb);
+ nfsd4_revoke_states(netns(file), path.dentry->d_sb);
path_put(&path);
return error;
diff --git a/fs/nfsd/nfsd.h b/fs/nfsd/nfsd.h
index f5ff42f41ee7..d46203eac3c8 100644
--- a/fs/nfsd/nfsd.h
+++ b/fs/nfsd/nfsd.h
@@ -280,6 +280,7 @@ void nfsd_lockd_shutdown(void);
#define nfserr_no_grace cpu_to_be32(NFSERR_NO_GRACE)
#define nfserr_reclaim_bad cpu_to_be32(NFSERR_RECLAIM_BAD)
#define nfserr_badname cpu_to_be32(NFSERR_BADNAME)
+#define nfserr_admin_revoked cpu_to_be32(NFS4ERR_ADMIN_REVOKED)
#define nfserr_cb_path_down cpu_to_be32(NFSERR_CB_PATH_DOWN)
#define nfserr_locked cpu_to_be32(NFSERR_LOCKED)
#define nfserr_wrongsec cpu_to_be32(NFSERR_WRONGSEC)
diff --git a/fs/nfsd/state.h b/fs/nfsd/state.h
index bb00dcd4c1ba..584378c43e0a 100644
--- a/fs/nfsd/state.h
+++ b/fs/nfsd/state.h
@@ -112,6 +112,7 @@ struct nfs4_stid {
#define NFS4_STID_CLOSED BIT(0)
/* For a deleg stateid kept around only to process free_stateid's: */
#define NFS4_STID_REVOKED BIT(1)
+#define NFS4_STID_ADMIN_REVOKED BIT(2)
unsigned short sc_status;
struct list_head sc_cp_list;
@@ -388,6 +389,7 @@ struct nfs4_client {
clientid_t cl_clientid; /* generated by server */
nfs4_verifier cl_confirm; /* generated by server */
u32 cl_minorversion;
+ atomic_t cl_admin_revoked; /* count of admin-revoked states */
/* NFSv4.1 client implementation id: */
struct xdr_netobj cl_nii_domain;
struct xdr_netobj cl_nii_name;
@@ -752,6 +754,14 @@ static inline void get_nfs4_file(struct nfs4_file *fi)
}
struct nfsd_file *find_any_file(struct nfs4_file *f);
+#ifdef CONFIG_NFSD_V4
+void nfsd4_revoke_states(struct net *net, struct super_block *sb);
+#else
+static inline void nfsd4_revoke_states(struct net *net, struct super_block *sb)
+{
+}
+#endif
+
/* grace period management */
void nfsd4_end_grace(struct nfsd_net *nn);
diff --git a/fs/nfsd/trace.h b/fs/nfsd/trace.h
index 568b4ec9a2af..281aeb42c9eb 100644
--- a/fs/nfsd/trace.h
+++ b/fs/nfsd/trace.h
@@ -651,7 +651,8 @@ DEFINE_STATESEQID_EVENT(open_confirm);
#define show_stid_status(x) \
__print_flags(x, "|", \
{ NFS4_STID_CLOSED, "CLOSED" }, \
- { NFS4_STID_REVOKED, "REVOKED" }) \
+ { NFS4_STID_REVOKED, "REVOKED" }, \
+ { NFS4_STID_ADMIN_REVOKED, "ADMIN_REVOKED" })
DECLARE_EVENT_CLASS(nfsd_stid_class,
TP_PROTO(
--
2.42.1
On Fri, Nov 24, 2023 at 11:28:40AM +1100, NeilBrown wrote:
> The NFSv4 protocol allows state to be revoked by the admin and has error
> codes which allow this to be communicated to the client.
>
> This patch
> - introduces a new state-id status NFS4_STID_ADMIN_REVOKE
> which can be set on open, lock, or delegation state.
> - reports NFS4ERR_ADMIN_REVOKED when these are accessed
> - introduces a per-client counter of these states and returns
> SEQ4_STATUS_ADMIN_STATE_REVOKED when the counter is not zero.
> Decrements this when freeing any admin-revoked state.
> - introduces stub code to find all interesting states for a given
> superblock so they can be revoked via the 'unlock_filesystem'
> file in /proc/fs/nfsd/
> No actual states are handled yet.
>
> Signed-off-by: NeilBrown <[email protected]>
> ---
> fs/nfsd/nfs4state.c | 71 ++++++++++++++++++++++++++++++++++++++++++++-
> fs/nfsd/nfsctl.c | 1 +
> fs/nfsd/nfsd.h | 1 +
> fs/nfsd/state.h | 10 +++++++
> fs/nfsd/trace.h | 3 +-
> 5 files changed, 84 insertions(+), 2 deletions(-)
>
> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
> index b9239f2ebc79..477a9e9aebbd 100644
> --- a/fs/nfsd/nfs4state.c
> +++ b/fs/nfsd/nfs4state.c
> @@ -1215,6 +1215,8 @@ nfs4_put_stid(struct nfs4_stid *s)
> return;
> }
> idr_remove(&clp->cl_stateids, s->sc_stateid.si_opaque.so_id);
> + if (s->sc_status & NFS4_STID_ADMIN_REVOKED)
> + atomic_dec(&s->sc_client->cl_admin_revoked);
> nfs4_free_cpntf_statelist(clp->net, s);
> spin_unlock(&clp->cl_lock);
> s->sc_free(s);
> @@ -1534,6 +1536,8 @@ static void put_ol_stateid_locked(struct nfs4_ol_stateid *stp,
> }
>
> idr_remove(&clp->cl_stateids, s->sc_stateid.si_opaque.so_id);
> + if (s->sc_status & NFS4_STID_ADMIN_REVOKED)
> + atomic_dec(&s->sc_client->cl_admin_revoked);
> list_add(&stp->st_locks, reaplist);
> }
>
> @@ -1679,6 +1683,54 @@ static void release_openowner(struct nfs4_openowner *oo)
> nfs4_put_stateowner(&oo->oo_owner);
> }
>
> +static struct nfs4_stid *find_one_sb_stid(struct nfs4_client *clp,
> + struct super_block *sb,
> + unsigned int sc_types)
> +{
> + unsigned long id, tmp;
> + struct nfs4_stid *stid;
> +
> + spin_lock(&clp->cl_lock);
> + idr_for_each_entry_ul(&clp->cl_stateids, stid, tmp, id)
> + if ((stid->sc_type & sc_types) &&
> + stid->sc_status == 0 &&
> + stid->sc_file->fi_inode->i_sb == sb) {
> + refcount_inc(&stid->sc_count);
> + break;
> + }
> + spin_unlock(&clp->cl_lock);
> + return stid;
> +}
> +
nfsd4_revoke_states() needs a kdoc comment.
> +void nfsd4_revoke_states(struct net *net, struct super_block *sb)
> +{
> + struct nfsd_net *nn = net_generic(net, nfsd_net_id);
> + unsigned int idhashval;
> + unsigned int sc_types;
> +
> + sc_types = 0;
> +
> + spin_lock(&nn->client_lock);
> + for (idhashval = 0; idhashval < CLIENT_HASH_MASK; idhashval++) {
> + struct list_head *head = &nn->conf_id_hashtbl[idhashval];
> + struct nfs4_client *clp;
> + retry:
> + list_for_each_entry(clp, head, cl_idhash) {
> + struct nfs4_stid *stid = find_one_sb_stid(clp, sb,
> + sc_types);
> + if (stid) {
> + spin_unlock(&nn->client_lock);
> + switch (stid->sc_type) {
This is "dead" code, for now. Does this stub really need to be
introduced in this patch?
> + }
> + nfs4_put_stid(stid);
> + spin_lock(&nn->client_lock);
> + goto retry;
> + }
> + }
> + }
> + spin_unlock(&nn->client_lock);
> +}
> +
> static inline int
> hash_sessionid(struct nfs4_sessionid *sessionid)
> {
> @@ -2550,6 +2602,8 @@ static int client_info_show(struct seq_file *m, void *v)
> }
> seq_printf(m, "callback state: %s\n", cb_state2str(clp->cl_cb_state));
> seq_printf(m, "callback address: %pISpc\n", &clp->cl_cb_conn.cb_addr);
> + seq_printf(m, "admin-revoked states: %d\n",
> + atomic_read(&clp->cl_admin_revoked));
> drop_client(clp);
>
> return 0;
> @@ -4109,6 +4163,8 @@ nfsd4_sequence(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
> }
> if (!list_empty(&clp->cl_revoked))
> seq->status_flags |= SEQ4_STATUS_RECALLABLE_STATE_REVOKED;
> + if (atomic_read(&clp->cl_admin_revoked))
> + seq->status_flags |= SEQ4_STATUS_ADMIN_STATE_REVOKED;
> out_no_session:
> if (conn)
> free_conn(conn);
> @@ -4597,7 +4653,9 @@ nfsd4_verify_open_stid(struct nfs4_stid *s)
> {
> __be32 ret = nfs_ok;
>
> - if (s->sc_status & NFS4_STID_REVOKED)
> + if (s->sc_status & NFS4_STID_ADMIN_REVOKED)
> + ret = nfserr_admin_revoked;
> + else if (s->sc_status & NFS4_STID_REVOKED)
> ret = nfserr_deleg_revoked;
> else if (s->sc_status & NFS4_STID_CLOSED)
> ret = nfserr_bad_stateid;
> @@ -5188,6 +5246,11 @@ nfs4_check_deleg(struct nfs4_client *cl, struct nfsd4_open *open,
> deleg = find_deleg_stateid(cl, &open->op_delegate_stateid);
> if (deleg == NULL)
> goto out;
> + if (deleg->dl_stid.sc_status & NFS4_STID_ADMIN_REVOKED) {
> + nfs4_put_stid(&deleg->dl_stid);
> + status = nfserr_admin_revoked;
> + goto out;
> + }
> if (deleg->dl_stid.sc_status & NFS4_STID_REVOKED) {
> nfs4_put_stid(&deleg->dl_stid);
> status = nfserr_deleg_revoked;
> @@ -6508,6 +6571,8 @@ nfsd4_lookup_stateid(struct nfsd4_compound_state *cstate,
> */
> statusmask |= NFS4_STID_REVOKED;
>
> + statusmask |= NFS4_STID_ADMIN_REVOKED;
> +
> if (ZERO_STATEID(stateid) || ONE_STATEID(stateid) ||
> CLOSE_STATEID(stateid))
> return nfserr_bad_stateid;
> @@ -6526,6 +6591,10 @@ nfsd4_lookup_stateid(struct nfsd4_compound_state *cstate,
> nfs4_put_stid(stid);
> return nfserr_deleg_revoked;
> }
> + if (stid->sc_type & NFS4_STID_ADMIN_REVOKED) {
> + nfs4_put_stid(stid);
> + return nfserr_admin_revoked;
> + }
> *s = stid;
> return nfs_ok;
> }
> diff --git a/fs/nfsd/nfsctl.c b/fs/nfsd/nfsctl.c
> index d6eeee149370..a622d773f428 100644
> --- a/fs/nfsd/nfsctl.c
> +++ b/fs/nfsd/nfsctl.c
> @@ -285,6 +285,7 @@ static ssize_t write_unlock_fs(struct file *file, char *buf, size_t size)
> * 3. Is that directory the root of an exported file system?
> */
> error = nlmsvc_unlock_all_by_sb(path.dentry->d_sb);
> + nfsd4_revoke_states(netns(file), path.dentry->d_sb);
>
> path_put(&path);
> return error;
> diff --git a/fs/nfsd/nfsd.h b/fs/nfsd/nfsd.h
> index f5ff42f41ee7..d46203eac3c8 100644
> --- a/fs/nfsd/nfsd.h
> +++ b/fs/nfsd/nfsd.h
> @@ -280,6 +280,7 @@ void nfsd_lockd_shutdown(void);
> #define nfserr_no_grace cpu_to_be32(NFSERR_NO_GRACE)
> #define nfserr_reclaim_bad cpu_to_be32(NFSERR_RECLAIM_BAD)
> #define nfserr_badname cpu_to_be32(NFSERR_BADNAME)
> +#define nfserr_admin_revoked cpu_to_be32(NFS4ERR_ADMIN_REVOKED)
> #define nfserr_cb_path_down cpu_to_be32(NFSERR_CB_PATH_DOWN)
> #define nfserr_locked cpu_to_be32(NFSERR_LOCKED)
> #define nfserr_wrongsec cpu_to_be32(NFSERR_WRONGSEC)
> diff --git a/fs/nfsd/state.h b/fs/nfsd/state.h
> index bb00dcd4c1ba..584378c43e0a 100644
> --- a/fs/nfsd/state.h
> +++ b/fs/nfsd/state.h
> @@ -112,6 +112,7 @@ struct nfs4_stid {
> #define NFS4_STID_CLOSED BIT(0)
> /* For a deleg stateid kept around only to process free_stateid's: */
> #define NFS4_STID_REVOKED BIT(1)
> +#define NFS4_STID_ADMIN_REVOKED BIT(2)
The names of these mask bits are now getting to be visually
indistinguishable from the stateid type names. The subtlety of
where the _STID_ falls in the name makes me blink a few times when
reading this code.
It would be a little more friendly to add _STATUS_ or some other
infix that makes it easy to tell these are not stateid types. I
know that makes the names longer and more unwieldy.
> unsigned short sc_status;
>
> struct list_head sc_cp_list;
> @@ -388,6 +389,7 @@ struct nfs4_client {
> clientid_t cl_clientid; /* generated by server */
> nfs4_verifier cl_confirm; /* generated by server */
> u32 cl_minorversion;
> + atomic_t cl_admin_revoked; /* count of admin-revoked states */
> /* NFSv4.1 client implementation id: */
> struct xdr_netobj cl_nii_domain;
> struct xdr_netobj cl_nii_name;
> @@ -752,6 +754,14 @@ static inline void get_nfs4_file(struct nfs4_file *fi)
> }
> struct nfsd_file *find_any_file(struct nfs4_file *f);
>
> +#ifdef CONFIG_NFSD_V4
> +void nfsd4_revoke_states(struct net *net, struct super_block *sb);
> +#else
> +static inline void nfsd4_revoke_states(struct net *net, struct super_block *sb)
> +{
> +}
> +#endif
> +
> /* grace period management */
> void nfsd4_end_grace(struct nfsd_net *nn);
>
> diff --git a/fs/nfsd/trace.h b/fs/nfsd/trace.h
> index 568b4ec9a2af..281aeb42c9eb 100644
> --- a/fs/nfsd/trace.h
> +++ b/fs/nfsd/trace.h
> @@ -651,7 +651,8 @@ DEFINE_STATESEQID_EVENT(open_confirm);
> #define show_stid_status(x) \
> __print_flags(x, "|", \
> { NFS4_STID_CLOSED, "CLOSED" }, \
> - { NFS4_STID_REVOKED, "REVOKED" }) \
> + { NFS4_STID_REVOKED, "REVOKED" }, \
> + { NFS4_STID_ADMIN_REVOKED, "ADMIN_REVOKED" })
>
> DECLARE_EVENT_CLASS(nfsd_stid_class,
> TP_PROTO(
> --
> 2.42.1
>
--
Chuck Lever
On Mon, 27 Nov 2023, Chuck Lever wrote:
> On Fri, Nov 24, 2023 at 11:28:40AM +1100, NeilBrown wrote:
> > The NFSv4 protocol allows state to be revoked by the admin and has error
> > codes which allow this to be communicated to the client.
> >
> > This patch
> > - introduces a new state-id status NFS4_STID_ADMIN_REVOKE
> > which can be set on open, lock, or delegation state.
> > - reports NFS4ERR_ADMIN_REVOKED when these are accessed
> > - introduces a per-client counter of these states and returns
> > SEQ4_STATUS_ADMIN_STATE_REVOKED when the counter is not zero.
> > Decrements this when freeing any admin-revoked state.
> > - introduces stub code to find all interesting states for a given
> > superblock so they can be revoked via the 'unlock_filesystem'
> > file in /proc/fs/nfsd/
> > No actual states are handled yet.
> >
> > Signed-off-by: NeilBrown <[email protected]>
> > ---
> > fs/nfsd/nfs4state.c | 71 ++++++++++++++++++++++++++++++++++++++++++++-
> > fs/nfsd/nfsctl.c | 1 +
> > fs/nfsd/nfsd.h | 1 +
> > fs/nfsd/state.h | 10 +++++++
> > fs/nfsd/trace.h | 3 +-
> > 5 files changed, 84 insertions(+), 2 deletions(-)
> >
> > diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
> > index b9239f2ebc79..477a9e9aebbd 100644
> > --- a/fs/nfsd/nfs4state.c
> > +++ b/fs/nfsd/nfs4state.c
> > @@ -1215,6 +1215,8 @@ nfs4_put_stid(struct nfs4_stid *s)
> > return;
> > }
> > idr_remove(&clp->cl_stateids, s->sc_stateid.si_opaque.so_id);
> > + if (s->sc_status & NFS4_STID_ADMIN_REVOKED)
> > + atomic_dec(&s->sc_client->cl_admin_revoked);
> > nfs4_free_cpntf_statelist(clp->net, s);
> > spin_unlock(&clp->cl_lock);
> > s->sc_free(s);
> > @@ -1534,6 +1536,8 @@ static void put_ol_stateid_locked(struct nfs4_ol_stateid *stp,
> > }
> >
> > idr_remove(&clp->cl_stateids, s->sc_stateid.si_opaque.so_id);
> > + if (s->sc_status & NFS4_STID_ADMIN_REVOKED)
> > + atomic_dec(&s->sc_client->cl_admin_revoked);
> > list_add(&stp->st_locks, reaplist);
> > }
> >
> > @@ -1679,6 +1683,54 @@ static void release_openowner(struct nfs4_openowner *oo)
> > nfs4_put_stateowner(&oo->oo_owner);
> > }
> >
> > +static struct nfs4_stid *find_one_sb_stid(struct nfs4_client *clp,
> > + struct super_block *sb,
> > + unsigned int sc_types)
> > +{
> > + unsigned long id, tmp;
> > + struct nfs4_stid *stid;
> > +
> > + spin_lock(&clp->cl_lock);
> > + idr_for_each_entry_ul(&clp->cl_stateids, stid, tmp, id)
> > + if ((stid->sc_type & sc_types) &&
> > + stid->sc_status == 0 &&
> > + stid->sc_file->fi_inode->i_sb == sb) {
> > + refcount_inc(&stid->sc_count);
> > + break;
> > + }
> > + spin_unlock(&clp->cl_lock);
> > + return stid;
> > +}
> > +
>
> nfsd4_revoke_states() needs a kdoc comment.
Done.
>
>
> > +void nfsd4_revoke_states(struct net *net, struct super_block *sb)
> > +{
> > + struct nfsd_net *nn = net_generic(net, nfsd_net_id);
> > + unsigned int idhashval;
> > + unsigned int sc_types;
> > +
> > + sc_types = 0;
> > +
> > + spin_lock(&nn->client_lock);
> > + for (idhashval = 0; idhashval < CLIENT_HASH_MASK; idhashval++) {
> > + struct list_head *head = &nn->conf_id_hashtbl[idhashval];
> > + struct nfs4_client *clp;
> > + retry:
> > + list_for_each_entry(clp, head, cl_idhash) {
> > + struct nfs4_stid *stid = find_one_sb_stid(clp, sb,
> > + sc_types);
> > + if (stid) {
> > + spin_unlock(&nn->client_lock);
> > + switch (stid->sc_type) {
>
> This is "dead" code, for now. Does this stub really need to be
> introduced in this patch?
"need" is a strong word..
The entire patch is "dead" code. I want to allow handling for the
different state types to be added one at a time. I could delay much of
this patch until handling the first state, but I think that would hurt
reviewability of the series...
>
>
> > + }
> > + nfs4_put_stid(stid);
> > + spin_lock(&nn->client_lock);
> > + goto retry;
> > + }
> > + }
> > + }
> > + spin_unlock(&nn->client_lock);
> > +}
> > +
> > static inline int
> > hash_sessionid(struct nfs4_sessionid *sessionid)
> > {
> > @@ -2550,6 +2602,8 @@ static int client_info_show(struct seq_file *m, void *v)
> > }
> > seq_printf(m, "callback state: %s\n", cb_state2str(clp->cl_cb_state));
> > seq_printf(m, "callback address: %pISpc\n", &clp->cl_cb_conn.cb_addr);
> > + seq_printf(m, "admin-revoked states: %d\n",
> > + atomic_read(&clp->cl_admin_revoked));
> > drop_client(clp);
> >
> > return 0;
> > @@ -4109,6 +4163,8 @@ nfsd4_sequence(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
> > }
> > if (!list_empty(&clp->cl_revoked))
> > seq->status_flags |= SEQ4_STATUS_RECALLABLE_STATE_REVOKED;
> > + if (atomic_read(&clp->cl_admin_revoked))
> > + seq->status_flags |= SEQ4_STATUS_ADMIN_STATE_REVOKED;
> > out_no_session:
> > if (conn)
> > free_conn(conn);
> > @@ -4597,7 +4653,9 @@ nfsd4_verify_open_stid(struct nfs4_stid *s)
> > {
> > __be32 ret = nfs_ok;
> >
> > - if (s->sc_status & NFS4_STID_REVOKED)
> > + if (s->sc_status & NFS4_STID_ADMIN_REVOKED)
> > + ret = nfserr_admin_revoked;
> > + else if (s->sc_status & NFS4_STID_REVOKED)
> > ret = nfserr_deleg_revoked;
> > else if (s->sc_status & NFS4_STID_CLOSED)
> > ret = nfserr_bad_stateid;
> > @@ -5188,6 +5246,11 @@ nfs4_check_deleg(struct nfs4_client *cl, struct nfsd4_open *open,
> > deleg = find_deleg_stateid(cl, &open->op_delegate_stateid);
> > if (deleg == NULL)
> > goto out;
> > + if (deleg->dl_stid.sc_status & NFS4_STID_ADMIN_REVOKED) {
> > + nfs4_put_stid(&deleg->dl_stid);
> > + status = nfserr_admin_revoked;
> > + goto out;
> > + }
> > if (deleg->dl_stid.sc_status & NFS4_STID_REVOKED) {
> > nfs4_put_stid(&deleg->dl_stid);
> > status = nfserr_deleg_revoked;
> > @@ -6508,6 +6571,8 @@ nfsd4_lookup_stateid(struct nfsd4_compound_state *cstate,
> > */
> > statusmask |= NFS4_STID_REVOKED;
> >
> > + statusmask |= NFS4_STID_ADMIN_REVOKED;
> > +
> > if (ZERO_STATEID(stateid) || ONE_STATEID(stateid) ||
> > CLOSE_STATEID(stateid))
> > return nfserr_bad_stateid;
> > @@ -6526,6 +6591,10 @@ nfsd4_lookup_stateid(struct nfsd4_compound_state *cstate,
> > nfs4_put_stid(stid);
> > return nfserr_deleg_revoked;
> > }
> > + if (stid->sc_type & NFS4_STID_ADMIN_REVOKED) {
> > + nfs4_put_stid(stid);
> > + return nfserr_admin_revoked;
> > + }
> > *s = stid;
> > return nfs_ok;
> > }
> > diff --git a/fs/nfsd/nfsctl.c b/fs/nfsd/nfsctl.c
> > index d6eeee149370..a622d773f428 100644
> > --- a/fs/nfsd/nfsctl.c
> > +++ b/fs/nfsd/nfsctl.c
> > @@ -285,6 +285,7 @@ static ssize_t write_unlock_fs(struct file *file, char *buf, size_t size)
> > * 3. Is that directory the root of an exported file system?
> > */
> > error = nlmsvc_unlock_all_by_sb(path.dentry->d_sb);
> > + nfsd4_revoke_states(netns(file), path.dentry->d_sb);
> >
> > path_put(&path);
> > return error;
> > diff --git a/fs/nfsd/nfsd.h b/fs/nfsd/nfsd.h
> > index f5ff42f41ee7..d46203eac3c8 100644
> > --- a/fs/nfsd/nfsd.h
> > +++ b/fs/nfsd/nfsd.h
> > @@ -280,6 +280,7 @@ void nfsd_lockd_shutdown(void);
> > #define nfserr_no_grace cpu_to_be32(NFSERR_NO_GRACE)
> > #define nfserr_reclaim_bad cpu_to_be32(NFSERR_RECLAIM_BAD)
> > #define nfserr_badname cpu_to_be32(NFSERR_BADNAME)
> > +#define nfserr_admin_revoked cpu_to_be32(NFS4ERR_ADMIN_REVOKED)
> > #define nfserr_cb_path_down cpu_to_be32(NFSERR_CB_PATH_DOWN)
> > #define nfserr_locked cpu_to_be32(NFSERR_LOCKED)
> > #define nfserr_wrongsec cpu_to_be32(NFSERR_WRONGSEC)
> > diff --git a/fs/nfsd/state.h b/fs/nfsd/state.h
> > index bb00dcd4c1ba..584378c43e0a 100644
> > --- a/fs/nfsd/state.h
> > +++ b/fs/nfsd/state.h
> > @@ -112,6 +112,7 @@ struct nfs4_stid {
> > #define NFS4_STID_CLOSED BIT(0)
> > /* For a deleg stateid kept around only to process free_stateid's: */
> > #define NFS4_STID_REVOKED BIT(1)
> > +#define NFS4_STID_ADMIN_REVOKED BIT(2)
>
> The names of these mask bits are now getting to be visually
> indistinguishable from the stateid type names. The subtlety of
> where the _STID_ falls in the name makes me blink a few times when
> reading this code.
>
> It would be a little more friendly to add _STATUS_ or some other
> infix that makes it easy to tell these are not stateid types. I
> know that makes the names longer and more unwieldy.
In an ideal world we could have just the words that 'trace' reports:
OPEN LOCK DELEG LAYOUT and CLOSED REVOKED ADMIN_REVOKED
and the language would tell us if the flag was not compatible with the
field it was stored in. But C does not provide that world so we need
something help the reader assess consistency.
Do we really need NFS4 here? These flags are local to nfsd/nfs4* (and
state.h and trace.h)
The values are stored in "sc_type" or "sc_status" (and occasionally
typemask or similar). So
TYPE_OPEN TYPE_DELETE TYPE_LAYOUT and STATUS_CLOSED STATUS_REVOKED STATUS_ADMIN_REVOKED
would be sufficiently informative for the reader. Putting "NFS4_STID_"
in front of each of those makes them unwieldy as you say, and doesn't add
any value that I can see. Possibly putting "SC_" in front to match the
field name could be justified.
Thoughts?
Thanks,
NeilBrown