* [PATCH 01/13] nfsd: remove stale comment in nfs4_show_deleg()
2024-01-29 3:29 [PATCH 00/13 v4] nfsd: support admin-revocation of v4 state NeilBrown
@ 2024-01-29 3:29 ` NeilBrown
2024-01-29 3:29 ` [PATCH 02/13] nfsd: hold ->cl_lock for hash_delegation_locked() NeilBrown
` (11 subsequent siblings)
12 siblings, 0 replies; 27+ messages in thread
From: NeilBrown @ 2024-01-29 3:29 UTC (permalink / raw)
To: Chuck Lever, Jeff Layton
Cc: linux-nfs, Olga Kornievskaia, Dai Ngo, Tom Talpey,
Christoph Hellwig, Tom Haynes
As we do now support write delegations, this comment is unhelpful and
misleading.
Reported-by: Jeff Layton <jlayton@kernel.org>
Signed-off-by: NeilBrown <neilb@suse.de>
---
fs/nfsd/nfs4state.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index 6dc6340e2852..d377a0a56e45 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -2711,7 +2711,6 @@ static int nfs4_show_deleg(struct seq_file *s, struct nfs4_stid *st)
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");
--
2.43.0
^ permalink raw reply related [flat|nested] 27+ messages in thread* [PATCH 02/13] nfsd: hold ->cl_lock for hash_delegation_locked()
2024-01-29 3:29 [PATCH 00/13 v4] nfsd: support admin-revocation of v4 state NeilBrown
2024-01-29 3:29 ` [PATCH 01/13] nfsd: remove stale comment in nfs4_show_deleg() NeilBrown
@ 2024-01-29 3:29 ` NeilBrown
2024-01-29 3:29 ` [PATCH 03/13] nfsd: don't call functions with side-effecting inside WARN_ON() NeilBrown
` (10 subsequent siblings)
12 siblings, 0 replies; 27+ messages in thread
From: NeilBrown @ 2024-01-29 3:29 UTC (permalink / raw)
To: Chuck Lever, Jeff Layton
Cc: linux-nfs, Olga Kornievskaia, Dai Ngo, Tom Talpey,
Christoph Hellwig, Tom Haynes
The protocol for creating a new state in nfsd is to allocate the state
leaving it largely uninitialised, add that state to the ->cl_stateids
idr so as to reserve a state-id, then complete initialisation of the
state and only set ->sc_type to non-zero once the state is fully
initialised.
If a state is found in the idr with ->sc_type == 0, it is ignored.
The ->cl_lock lock is used to avoid races - it is held while checking
sc_type during lookup, and held when a non-zero value is stored in
->sc_type.
... except... hash_delegation_locked() finalises the initialisation of a
delegation state, but does NOT hold ->cl_lock.
So this patch takes ->cl_lock at the appropriate time w.r.t other locks,
and so ensures there are no races (which are extremely unlikely in any
case).
As ->fi_lock is often taken when ->cl_lock is held, we need to take
->cl_lock first of those two.
Currently ->cl_lock and state_lock are never both taken at the same time.
We need both for this patch so an arbitrary choice is needed concerning
which to take first. As state_lock is more global, it might be more
contended, so take it first.
Reviewed-by: Jeff Layton <jlayton@kernel.org>
Signed-off-by: NeilBrown <neilb@suse.de>
---
fs/nfsd/nfs4state.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index d377a0a56e45..051c3e99fac6 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -1312,6 +1312,7 @@ hash_delegation_locked(struct nfs4_delegation *dp, struct nfs4_file *fp)
lockdep_assert_held(&state_lock);
lockdep_assert_held(&fp->fi_lock);
+ lockdep_assert_held(&clp->cl_lock);
if (nfs4_delegation_exists(clp, fp))
return -EAGAIN;
@@ -5557,12 +5558,14 @@ nfs4_set_delegation(struct nfsd4_open *open, struct nfs4_ol_stateid *stp,
goto out_unlock;
spin_lock(&state_lock);
+ spin_lock(&clp->cl_lock);
spin_lock(&fp->fi_lock);
if (fp->fi_had_conflict)
status = -EAGAIN;
else
status = hash_delegation_locked(dp, fp);
spin_unlock(&fp->fi_lock);
+ spin_unlock(&clp->cl_lock);
spin_unlock(&state_lock);
if (status)
--
2.43.0
^ permalink raw reply related [flat|nested] 27+ messages in thread* [PATCH 03/13] nfsd: don't call functions with side-effecting inside WARN_ON()
2024-01-29 3:29 [PATCH 00/13 v4] nfsd: support admin-revocation of v4 state NeilBrown
2024-01-29 3:29 ` [PATCH 01/13] nfsd: remove stale comment in nfs4_show_deleg() NeilBrown
2024-01-29 3:29 ` [PATCH 02/13] nfsd: hold ->cl_lock for hash_delegation_locked() NeilBrown
@ 2024-01-29 3:29 ` NeilBrown
2024-01-29 11:18 ` Jeff Layton
2024-01-29 3:29 ` [PATCH 04/13] nfsd: avoid race after unhash_delegation_locked() NeilBrown
` (9 subsequent siblings)
12 siblings, 1 reply; 27+ messages in thread
From: NeilBrown @ 2024-01-29 3:29 UTC (permalink / raw)
To: Chuck Lever, Jeff Layton
Cc: linux-nfs, Olga Kornievskaia, Dai Ngo, Tom Talpey,
Christoph Hellwig, Tom Haynes
Code like:
WARN_ON(foo())
looks like an assertion and might not be expected to have any side
effects.
When testing if a function with side-effects fails a construct like
if (foo())
WARN_ON(1);
makes the intent more obvious.
nfsd has several WARN_ON calls where the test has side effects, so it
would be good to change them. These cases don't really need the
WARN_ON. They have never failed in 8 years of usage so let's just
remove the WARN_ON wrapper.
Suggested-by: Chuck Lever <chuck.lever@oracle.com>
Signed-off-by: NeilBrown <neilb@suse.de>
---
fs/nfsd/nfs4state.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index 051c3e99fac6..2ddbb7b4a40e 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -1600,7 +1600,7 @@ static void release_open_stateid_locks(struct nfs4_ol_stateid *open_stp,
while (!list_empty(&open_stp->st_locks)) {
stp = list_entry(open_stp->st_locks.next,
struct nfs4_ol_stateid, st_locks);
- WARN_ON(!unhash_lock_stateid(stp));
+ unhash_lock_stateid(stp);
put_ol_stateid_locked(stp, reaplist);
}
}
@@ -2229,7 +2229,7 @@ __destroy_client(struct nfs4_client *clp)
spin_lock(&state_lock);
while (!list_empty(&clp->cl_delegations)) {
dp = list_entry(clp->cl_delegations.next, struct nfs4_delegation, dl_perclnt);
- WARN_ON(!unhash_delegation_locked(dp));
+ unhash_delegation_locked(dp);
list_add(&dp->dl_recall_lru, &reaplist);
}
spin_unlock(&state_lock);
@@ -6169,7 +6169,7 @@ nfs4_laundromat(struct nfsd_net *nn)
dp = list_entry (pos, struct nfs4_delegation, dl_recall_lru);
if (!state_expired(<, dp->dl_time))
break;
- WARN_ON(!unhash_delegation_locked(dp));
+ unhash_delegation_locked(dp);
list_add(&dp->dl_recall_lru, &reaplist);
}
spin_unlock(&state_lock);
@@ -7999,7 +7999,7 @@ nfsd4_release_lockowner(struct svc_rqst *rqstp,
stp = list_first_entry(&lo->lo_owner.so_stateids,
struct nfs4_ol_stateid,
st_perstateowner);
- WARN_ON(!unhash_lock_stateid(stp));
+ unhash_lock_stateid(stp);
put_ol_stateid_locked(stp, &reaplist);
}
spin_unlock(&clp->cl_lock);
@@ -8292,7 +8292,7 @@ nfs4_state_shutdown_net(struct net *net)
spin_lock(&state_lock);
list_for_each_safe(pos, next, &nn->del_recall_lru) {
dp = list_entry (pos, struct nfs4_delegation, dl_recall_lru);
- WARN_ON(!unhash_delegation_locked(dp));
+ unhash_delegation_locked(dp);
list_add(&dp->dl_recall_lru, &reaplist);
}
spin_unlock(&state_lock);
--
2.43.0
^ permalink raw reply related [flat|nested] 27+ messages in thread* Re: [PATCH 03/13] nfsd: don't call functions with side-effecting inside WARN_ON()
2024-01-29 3:29 ` [PATCH 03/13] nfsd: don't call functions with side-effecting inside WARN_ON() NeilBrown
@ 2024-01-29 11:18 ` Jeff Layton
0 siblings, 0 replies; 27+ messages in thread
From: Jeff Layton @ 2024-01-29 11:18 UTC (permalink / raw)
To: NeilBrown, Chuck Lever
Cc: linux-nfs, Olga Kornievskaia, Dai Ngo, Tom Talpey,
Christoph Hellwig, Tom Haynes
On Mon, 2024-01-29 at 14:29 +1100, NeilBrown wrote:
> Code like:
>
> WARN_ON(foo())
>
> looks like an assertion and might not be expected to have any side
> effects.
> When testing if a function with side-effects fails a construct like
>
> if (foo())
> WARN_ON(1);
>
> makes the intent more obvious.
>
> nfsd has several WARN_ON calls where the test has side effects, so it
> would be good to change them. These cases don't really need the
> WARN_ON. They have never failed in 8 years of usage so let's just
> remove the WARN_ON wrapper.
>
> Suggested-by: Chuck Lever <chuck.lever@oracle.com>
> Signed-off-by: NeilBrown <neilb@suse.de>
> ---
> fs/nfsd/nfs4state.c | 10 +++++-----
> 1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
> index 051c3e99fac6..2ddbb7b4a40e 100644
> --- a/fs/nfsd/nfs4state.c
> +++ b/fs/nfsd/nfs4state.c
> @@ -1600,7 +1600,7 @@ static void release_open_stateid_locks(struct nfs4_ol_stateid *open_stp,
> while (!list_empty(&open_stp->st_locks)) {
> stp = list_entry(open_stp->st_locks.next,
> struct nfs4_ol_stateid, st_locks);
> - WARN_ON(!unhash_lock_stateid(stp));
> + unhash_lock_stateid(stp);
> put_ol_stateid_locked(stp, reaplist);
> }
> }
> @@ -2229,7 +2229,7 @@ __destroy_client(struct nfs4_client *clp)
> spin_lock(&state_lock);
> while (!list_empty(&clp->cl_delegations)) {
> dp = list_entry(clp->cl_delegations.next, struct nfs4_delegation, dl_perclnt);
> - WARN_ON(!unhash_delegation_locked(dp));
> + unhash_delegation_locked(dp);
> list_add(&dp->dl_recall_lru, &reaplist);
> }
> spin_unlock(&state_lock);
> @@ -6169,7 +6169,7 @@ nfs4_laundromat(struct nfsd_net *nn)
> dp = list_entry (pos, struct nfs4_delegation, dl_recall_lru);
> if (!state_expired(<, dp->dl_time))
> break;
> - WARN_ON(!unhash_delegation_locked(dp));
> + unhash_delegation_locked(dp);
> list_add(&dp->dl_recall_lru, &reaplist);
> }
> spin_unlock(&state_lock);
> @@ -7999,7 +7999,7 @@ nfsd4_release_lockowner(struct svc_rqst *rqstp,
> stp = list_first_entry(&lo->lo_owner.so_stateids,
> struct nfs4_ol_stateid,
> st_perstateowner);
> - WARN_ON(!unhash_lock_stateid(stp));
> + unhash_lock_stateid(stp);
> put_ol_stateid_locked(stp, &reaplist);
> }
> spin_unlock(&clp->cl_lock);
> @@ -8292,7 +8292,7 @@ nfs4_state_shutdown_net(struct net *net)
> spin_lock(&state_lock);
> list_for_each_safe(pos, next, &nn->del_recall_lru) {
> dp = list_entry (pos, struct nfs4_delegation, dl_recall_lru);
> - WARN_ON(!unhash_delegation_locked(dp));
> + unhash_delegation_locked(dp);
> list_add(&dp->dl_recall_lru, &reaplist);
> }
> spin_unlock(&state_lock);
Reviewed-by: Jeff Layton <jlayton@kernel.org>
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH 04/13] nfsd: avoid race after unhash_delegation_locked()
2024-01-29 3:29 [PATCH 00/13 v4] nfsd: support admin-revocation of v4 state NeilBrown
` (2 preceding siblings ...)
2024-01-29 3:29 ` [PATCH 03/13] nfsd: don't call functions with side-effecting inside WARN_ON() NeilBrown
@ 2024-01-29 3:29 ` NeilBrown
2024-01-29 3:29 ` [PATCH 05/13] nfsd: split sc_status out of sc_type NeilBrown
` (8 subsequent siblings)
12 siblings, 0 replies; 27+ messages in thread
From: NeilBrown @ 2024-01-29 3:29 UTC (permalink / raw)
To: Chuck Lever, Jeff Layton
Cc: linux-nfs, Olga Kornievskaia, Dai Ngo, Tom Talpey,
Christoph Hellwig, Tom Haynes
NFS4_CLOSED_DELEG_STID and NFS4_REVOKED_DELEG_STID are similar in
purpose.
REVOKED is used for NFSv4.1 states which have been revoked because the
lease has expired. CLOSED is used in other cases.
The difference has two practical effects.
1/ REVOKED states are on the ->cl_revoked list
2/ REVOKED states result in nfserr_deleg_revoked from
nfsd4_verify_open_stid() and nfsd4_validate_stateid while
CLOSED states result in nfserr_bad_stid.
Currently a state that is being revoked is first set to "CLOSED" in
unhash_delegation_locked(), then possibly to "REVOKED" in
revoke_delegation(), at which point it is added to the cl_revoked list.
It is possible that a stateid test could see the CLOSED state
which really should be REVOKED, and so return the wrong error code. So
it is safest to remove this window of inconsistency.
With this patch, unhash_delegation_locked() always sets the state
correctly, and revoke_delegation() no longer changes the state.
Also remove a redundant test on minorversion when
NFS4_REVOKED_DELEG_STID is seen - it can only be seen when minorversion
is non-zero.
Reviewed-by: Jeff Layton <jlayton@kernel.org>
Signed-off-by: NeilBrown <neilb@suse.de>
---
fs/nfsd/nfs4state.c | 20 ++++++++++----------
1 file changed, 10 insertions(+), 10 deletions(-)
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index 2ddbb7b4a40e..dbf9ed84610e 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -1329,7 +1329,7 @@ static bool delegation_hashed(struct nfs4_delegation *dp)
}
static bool
-unhash_delegation_locked(struct nfs4_delegation *dp)
+unhash_delegation_locked(struct nfs4_delegation *dp, unsigned char type)
{
struct nfs4_file *fp = dp->dl_stid.sc_file;
@@ -1338,7 +1338,9 @@ unhash_delegation_locked(struct nfs4_delegation *dp)
if (!delegation_hashed(dp))
return false;
- dp->dl_stid.sc_type = NFS4_CLOSED_DELEG_STID;
+ if (dp->dl_stid.sc_client->cl_minorversion == 0)
+ type = NFS4_CLOSED_DELEG_STID;
+ dp->dl_stid.sc_type = type;
/* Ensure that deleg break won't try to requeue it */
++dp->dl_time;
spin_lock(&fp->fi_lock);
@@ -1354,7 +1356,7 @@ static void destroy_delegation(struct nfs4_delegation *dp)
bool unhashed;
spin_lock(&state_lock);
- unhashed = unhash_delegation_locked(dp);
+ unhashed = unhash_delegation_locked(dp, NFS4_CLOSED_DELEG_STID);
spin_unlock(&state_lock);
if (unhashed)
destroy_unhashed_deleg(dp);
@@ -1368,9 +1370,8 @@ static void revoke_delegation(struct nfs4_delegation *dp)
trace_nfsd_stid_revoke(&dp->dl_stid);
- if (clp->cl_minorversion) {
+ if (dp->dl_stid.sc_type == NFS4_REVOKED_DELEG_STID) {
spin_lock(&clp->cl_lock);
- dp->dl_stid.sc_type = NFS4_REVOKED_DELEG_STID;
refcount_inc(&dp->dl_stid.sc_count);
list_add(&dp->dl_recall_lru, &clp->cl_revoked);
spin_unlock(&clp->cl_lock);
@@ -2229,7 +2230,7 @@ __destroy_client(struct nfs4_client *clp)
spin_lock(&state_lock);
while (!list_empty(&clp->cl_delegations)) {
dp = list_entry(clp->cl_delegations.next, struct nfs4_delegation, dl_perclnt);
- unhash_delegation_locked(dp);
+ unhash_delegation_locked(dp, NFS4_CLOSED_DELEG_STID);
list_add(&dp->dl_recall_lru, &reaplist);
}
spin_unlock(&state_lock);
@@ -5145,8 +5146,7 @@ nfs4_check_deleg(struct nfs4_client *cl, struct nfsd4_open *open,
goto out;
if (deleg->dl_stid.sc_type == NFS4_REVOKED_DELEG_STID) {
nfs4_put_stid(&deleg->dl_stid);
- if (cl->cl_minorversion)
- status = nfserr_deleg_revoked;
+ status = nfserr_deleg_revoked;
goto out;
}
flags = share_access_to_flags(open->op_share_access);
@@ -6169,7 +6169,7 @@ nfs4_laundromat(struct nfsd_net *nn)
dp = list_entry (pos, struct nfs4_delegation, dl_recall_lru);
if (!state_expired(<, dp->dl_time))
break;
- unhash_delegation_locked(dp);
+ unhash_delegation_locked(dp, NFS4_REVOKED_DELEG_STID);
list_add(&dp->dl_recall_lru, &reaplist);
}
spin_unlock(&state_lock);
@@ -8292,7 +8292,7 @@ nfs4_state_shutdown_net(struct net *net)
spin_lock(&state_lock);
list_for_each_safe(pos, next, &nn->del_recall_lru) {
dp = list_entry (pos, struct nfs4_delegation, dl_recall_lru);
- unhash_delegation_locked(dp);
+ unhash_delegation_locked(dp, NFS4_CLOSED_DELEG_STID);
list_add(&dp->dl_recall_lru, &reaplist);
}
spin_unlock(&state_lock);
--
2.43.0
^ permalink raw reply related [flat|nested] 27+ messages in thread* [PATCH 05/13] nfsd: split sc_status out of sc_type
2024-01-29 3:29 [PATCH 00/13 v4] nfsd: support admin-revocation of v4 state NeilBrown
` (3 preceding siblings ...)
2024-01-29 3:29 ` [PATCH 04/13] nfsd: avoid race after unhash_delegation_locked() NeilBrown
@ 2024-01-29 3:29 ` NeilBrown
2024-01-29 12:21 ` Jeff Layton
2024-01-29 14:04 ` Chuck Lever
2024-01-29 3:29 ` [PATCH 06/13] nfsd: prepare for supporting admin-revocation of state NeilBrown
` (7 subsequent siblings)
12 siblings, 2 replies; 27+ messages in thread
From: NeilBrown @ 2024-01-29 3:29 UTC (permalink / raw)
To: Chuck Lever, Jeff Layton
Cc: linux-nfs, Olga Kornievskaia, Dai Ngo, Tom Talpey,
Christoph Hellwig, Tom Haynes
sc_type identifies the type of a state - open, lock, deleg, layout - and
also the status of a state - closed or revoked.
This is a bit untidy and could get worse when "admin-revoked" states are
added. So clean it up.
With this patch, the type is now all that is stored in sc_type. This is
zero when the state is first added to ->cl_stateids (causing it to be
ignored), and is then set appropriately once it is fully initialised.
It is set under ->cl_lock to ensure atomicity w.r.t lookup. It is now
never cleared.
sc_type is still a bit-set even though at most one bit is set. This allows
lookup functions to be given a bitmap of acceptable types.
sc_type is now an unsigned short rather than char. There is no value in
restricting to just 8 bits.
All the constants now start SC_TYPE_ matching the field in which they
are stored. Keeping the existing names and ensuring clear separation
from non-type flags would have required something like
NFS4_STID_TYPE_CLOSED which is cumbersome. The "NFS4" prefix is
redundant was they only appear in NFS4 code, so remove that and change
STID to SC to match the field.
The status is stored in a separate unsigned short named "sc_status". It
has two flags: SC_STATUS_CLOSED and SC_STATUS_REVOKED.
CLOSED combines NFS4_CLOSED_STID, NFS4_CLOSED_DELEG_STID, and is used
for SC_TYPE_LOCK and SC_TYPE_LAYOUT instead of setting the sc_type to zero.
These flags are only ever set, never cleared.
For deleg stateids they are set under the global state_lock.
For open and lock stateids they are set under ->cl_lock.
For layout stateids they are set under ->ls_lock
nfs4_unhash_stid() has been removed, and we never set sc_type = 0. This
was only used for LOCK and LAYOUT stids and they now use
SC_STATUS_CLOSED.
Also TRACE_DEFINE_NUM() calls for the various STID #define have been
removed because these things are not enums, and so that call is
incorrect.
Signed-off-by: NeilBrown <neilb@suse.de>
---
fs/nfsd/nfs4layouts.c | 14 +--
fs/nfsd/nfs4state.c | 207 +++++++++++++++++++++---------------------
fs/nfsd/state.h | 40 +++++---
fs/nfsd/trace.h | 31 +++----
4 files changed, 151 insertions(+), 141 deletions(-)
diff --git a/fs/nfsd/nfs4layouts.c b/fs/nfsd/nfs4layouts.c
index 5e8096bc5eaa..857b822450b4 100644
--- a/fs/nfsd/nfs4layouts.c
+++ b/fs/nfsd/nfs4layouts.c
@@ -236,7 +236,7 @@ nfsd4_alloc_layout_stateid(struct nfsd4_compound_state *cstate,
nfsd4_init_cb(&ls->ls_recall, clp, &nfsd4_cb_layout_ops,
NFSPROC4_CLNT_CB_LAYOUT);
- if (parent->sc_type == NFS4_DELEG_STID)
+ if (parent->sc_type == SC_TYPE_DELEG)
ls->ls_file = nfsd_file_get(fp->fi_deleg_file);
else
ls->ls_file = find_any_file(fp);
@@ -250,7 +250,7 @@ nfsd4_alloc_layout_stateid(struct nfsd4_compound_state *cstate,
}
spin_lock(&clp->cl_lock);
- stp->sc_type = NFS4_LAYOUT_STID;
+ stp->sc_type = SC_TYPE_LAYOUT;
list_add(&ls->ls_perclnt, &clp->cl_lo_states);
spin_unlock(&clp->cl_lock);
@@ -269,13 +269,13 @@ nfsd4_preprocess_layout_stateid(struct svc_rqst *rqstp,
{
struct nfs4_layout_stateid *ls;
struct nfs4_stid *stid;
- unsigned char typemask = NFS4_LAYOUT_STID;
+ unsigned short typemask = SC_TYPE_LAYOUT;
__be32 status;
if (create)
- typemask |= (NFS4_OPEN_STID | NFS4_LOCK_STID | NFS4_DELEG_STID);
+ typemask |= (SC_TYPE_OPEN | SC_TYPE_LOCK | SC_TYPE_DELEG);
- status = nfsd4_lookup_stateid(cstate, stateid, typemask, &stid,
+ status = nfsd4_lookup_stateid(cstate, stateid, typemask, 0, &stid,
net_generic(SVC_NET(rqstp), nfsd_net_id));
if (status)
goto out;
@@ -286,7 +286,7 @@ nfsd4_preprocess_layout_stateid(struct svc_rqst *rqstp,
goto out_put_stid;
}
- if (stid->sc_type != NFS4_LAYOUT_STID) {
+ if (stid->sc_type != SC_TYPE_LAYOUT) {
ls = nfsd4_alloc_layout_stateid(cstate, stid, layout_type);
nfs4_put_stid(stid);
@@ -518,7 +518,7 @@ nfsd4_return_file_layouts(struct svc_rqst *rqstp,
lrp->lrs_present = true;
} else {
trace_nfsd_layoutstate_unhash(&ls->ls_stid.sc_stateid);
- nfs4_unhash_stid(&ls->ls_stid);
+ ls->ls_stid.sc_status |= SC_STATUS_CLOSED;
lrp->lrs_present = false;
}
spin_unlock(&ls->ls_lock);
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index dbf9ed84610e..6bccdd0af814 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -1260,11 +1260,6 @@ static void destroy_unhashed_deleg(struct nfs4_delegation *dp)
nfs4_put_stid(&dp->dl_stid);
}
-void nfs4_unhash_stid(struct nfs4_stid *s)
-{
- s->sc_type = 0;
-}
-
/**
* nfs4_delegation_exists - Discover if this delegation already exists
* @clp: a pointer to the nfs4_client we're granting a delegation to
@@ -1317,7 +1312,7 @@ hash_delegation_locked(struct nfs4_delegation *dp, struct nfs4_file *fp)
if (nfs4_delegation_exists(clp, fp))
return -EAGAIN;
refcount_inc(&dp->dl_stid.sc_count);
- dp->dl_stid.sc_type = NFS4_DELEG_STID;
+ dp->dl_stid.sc_type = SC_TYPE_DELEG;
list_add(&dp->dl_perfile, &fp->fi_delegations);
list_add(&dp->dl_perclnt, &clp->cl_delegations);
return 0;
@@ -1329,7 +1324,7 @@ static bool delegation_hashed(struct nfs4_delegation *dp)
}
static bool
-unhash_delegation_locked(struct nfs4_delegation *dp, unsigned char type)
+unhash_delegation_locked(struct nfs4_delegation *dp, unsigned short statusmask)
{
struct nfs4_file *fp = dp->dl_stid.sc_file;
@@ -1339,8 +1334,9 @@ unhash_delegation_locked(struct nfs4_delegation *dp, unsigned char type)
return false;
if (dp->dl_stid.sc_client->cl_minorversion == 0)
- type = NFS4_CLOSED_DELEG_STID;
- dp->dl_stid.sc_type = type;
+ statusmask = SC_STATUS_CLOSED;
+ dp->dl_stid.sc_status |= statusmask;
+
/* Ensure that deleg break won't try to requeue it */
++dp->dl_time;
spin_lock(&fp->fi_lock);
@@ -1356,7 +1352,7 @@ static void destroy_delegation(struct nfs4_delegation *dp)
bool unhashed;
spin_lock(&state_lock);
- unhashed = unhash_delegation_locked(dp, NFS4_CLOSED_DELEG_STID);
+ unhashed = unhash_delegation_locked(dp, SC_STATUS_CLOSED);
spin_unlock(&state_lock);
if (unhashed)
destroy_unhashed_deleg(dp);
@@ -1370,7 +1366,7 @@ static void revoke_delegation(struct nfs4_delegation *dp)
trace_nfsd_stid_revoke(&dp->dl_stid);
- if (dp->dl_stid.sc_type == NFS4_REVOKED_DELEG_STID) {
+ if (dp->dl_stid.sc_status & SC_STATUS_REVOKED) {
spin_lock(&clp->cl_lock);
refcount_inc(&dp->dl_stid.sc_count);
list_add(&dp->dl_recall_lru, &clp->cl_revoked);
@@ -1379,8 +1375,8 @@ static void revoke_delegation(struct nfs4_delegation *dp)
destroy_unhashed_deleg(dp);
}
-/*
- * SETCLIENTID state
+/*
+ * SETCLIENTID state
*/
static unsigned int clientid_hashval(u32 id)
@@ -1543,7 +1539,7 @@ static bool unhash_lock_stateid(struct nfs4_ol_stateid *stp)
if (!unhash_ol_stateid(stp))
return false;
list_del_init(&stp->st_locks);
- nfs4_unhash_stid(&stp->st_stid);
+ stp->st_stid.sc_status |= SC_STATUS_CLOSED;
return true;
}
@@ -1622,6 +1618,7 @@ static void release_open_stateid(struct nfs4_ol_stateid *stp)
LIST_HEAD(reaplist);
spin_lock(&stp->st_stid.sc_client->cl_lock);
+ stp->st_stid.sc_status |= SC_STATUS_CLOSED;
if (unhash_open_stateid(stp, &reaplist))
put_ol_stateid_locked(stp, &reaplist);
spin_unlock(&stp->st_stid.sc_client->cl_lock);
@@ -2230,7 +2227,7 @@ __destroy_client(struct nfs4_client *clp)
spin_lock(&state_lock);
while (!list_empty(&clp->cl_delegations)) {
dp = list_entry(clp->cl_delegations.next, struct nfs4_delegation, dl_perclnt);
- unhash_delegation_locked(dp, NFS4_CLOSED_DELEG_STID);
+ unhash_delegation_locked(dp, SC_STATUS_CLOSED);
list_add(&dp->dl_recall_lru, &reaplist);
}
spin_unlock(&state_lock);
@@ -2462,14 +2459,16 @@ find_stateid_locked(struct nfs4_client *cl, stateid_t *t)
}
static struct nfs4_stid *
-find_stateid_by_type(struct nfs4_client *cl, stateid_t *t, char typemask)
+find_stateid_by_type(struct nfs4_client *cl, stateid_t *t,
+ unsigned short typemask, unsigned short ok_states)
{
struct nfs4_stid *s;
spin_lock(&cl->cl_lock);
s = find_stateid_locked(cl, t);
if (s != NULL) {
- if (typemask & s->sc_type)
+ if ((s->sc_status & ~ok_states) == 0 &&
+ (typemask & s->sc_type))
refcount_inc(&s->sc_count);
else
s = NULL;
@@ -2622,7 +2621,7 @@ 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)
+ if (st->sc_type != SC_TYPE_OPEN && st->sc_type != SC_TYPE_LOCK)
return 0; /* XXX: or SEQ_SKIP? */
ols = openlockstateid(st);
oo = ols->st_stateowner;
@@ -2754,13 +2753,13 @@ static int states_show(struct seq_file *s, void *v)
struct nfs4_stid *st = v;
switch (st->sc_type) {
- case NFS4_OPEN_STID:
+ case SC_TYPE_OPEN:
return nfs4_show_open(s, st);
- case NFS4_LOCK_STID:
+ case SC_TYPE_LOCK:
return nfs4_show_lock(s, st);
- case NFS4_DELEG_STID:
+ case SC_TYPE_DELEG:
return nfs4_show_deleg(s, st);
- case NFS4_LAYOUT_STID:
+ case SC_TYPE_LAYOUT:
return nfs4_show_layout(s, st);
default:
return 0; /* XXX: or SEQ_SKIP? */
@@ -4532,7 +4531,8 @@ nfsd4_find_existing_open(struct nfs4_file *fp, struct nfsd4_open *open)
continue;
if (local->st_stateowner != &oo->oo_owner)
continue;
- if (local->st_stid.sc_type == NFS4_OPEN_STID) {
+ if (local->st_stid.sc_type == SC_TYPE_OPEN &&
+ !local->st_stid.sc_status) {
ret = local;
refcount_inc(&ret->st_stid.sc_count);
break;
@@ -4546,17 +4546,10 @@ nfsd4_verify_open_stid(struct nfs4_stid *s)
{
__be32 ret = nfs_ok;
- switch (s->sc_type) {
- default:
- break;
- case 0:
- case NFS4_CLOSED_STID:
- case NFS4_CLOSED_DELEG_STID:
- ret = nfserr_bad_stateid;
- break;
- case NFS4_REVOKED_DELEG_STID:
+ if (s->sc_status & SC_STATUS_REVOKED)
ret = nfserr_deleg_revoked;
- }
+ else if (s->sc_status & SC_STATUS_CLOSED)
+ ret = nfserr_bad_stateid;
return ret;
}
@@ -4642,7 +4635,7 @@ init_open_stateid(struct nfs4_file *fp, struct nfsd4_open *open)
open->op_stp = NULL;
refcount_inc(&stp->st_stid.sc_count);
- stp->st_stid.sc_type = NFS4_OPEN_STID;
+ stp->st_stid.sc_type = SC_TYPE_OPEN;
INIT_LIST_HEAD(&stp->st_locks);
stp->st_stateowner = nfs4_get_stateowner(&oo->oo_owner);
get_nfs4_file(fp);
@@ -4869,9 +4862,9 @@ static int nfsd4_cb_recall_done(struct nfsd4_callback *cb,
trace_nfsd_cb_recall_done(&dp->dl_stid.sc_stateid, task);
- if (dp->dl_stid.sc_type == NFS4_CLOSED_DELEG_STID ||
- dp->dl_stid.sc_type == NFS4_REVOKED_DELEG_STID)
- return 1;
+ if (dp->dl_stid.sc_status)
+ /* CLOSED or REVOKED */
+ return 1;
switch (task->tk_status) {
case 0:
@@ -5116,12 +5109,12 @@ static int share_access_to_flags(u32 share_access)
return share_access == NFS4_SHARE_ACCESS_READ ? RD_STATE : WR_STATE;
}
-static struct nfs4_delegation *find_deleg_stateid(struct nfs4_client *cl, stateid_t *s)
+static struct nfs4_delegation *find_deleg_stateid(struct nfs4_client *cl,
+ stateid_t *s)
{
struct nfs4_stid *ret;
- ret = find_stateid_by_type(cl, s,
- NFS4_DELEG_STID|NFS4_REVOKED_DELEG_STID);
+ ret = find_stateid_by_type(cl, s, SC_TYPE_DELEG, SC_STATUS_REVOKED);
if (!ret)
return NULL;
return delegstateid(ret);
@@ -5144,7 +5137,7 @@ 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_type == NFS4_REVOKED_DELEG_STID) {
+ if (deleg->dl_stid.sc_status & SC_STATUS_REVOKED) {
nfs4_put_stid(&deleg->dl_stid);
status = nfserr_deleg_revoked;
goto out;
@@ -5777,7 +5770,6 @@ nfsd4_process_open2(struct svc_rqst *rqstp, struct svc_fh *current_fh, struct nf
} else {
status = nfs4_get_vfs_file(rqstp, fp, current_fh, stp, open, true);
if (status) {
- stp->st_stid.sc_type = NFS4_CLOSED_STID;
release_open_stateid(stp);
mutex_unlock(&stp->st_mutex);
goto out;
@@ -6169,7 +6161,7 @@ nfs4_laundromat(struct nfsd_net *nn)
dp = list_entry (pos, struct nfs4_delegation, dl_recall_lru);
if (!state_expired(<, dp->dl_time))
break;
- unhash_delegation_locked(dp, NFS4_REVOKED_DELEG_STID);
+ unhash_delegation_locked(dp, SC_STATUS_REVOKED);
list_add(&dp->dl_recall_lru, &reaplist);
}
spin_unlock(&state_lock);
@@ -6408,22 +6400,20 @@ static __be32 nfsd4_validate_stateid(struct nfs4_client *cl, stateid_t *stateid)
status = nfsd4_stid_check_stateid_generation(stateid, s, 1);
if (status)
goto out_unlock;
+ status = nfsd4_verify_open_stid(s);
+ if (status)
+ goto out_unlock;
+
switch (s->sc_type) {
- case NFS4_DELEG_STID:
+ case SC_TYPE_DELEG:
status = nfs_ok;
break;
- case NFS4_REVOKED_DELEG_STID:
- status = nfserr_deleg_revoked;
- break;
- case NFS4_OPEN_STID:
- case NFS4_LOCK_STID:
+ case SC_TYPE_OPEN:
+ case SC_TYPE_LOCK:
status = nfsd4_check_openowner_confirmed(openlockstateid(s));
break;
default:
printk("unknown stateid type %x\n", s->sc_type);
- fallthrough;
- case NFS4_CLOSED_STID:
- case NFS4_CLOSED_DELEG_STID:
status = nfserr_bad_stateid;
}
out_unlock:
@@ -6433,7 +6423,8 @@ static __be32 nfsd4_validate_stateid(struct nfs4_client *cl, stateid_t *stateid)
__be32
nfsd4_lookup_stateid(struct nfsd4_compound_state *cstate,
- stateid_t *stateid, unsigned char typemask,
+ stateid_t *stateid,
+ unsigned short typemask, unsigned short statusmask,
struct nfs4_stid **s, struct nfsd_net *nn)
{
__be32 status;
@@ -6444,10 +6435,13 @@ nfsd4_lookup_stateid(struct nfsd4_compound_state *cstate,
* only return revoked delegations if explicitly asked.
* otherwise we report revoked or bad_stateid status.
*/
- if (typemask & NFS4_REVOKED_DELEG_STID)
+ if (statusmask & SC_STATUS_REVOKED)
return_revoked = true;
- else if (typemask & NFS4_DELEG_STID)
- typemask |= NFS4_REVOKED_DELEG_STID;
+ if (typemask & SC_TYPE_DELEG)
+ /* Always allow REVOKED for DELEG so we can
+ * retturn the appropriate error.
+ */
+ statusmask |= SC_STATUS_REVOKED;
if (ZERO_STATEID(stateid) || ONE_STATEID(stateid) ||
CLOSE_STATEID(stateid))
@@ -6460,14 +6454,12 @@ nfsd4_lookup_stateid(struct nfsd4_compound_state *cstate,
}
if (status)
return status;
- stid = find_stateid_by_type(cstate->clp, stateid, typemask);
+ stid = find_stateid_by_type(cstate->clp, stateid, typemask, statusmask);
if (!stid)
return nfserr_bad_stateid;
- if ((stid->sc_type == NFS4_REVOKED_DELEG_STID) && !return_revoked) {
+ if ((stid->sc_status & SC_STATUS_REVOKED) && !return_revoked) {
nfs4_put_stid(stid);
- if (cstate->minorversion)
- return nfserr_deleg_revoked;
- return nfserr_bad_stateid;
+ return nfserr_deleg_revoked;
}
*s = stid;
return nfs_ok;
@@ -6478,17 +6470,17 @@ nfs4_find_file(struct nfs4_stid *s, int flags)
{
struct nfsd_file *ret = NULL;
- if (!s)
+ if (!s || s->sc_status)
return NULL;
switch (s->sc_type) {
- case NFS4_DELEG_STID:
+ case SC_TYPE_DELEG:
spin_lock(&s->sc_file->fi_lock);
ret = nfsd_file_get(s->sc_file->fi_deleg_file);
spin_unlock(&s->sc_file->fi_lock);
break;
- case NFS4_OPEN_STID:
- case NFS4_LOCK_STID:
+ case SC_TYPE_OPEN:
+ case SC_TYPE_LOCK:
if (flags & RD_STATE)
ret = find_readable_file(s->sc_file);
else
@@ -6601,7 +6593,8 @@ static __be32 find_cpntf_state(struct nfsd_net *nn, stateid_t *st,
goto out;
*stid = find_stateid_by_type(found, &cps->cp_p_stateid,
- NFS4_DELEG_STID|NFS4_OPEN_STID|NFS4_LOCK_STID);
+ SC_TYPE_DELEG|SC_TYPE_OPEN|SC_TYPE_LOCK,
+ 0);
if (*stid)
status = nfs_ok;
else
@@ -6658,8 +6651,8 @@ nfs4_preprocess_stateid_op(struct svc_rqst *rqstp,
}
status = nfsd4_lookup_stateid(cstate, stateid,
- NFS4_DELEG_STID|NFS4_OPEN_STID|NFS4_LOCK_STID,
- &s, nn);
+ SC_TYPE_DELEG|SC_TYPE_OPEN|SC_TYPE_LOCK,
+ 0, &s, nn);
if (status == nfserr_bad_stateid)
status = find_cpntf_state(nn, stateid, &s);
if (status)
@@ -6670,16 +6663,13 @@ nfs4_preprocess_stateid_op(struct svc_rqst *rqstp,
goto out;
switch (s->sc_type) {
- case NFS4_DELEG_STID:
+ case SC_TYPE_DELEG:
status = nfs4_check_delegmode(delegstateid(s), flags);
break;
- case NFS4_OPEN_STID:
- case NFS4_LOCK_STID:
+ case SC_TYPE_OPEN:
+ case SC_TYPE_LOCK:
status = nfs4_check_olstateid(openlockstateid(s), flags);
break;
- default:
- status = nfserr_bad_stateid;
- break;
}
if (status)
goto out;
@@ -6758,33 +6748,34 @@ nfsd4_free_stateid(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
spin_lock(&cl->cl_lock);
s = find_stateid_locked(cl, stateid);
- if (!s)
+ if (!s || s->sc_status & SC_STATUS_CLOSED)
goto out_unlock;
spin_lock(&s->sc_lock);
switch (s->sc_type) {
- case NFS4_DELEG_STID:
+ case SC_TYPE_DELEG:
+ if (s->sc_status & SC_STATUS_REVOKED) {
+ spin_unlock(&s->sc_lock);
+ dp = delegstateid(s);
+ list_del_init(&dp->dl_recall_lru);
+ spin_unlock(&cl->cl_lock);
+ nfs4_put_stid(s);
+ ret = nfs_ok;
+ goto out;
+ }
ret = nfserr_locks_held;
break;
- case NFS4_OPEN_STID:
+ case SC_TYPE_OPEN:
ret = check_stateid_generation(stateid, &s->sc_stateid, 1);
if (ret)
break;
ret = nfserr_locks_held;
break;
- case NFS4_LOCK_STID:
+ case SC_TYPE_LOCK:
spin_unlock(&s->sc_lock);
refcount_inc(&s->sc_count);
spin_unlock(&cl->cl_lock);
ret = nfsd4_free_lock_stateid(stateid, s);
goto out;
- case NFS4_REVOKED_DELEG_STID:
- spin_unlock(&s->sc_lock);
- dp = delegstateid(s);
- list_del_init(&dp->dl_recall_lru);
- spin_unlock(&cl->cl_lock);
- nfs4_put_stid(s);
- ret = nfs_ok;
- goto out;
/* Default falls through and returns nfserr_bad_stateid */
}
spin_unlock(&s->sc_lock);
@@ -6827,6 +6818,7 @@ static __be32 nfs4_seqid_op_checks(struct nfsd4_compound_state *cstate, stateid_
* @seqid: seqid (provided by client)
* @stateid: stateid (provided by client)
* @typemask: mask of allowable types for this operation
+ * @statusmask: mask of allowed states: 0 or STID_CLOSED
* @stpp: return pointer for the stateid found
* @nn: net namespace for request
*
@@ -6836,7 +6828,8 @@ static __be32 nfs4_seqid_op_checks(struct nfsd4_compound_state *cstate, stateid_
*/
static __be32
nfs4_preprocess_seqid_op(struct nfsd4_compound_state *cstate, u32 seqid,
- stateid_t *stateid, char typemask,
+ stateid_t *stateid,
+ unsigned short typemask, unsigned short statusmask,
struct nfs4_ol_stateid **stpp,
struct nfsd_net *nn)
{
@@ -6847,7 +6840,8 @@ nfs4_preprocess_seqid_op(struct nfsd4_compound_state *cstate, u32 seqid,
trace_nfsd_preprocess(seqid, stateid);
*stpp = NULL;
- status = nfsd4_lookup_stateid(cstate, stateid, typemask, &s, nn);
+ status = nfsd4_lookup_stateid(cstate, stateid,
+ typemask, statusmask, &s, nn);
if (status)
return status;
stp = openlockstateid(s);
@@ -6869,7 +6863,7 @@ static __be32 nfs4_preprocess_confirmed_seqid_op(struct nfsd4_compound_state *cs
struct nfs4_ol_stateid *stp;
status = nfs4_preprocess_seqid_op(cstate, seqid, stateid,
- NFS4_OPEN_STID, &stp, nn);
+ SC_TYPE_OPEN, 0, &stp, nn);
if (status)
return status;
oo = openowner(stp->st_stateowner);
@@ -6900,8 +6894,8 @@ nfsd4_open_confirm(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
return status;
status = nfs4_preprocess_seqid_op(cstate,
- oc->oc_seqid, &oc->oc_req_stateid,
- NFS4_OPEN_STID, &stp, nn);
+ oc->oc_seqid, &oc->oc_req_stateid,
+ SC_TYPE_OPEN, 0, &stp, nn);
if (status)
goto out;
oo = openowner(stp->st_stateowner);
@@ -7031,18 +7025,20 @@ nfsd4_close(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
struct net *net = SVC_NET(rqstp);
struct nfsd_net *nn = net_generic(net, nfsd_net_id);
- dprintk("NFSD: nfsd4_close on file %pd\n",
+ dprintk("NFSD: nfsd4_close on file %pd\n",
cstate->current_fh.fh_dentry);
status = nfs4_preprocess_seqid_op(cstate, close->cl_seqid,
- &close->cl_stateid,
- NFS4_OPEN_STID|NFS4_CLOSED_STID,
- &stp, nn);
+ &close->cl_stateid,
+ SC_TYPE_OPEN, SC_STATUS_CLOSED,
+ &stp, nn);
nfsd4_bump_seqid(cstate, status);
if (status)
- goto out;
+ goto out;
- stp->st_stid.sc_type = NFS4_CLOSED_STID;
+ spin_lock(&stp->st_stid.sc_client->cl_lock);
+ stp->st_stid.sc_status |= SC_STATUS_CLOSED;
+ spin_unlock(&stp->st_stid.sc_client->cl_lock);
/*
* Technically we don't _really_ have to increment or copy it, since
@@ -7084,7 +7080,7 @@ nfsd4_delegreturn(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
if ((status = fh_verify(rqstp, &cstate->current_fh, S_IFREG, 0)))
return status;
- status = nfsd4_lookup_stateid(cstate, stateid, NFS4_DELEG_STID, &s, nn);
+ status = nfsd4_lookup_stateid(cstate, stateid, SC_TYPE_DELEG, 0, &s, nn);
if (status)
goto out;
dp = delegstateid(s);
@@ -7351,7 +7347,7 @@ init_lock_stateid(struct nfs4_ol_stateid *stp, struct nfs4_lockowner *lo,
if (retstp)
goto out_found;
refcount_inc(&stp->st_stid.sc_count);
- stp->st_stid.sc_type = NFS4_LOCK_STID;
+ stp->st_stid.sc_type = SC_TYPE_LOCK;
stp->st_stateowner = nfs4_get_stateowner(&lo->lo_owner);
get_nfs4_file(fp);
stp->st_stid.sc_file = fp;
@@ -7538,9 +7534,10 @@ nfsd4_lock(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
&lock_stp, &new);
} else {
status = nfs4_preprocess_seqid_op(cstate,
- lock->lk_old_lock_seqid,
- &lock->lk_old_lock_stateid,
- NFS4_LOCK_STID, &lock_stp, nn);
+ lock->lk_old_lock_seqid,
+ &lock->lk_old_lock_stateid,
+ SC_TYPE_LOCK, 0, &lock_stp,
+ nn);
}
if (status)
goto out;
@@ -7853,8 +7850,8 @@ nfsd4_locku(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
return nfserr_inval;
status = nfs4_preprocess_seqid_op(cstate, locku->lu_seqid,
- &locku->lu_stateid, NFS4_LOCK_STID,
- &stp, nn);
+ &locku->lu_stateid, SC_TYPE_LOCK, 0,
+ &stp, nn);
if (status)
goto out;
nf = find_any_file(stp->st_stid.sc_file);
@@ -8292,7 +8289,7 @@ nfs4_state_shutdown_net(struct net *net)
spin_lock(&state_lock);
list_for_each_safe(pos, next, &nn->del_recall_lru) {
dp = list_entry (pos, struct nfs4_delegation, dl_recall_lru);
- unhash_delegation_locked(dp, NFS4_CLOSED_DELEG_STID);
+ unhash_delegation_locked(dp, SC_STATUS_CLOSED);
list_add(&dp->dl_recall_lru, &reaplist);
}
spin_unlock(&state_lock);
diff --git a/fs/nfsd/state.h b/fs/nfsd/state.h
index 41bdc913fa71..ffc8920d0558 100644
--- a/fs/nfsd/state.h
+++ b/fs/nfsd/state.h
@@ -88,17 +88,33 @@ struct nfsd4_callback_ops {
*/
struct nfs4_stid {
refcount_t sc_count;
-#define NFS4_OPEN_STID 1
-#define NFS4_LOCK_STID 2
-#define NFS4_DELEG_STID 4
-/* For an open stateid kept around *only* to process close replays: */
-#define NFS4_CLOSED_STID 8
+
+ /* A new stateid is added to the cl_stateids idr early before it
+ * is fully initialised. Its sc_type is then zero. After
+ * initialisation the sc_type it set under cl_lock, and then
+ * never changes.
+ */
+#define SC_TYPE_OPEN BIT(0)
+#define SC_TYPE_LOCK BIT(1)
+#define SC_TYPE_DELEG BIT(2)
+#define SC_TYPE_LAYOUT BIT(3)
+ unsigned short sc_type;
+
+/* state_lock protects sc_status for delegation stateids.
+ * ->cl_lock protects sc_status for open and lock stateids.
+ * ->st_mutex also protect sc_status for open stateids.
+ * ->ls_lock protects sc_status for layout stateids.
+ */
+/*
+ * For an open stateid kept around *only* to process close replays.
+ * For deleg stateid, kept in idr until last reference is dropped.
+ */
+#define SC_STATUS_CLOSED BIT(0)
/* For a deleg stateid kept around only to process free_stateid's: */
-#define NFS4_REVOKED_DELEG_STID 16
-#define NFS4_CLOSED_DELEG_STID 32
-#define NFS4_LAYOUT_STID 64
+#define SC_STATUS_REVOKED BIT(1)
+ unsigned short sc_status;
+
struct list_head sc_cp_list;
- unsigned char sc_type;
stateid_t sc_stateid;
spinlock_t sc_lock;
struct nfs4_client *sc_client;
@@ -672,15 +688,15 @@ extern __be32 nfs4_preprocess_stateid_op(struct svc_rqst *rqstp,
stateid_t *stateid, int flags, struct nfsd_file **filp,
struct nfs4_stid **cstid);
__be32 nfsd4_lookup_stateid(struct nfsd4_compound_state *cstate,
- stateid_t *stateid, unsigned char typemask,
- struct nfs4_stid **s, struct nfsd_net *nn);
+ stateid_t *stateid, unsigned short typemask,
+ unsigned short statusmask,
+ struct nfs4_stid **s, struct nfsd_net *nn);
struct nfs4_stid *nfs4_alloc_stid(struct nfs4_client *cl, struct kmem_cache *slab,
void (*sc_free)(struct nfs4_stid *));
int nfs4_init_copy_state(struct nfsd_net *nn, struct nfsd4_copy *copy);
void nfs4_free_copy_state(struct nfsd4_copy *copy);
struct nfs4_cpntf_state *nfs4_alloc_init_cpntf_state(struct nfsd_net *nn,
struct nfs4_stid *p_stid);
-void nfs4_unhash_stid(struct nfs4_stid *s);
void nfs4_put_stid(struct nfs4_stid *s);
void nfs4_inc_and_copy_stateid(stateid_t *dst, struct nfs4_stid *stid);
void nfs4_remove_reclaim_record(struct nfs4_client_reclaim *, struct nfsd_net *);
diff --git a/fs/nfsd/trace.h b/fs/nfsd/trace.h
index d1e8cf079b0f..fe08ca18b647 100644
--- a/fs/nfsd/trace.h
+++ b/fs/nfsd/trace.h
@@ -641,23 +641,17 @@ DEFINE_EVENT(nfsd_stateseqid_class, nfsd_##name, \
DEFINE_STATESEQID_EVENT(preprocess);
DEFINE_STATESEQID_EVENT(open_confirm);
-TRACE_DEFINE_ENUM(NFS4_OPEN_STID);
-TRACE_DEFINE_ENUM(NFS4_LOCK_STID);
-TRACE_DEFINE_ENUM(NFS4_DELEG_STID);
-TRACE_DEFINE_ENUM(NFS4_CLOSED_STID);
-TRACE_DEFINE_ENUM(NFS4_REVOKED_DELEG_STID);
-TRACE_DEFINE_ENUM(NFS4_CLOSED_DELEG_STID);
-TRACE_DEFINE_ENUM(NFS4_LAYOUT_STID);
-
#define show_stid_type(x) \
__print_flags(x, "|", \
- { NFS4_OPEN_STID, "OPEN" }, \
- { NFS4_LOCK_STID, "LOCK" }, \
- { NFS4_DELEG_STID, "DELEG" }, \
- { NFS4_CLOSED_STID, "CLOSED" }, \
- { NFS4_REVOKED_DELEG_STID, "REVOKED" }, \
- { NFS4_CLOSED_DELEG_STID, "CLOSED_DELEG" }, \
- { NFS4_LAYOUT_STID, "LAYOUT" })
+ { SC_TYPE_OPEN, "OPEN" }, \
+ { SC_TYPE_LOCK, "LOCK" }, \
+ { SC_TYPE_DELEG, "DELEG" }, \
+ { SC_TYPE_LAYOUT, "LAYOUT" })
+
+#define show_stid_status(x) \
+ __print_flags(x, "|", \
+ { SC_STATUS_CLOSED, "CLOSED" }, \
+ { SC_STATUS_REVOKED, "REVOKED" }) \
DECLARE_EVENT_CLASS(nfsd_stid_class,
TP_PROTO(
@@ -666,6 +660,7 @@ DECLARE_EVENT_CLASS(nfsd_stid_class,
TP_ARGS(stid),
TP_STRUCT__entry(
__field(unsigned long, sc_type)
+ __field(unsigned long, sc_status)
__field(int, sc_count)
__field(u32, cl_boot)
__field(u32, cl_id)
@@ -676,16 +671,18 @@ DECLARE_EVENT_CLASS(nfsd_stid_class,
const stateid_t *stp = &stid->sc_stateid;
__entry->sc_type = stid->sc_type;
+ __entry->sc_status = stid->sc_status;
__entry->sc_count = refcount_read(&stid->sc_count);
__entry->cl_boot = stp->si_opaque.so_clid.cl_boot;
__entry->cl_id = stp->si_opaque.so_clid.cl_id;
__entry->si_id = stp->si_opaque.so_id;
__entry->si_generation = stp->si_generation;
),
- TP_printk("client %08x:%08x stateid %08x:%08x ref=%d type=%s",
+ TP_printk("client %08x:%08x stateid %08x:%08x ref=%d type=%s state=%s",
__entry->cl_boot, __entry->cl_id,
__entry->si_id, __entry->si_generation,
- __entry->sc_count, show_stid_type(__entry->sc_type)
+ __entry->sc_count, show_stid_type(__entry->sc_type),
+ show_stid_status(__entry->sc_status)
)
);
--
2.43.0
^ permalink raw reply related [flat|nested] 27+ messages in thread* Re: [PATCH 05/13] nfsd: split sc_status out of sc_type
2024-01-29 3:29 ` [PATCH 05/13] nfsd: split sc_status out of sc_type NeilBrown
@ 2024-01-29 12:21 ` Jeff Layton
2024-01-29 14:04 ` Chuck Lever
1 sibling, 0 replies; 27+ messages in thread
From: Jeff Layton @ 2024-01-29 12:21 UTC (permalink / raw)
To: NeilBrown, Chuck Lever
Cc: linux-nfs, Olga Kornievskaia, Dai Ngo, Tom Talpey,
Christoph Hellwig, Tom Haynes
On Mon, 2024-01-29 at 14:29 +1100, NeilBrown wrote:
> sc_type identifies the type of a state - open, lock, deleg, layout - and
> also the status of a state - closed or revoked.
>
> This is a bit untidy and could get worse when "admin-revoked" states are
> added. So clean it up.
>
> With this patch, the type is now all that is stored in sc_type. This is
> zero when the state is first added to ->cl_stateids (causing it to be
> ignored), and is then set appropriately once it is fully initialised.
> It is set under ->cl_lock to ensure atomicity w.r.t lookup. It is now
> never cleared.
>
> sc_type is still a bit-set even though at most one bit is set. This allows
> lookup functions to be given a bitmap of acceptable types.
>
> sc_type is now an unsigned short rather than char. There is no value in
> restricting to just 8 bits.
>
> All the constants now start SC_TYPE_ matching the field in which they
> are stored. Keeping the existing names and ensuring clear separation
> from non-type flags would have required something like
> NFS4_STID_TYPE_CLOSED which is cumbersome. The "NFS4" prefix is
> redundant was they only appear in NFS4 code, so remove that and change
> STID to SC to match the field.
>
> The status is stored in a separate unsigned short named "sc_status". It
> has two flags: SC_STATUS_CLOSED and SC_STATUS_REVOKED.
> CLOSED combines NFS4_CLOSED_STID, NFS4_CLOSED_DELEG_STID, and is used
> for SC_TYPE_LOCK and SC_TYPE_LAYOUT instead of setting the sc_type to zero.
> These flags are only ever set, never cleared.
> For deleg stateids they are set under the global state_lock.
> For open and lock stateids they are set under ->cl_lock.
> For layout stateids they are set under ->ls_lock
>
> nfs4_unhash_stid() has been removed, and we never set sc_type = 0. This
> was only used for LOCK and LAYOUT stids and they now use
> SC_STATUS_CLOSED.
>
> Also TRACE_DEFINE_NUM() calls for the various STID #define have been
> removed because these things are not enums, and so that call is
> incorrect.
>
> Signed-off-by: NeilBrown <neilb@suse.de>
> ---
> fs/nfsd/nfs4layouts.c | 14 +--
> fs/nfsd/nfs4state.c | 207 +++++++++++++++++++++---------------------
> fs/nfsd/state.h | 40 +++++---
> fs/nfsd/trace.h | 31 +++----
> 4 files changed, 151 insertions(+), 141 deletions(-)
>
> diff --git a/fs/nfsd/nfs4layouts.c b/fs/nfsd/nfs4layouts.c
> index 5e8096bc5eaa..857b822450b4 100644
> --- a/fs/nfsd/nfs4layouts.c
> +++ b/fs/nfsd/nfs4layouts.c
> @@ -236,7 +236,7 @@ nfsd4_alloc_layout_stateid(struct nfsd4_compound_state *cstate,
> nfsd4_init_cb(&ls->ls_recall, clp, &nfsd4_cb_layout_ops,
> NFSPROC4_CLNT_CB_LAYOUT);
>
> - if (parent->sc_type == NFS4_DELEG_STID)
> + if (parent->sc_type == SC_TYPE_DELEG)
> ls->ls_file = nfsd_file_get(fp->fi_deleg_file);
> else
> ls->ls_file = find_any_file(fp);
> @@ -250,7 +250,7 @@ nfsd4_alloc_layout_stateid(struct nfsd4_compound_state *cstate,
> }
>
> spin_lock(&clp->cl_lock);
> - stp->sc_type = NFS4_LAYOUT_STID;
> + stp->sc_type = SC_TYPE_LAYOUT;
> list_add(&ls->ls_perclnt, &clp->cl_lo_states);
> spin_unlock(&clp->cl_lock);
>
> @@ -269,13 +269,13 @@ nfsd4_preprocess_layout_stateid(struct svc_rqst *rqstp,
> {
> struct nfs4_layout_stateid *ls;
> struct nfs4_stid *stid;
> - unsigned char typemask = NFS4_LAYOUT_STID;
> + unsigned short typemask = SC_TYPE_LAYOUT;
> __be32 status;
>
> if (create)
> - typemask |= (NFS4_OPEN_STID | NFS4_LOCK_STID | NFS4_DELEG_STID);
> + typemask |= (SC_TYPE_OPEN | SC_TYPE_LOCK | SC_TYPE_DELEG);
>
> - status = nfsd4_lookup_stateid(cstate, stateid, typemask, &stid,
> + status = nfsd4_lookup_stateid(cstate, stateid, typemask, 0, &stid,
> net_generic(SVC_NET(rqstp), nfsd_net_id));
> if (status)
> goto out;
> @@ -286,7 +286,7 @@ nfsd4_preprocess_layout_stateid(struct svc_rqst *rqstp,
> goto out_put_stid;
> }
>
> - if (stid->sc_type != NFS4_LAYOUT_STID) {
> + if (stid->sc_type != SC_TYPE_LAYOUT) {
> ls = nfsd4_alloc_layout_stateid(cstate, stid, layout_type);
> nfs4_put_stid(stid);
>
> @@ -518,7 +518,7 @@ nfsd4_return_file_layouts(struct svc_rqst *rqstp,
> lrp->lrs_present = true;
> } else {
> trace_nfsd_layoutstate_unhash(&ls->ls_stid.sc_stateid);
> - nfs4_unhash_stid(&ls->ls_stid);
> + ls->ls_stid.sc_status |= SC_STATUS_CLOSED;
> lrp->lrs_present = false;
> }
> spin_unlock(&ls->ls_lock);
> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
> index dbf9ed84610e..6bccdd0af814 100644
> --- a/fs/nfsd/nfs4state.c
> +++ b/fs/nfsd/nfs4state.c
> @@ -1260,11 +1260,6 @@ static void destroy_unhashed_deleg(struct nfs4_delegation *dp)
> nfs4_put_stid(&dp->dl_stid);
> }
>
> -void nfs4_unhash_stid(struct nfs4_stid *s)
> -{
> - s->sc_type = 0;
> -}
> -
> /**
> * nfs4_delegation_exists - Discover if this delegation already exists
> * @clp: a pointer to the nfs4_client we're granting a delegation to
> @@ -1317,7 +1312,7 @@ hash_delegation_locked(struct nfs4_delegation *dp, struct nfs4_file *fp)
> if (nfs4_delegation_exists(clp, fp))
> return -EAGAIN;
> refcount_inc(&dp->dl_stid.sc_count);
> - dp->dl_stid.sc_type = NFS4_DELEG_STID;
> + dp->dl_stid.sc_type = SC_TYPE_DELEG;
> list_add(&dp->dl_perfile, &fp->fi_delegations);
> list_add(&dp->dl_perclnt, &clp->cl_delegations);
> return 0;
> @@ -1329,7 +1324,7 @@ static bool delegation_hashed(struct nfs4_delegation *dp)
> }
>
> static bool
> -unhash_delegation_locked(struct nfs4_delegation *dp, unsigned char type)
> +unhash_delegation_locked(struct nfs4_delegation *dp, unsigned short statusmask)
> {
> struct nfs4_file *fp = dp->dl_stid.sc_file;
>
> @@ -1339,8 +1334,9 @@ unhash_delegation_locked(struct nfs4_delegation *dp, unsigned char type)
> return false;
>
> if (dp->dl_stid.sc_client->cl_minorversion == 0)
> - type = NFS4_CLOSED_DELEG_STID;
> - dp->dl_stid.sc_type = type;
> + statusmask = SC_STATUS_CLOSED;
> + dp->dl_stid.sc_status |= statusmask;
> +
> /* Ensure that deleg break won't try to requeue it */
> ++dp->dl_time;
> spin_lock(&fp->fi_lock);
> @@ -1356,7 +1352,7 @@ static void destroy_delegation(struct nfs4_delegation *dp)
> bool unhashed;
>
> spin_lock(&state_lock);
> - unhashed = unhash_delegation_locked(dp, NFS4_CLOSED_DELEG_STID);
> + unhashed = unhash_delegation_locked(dp, SC_STATUS_CLOSED);
> spin_unlock(&state_lock);
> if (unhashed)
> destroy_unhashed_deleg(dp);
> @@ -1370,7 +1366,7 @@ static void revoke_delegation(struct nfs4_delegation *dp)
>
> trace_nfsd_stid_revoke(&dp->dl_stid);
>
> - if (dp->dl_stid.sc_type == NFS4_REVOKED_DELEG_STID) {
> + if (dp->dl_stid.sc_status & SC_STATUS_REVOKED) {
> spin_lock(&clp->cl_lock);
> refcount_inc(&dp->dl_stid.sc_count);
> list_add(&dp->dl_recall_lru, &clp->cl_revoked);
> @@ -1379,8 +1375,8 @@ static void revoke_delegation(struct nfs4_delegation *dp)
> destroy_unhashed_deleg(dp);
> }
>
> -/*
> - * SETCLIENTID state
> +/*
> + * SETCLIENTID state
> */
>
> static unsigned int clientid_hashval(u32 id)
> @@ -1543,7 +1539,7 @@ static bool unhash_lock_stateid(struct nfs4_ol_stateid *stp)
> if (!unhash_ol_stateid(stp))
> return false;
> list_del_init(&stp->st_locks);
> - nfs4_unhash_stid(&stp->st_stid);
> + stp->st_stid.sc_status |= SC_STATUS_CLOSED;
> return true;
> }
>
> @@ -1622,6 +1618,7 @@ static void release_open_stateid(struct nfs4_ol_stateid *stp)
> LIST_HEAD(reaplist);
>
> spin_lock(&stp->st_stid.sc_client->cl_lock);
> + stp->st_stid.sc_status |= SC_STATUS_CLOSED;
> if (unhash_open_stateid(stp, &reaplist))
> put_ol_stateid_locked(stp, &reaplist);
> spin_unlock(&stp->st_stid.sc_client->cl_lock);
> @@ -2230,7 +2227,7 @@ __destroy_client(struct nfs4_client *clp)
> spin_lock(&state_lock);
> while (!list_empty(&clp->cl_delegations)) {
> dp = list_entry(clp->cl_delegations.next, struct nfs4_delegation, dl_perclnt);
> - unhash_delegation_locked(dp, NFS4_CLOSED_DELEG_STID);
> + unhash_delegation_locked(dp, SC_STATUS_CLOSED);
> list_add(&dp->dl_recall_lru, &reaplist);
> }
> spin_unlock(&state_lock);
> @@ -2462,14 +2459,16 @@ find_stateid_locked(struct nfs4_client *cl, stateid_t *t)
> }
>
> static struct nfs4_stid *
> -find_stateid_by_type(struct nfs4_client *cl, stateid_t *t, char typemask)
> +find_stateid_by_type(struct nfs4_client *cl, stateid_t *t,
> + unsigned short typemask, unsigned short ok_states)
> {
> struct nfs4_stid *s;
>
> spin_lock(&cl->cl_lock);
> s = find_stateid_locked(cl, t);
> if (s != NULL) {
> - if (typemask & s->sc_type)
> + if ((s->sc_status & ~ok_states) == 0 &&
> + (typemask & s->sc_type))
> refcount_inc(&s->sc_count);
> else
> s = NULL;
> @@ -2622,7 +2621,7 @@ 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)
> + if (st->sc_type != SC_TYPE_OPEN && st->sc_type != SC_TYPE_LOCK)
> return 0; /* XXX: or SEQ_SKIP? */
> ols = openlockstateid(st);
> oo = ols->st_stateowner;
> @@ -2754,13 +2753,13 @@ static int states_show(struct seq_file *s, void *v)
> struct nfs4_stid *st = v;
>
> switch (st->sc_type) {
> - case NFS4_OPEN_STID:
> + case SC_TYPE_OPEN:
> return nfs4_show_open(s, st);
> - case NFS4_LOCK_STID:
> + case SC_TYPE_LOCK:
> return nfs4_show_lock(s, st);
> - case NFS4_DELEG_STID:
> + case SC_TYPE_DELEG:
> return nfs4_show_deleg(s, st);
> - case NFS4_LAYOUT_STID:
> + case SC_TYPE_LAYOUT:
> return nfs4_show_layout(s, st);
> default:
> return 0; /* XXX: or SEQ_SKIP? */
> @@ -4532,7 +4531,8 @@ nfsd4_find_existing_open(struct nfs4_file *fp, struct nfsd4_open *open)
> continue;
> if (local->st_stateowner != &oo->oo_owner)
> continue;
> - if (local->st_stid.sc_type == NFS4_OPEN_STID) {
> + if (local->st_stid.sc_type == SC_TYPE_OPEN &&
> + !local->st_stid.sc_status) {
> ret = local;
> refcount_inc(&ret->st_stid.sc_count);
> break;
> @@ -4546,17 +4546,10 @@ nfsd4_verify_open_stid(struct nfs4_stid *s)
> {
> __be32 ret = nfs_ok;
>
> - switch (s->sc_type) {
> - default:
> - break;
> - case 0:
> - case NFS4_CLOSED_STID:
> - case NFS4_CLOSED_DELEG_STID:
> - ret = nfserr_bad_stateid;
> - break;
> - case NFS4_REVOKED_DELEG_STID:
> + if (s->sc_status & SC_STATUS_REVOKED)
> ret = nfserr_deleg_revoked;
> - }
> + else if (s->sc_status & SC_STATUS_CLOSED)
> + ret = nfserr_bad_stateid;
> return ret;
> }
>
> @@ -4642,7 +4635,7 @@ init_open_stateid(struct nfs4_file *fp, struct nfsd4_open *open)
>
> open->op_stp = NULL;
> refcount_inc(&stp->st_stid.sc_count);
> - stp->st_stid.sc_type = NFS4_OPEN_STID;
> + stp->st_stid.sc_type = SC_TYPE_OPEN;
> INIT_LIST_HEAD(&stp->st_locks);
> stp->st_stateowner = nfs4_get_stateowner(&oo->oo_owner);
> get_nfs4_file(fp);
> @@ -4869,9 +4862,9 @@ static int nfsd4_cb_recall_done(struct nfsd4_callback *cb,
>
> trace_nfsd_cb_recall_done(&dp->dl_stid.sc_stateid, task);
>
> - if (dp->dl_stid.sc_type == NFS4_CLOSED_DELEG_STID ||
> - dp->dl_stid.sc_type == NFS4_REVOKED_DELEG_STID)
> - return 1;
> + if (dp->dl_stid.sc_status)
> + /* CLOSED or REVOKED */
> + return 1;
>
> switch (task->tk_status) {
> case 0:
> @@ -5116,12 +5109,12 @@ static int share_access_to_flags(u32 share_access)
> return share_access == NFS4_SHARE_ACCESS_READ ? RD_STATE : WR_STATE;
> }
>
> -static struct nfs4_delegation *find_deleg_stateid(struct nfs4_client *cl, stateid_t *s)
> +static struct nfs4_delegation *find_deleg_stateid(struct nfs4_client *cl,
> + stateid_t *s)
> {
> struct nfs4_stid *ret;
>
> - ret = find_stateid_by_type(cl, s,
> - NFS4_DELEG_STID|NFS4_REVOKED_DELEG_STID);
> + ret = find_stateid_by_type(cl, s, SC_TYPE_DELEG, SC_STATUS_REVOKED);
> if (!ret)
> return NULL;
> return delegstateid(ret);
> @@ -5144,7 +5137,7 @@ 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_type == NFS4_REVOKED_DELEG_STID) {
> + if (deleg->dl_stid.sc_status & SC_STATUS_REVOKED) {
> nfs4_put_stid(&deleg->dl_stid);
> status = nfserr_deleg_revoked;
> goto out;
> @@ -5777,7 +5770,6 @@ nfsd4_process_open2(struct svc_rqst *rqstp, struct svc_fh *current_fh, struct nf
> } else {
> status = nfs4_get_vfs_file(rqstp, fp, current_fh, stp, open, true);
> if (status) {
> - stp->st_stid.sc_type = NFS4_CLOSED_STID;
> release_open_stateid(stp);
> mutex_unlock(&stp->st_mutex);
> goto out;
> @@ -6169,7 +6161,7 @@ nfs4_laundromat(struct nfsd_net *nn)
> dp = list_entry (pos, struct nfs4_delegation, dl_recall_lru);
> if (!state_expired(<, dp->dl_time))
> break;
> - unhash_delegation_locked(dp, NFS4_REVOKED_DELEG_STID);
> + unhash_delegation_locked(dp, SC_STATUS_REVOKED);
> list_add(&dp->dl_recall_lru, &reaplist);
> }
> spin_unlock(&state_lock);
> @@ -6408,22 +6400,20 @@ static __be32 nfsd4_validate_stateid(struct nfs4_client *cl, stateid_t *stateid)
> status = nfsd4_stid_check_stateid_generation(stateid, s, 1);
> if (status)
> goto out_unlock;
> + status = nfsd4_verify_open_stid(s);
> + if (status)
> + goto out_unlock;
> +
> switch (s->sc_type) {
> - case NFS4_DELEG_STID:
> + case SC_TYPE_DELEG:
> status = nfs_ok;
> break;
> - case NFS4_REVOKED_DELEG_STID:
> - status = nfserr_deleg_revoked;
> - break;
> - case NFS4_OPEN_STID:
> - case NFS4_LOCK_STID:
> + case SC_TYPE_OPEN:
> + case SC_TYPE_LOCK:
> status = nfsd4_check_openowner_confirmed(openlockstateid(s));
> break;
> default:
> printk("unknown stateid type %x\n", s->sc_type);
> - fallthrough;
> - case NFS4_CLOSED_STID:
> - case NFS4_CLOSED_DELEG_STID:
> status = nfserr_bad_stateid;
> }
> out_unlock:
> @@ -6433,7 +6423,8 @@ static __be32 nfsd4_validate_stateid(struct nfs4_client *cl, stateid_t *stateid)
>
> __be32
> nfsd4_lookup_stateid(struct nfsd4_compound_state *cstate,
> - stateid_t *stateid, unsigned char typemask,
> + stateid_t *stateid,
> + unsigned short typemask, unsigned short statusmask,
> struct nfs4_stid **s, struct nfsd_net *nn)
> {
> __be32 status;
> @@ -6444,10 +6435,13 @@ nfsd4_lookup_stateid(struct nfsd4_compound_state *cstate,
> * only return revoked delegations if explicitly asked.
> * otherwise we report revoked or bad_stateid status.
> */
> - if (typemask & NFS4_REVOKED_DELEG_STID)
> + if (statusmask & SC_STATUS_REVOKED)
> return_revoked = true;
> - else if (typemask & NFS4_DELEG_STID)
> - typemask |= NFS4_REVOKED_DELEG_STID;
> + if (typemask & SC_TYPE_DELEG)
> + /* Always allow REVOKED for DELEG so we can
> + * retturn the appropriate error.
> + */
> + statusmask |= SC_STATUS_REVOKED;
>
> if (ZERO_STATEID(stateid) || ONE_STATEID(stateid) ||
> CLOSE_STATEID(stateid))
> @@ -6460,14 +6454,12 @@ nfsd4_lookup_stateid(struct nfsd4_compound_state *cstate,
> }
> if (status)
> return status;
> - stid = find_stateid_by_type(cstate->clp, stateid, typemask);
> + stid = find_stateid_by_type(cstate->clp, stateid, typemask, statusmask);
> if (!stid)
> return nfserr_bad_stateid;
> - if ((stid->sc_type == NFS4_REVOKED_DELEG_STID) && !return_revoked) {
> + if ((stid->sc_status & SC_STATUS_REVOKED) && !return_revoked) {
> nfs4_put_stid(stid);
> - if (cstate->minorversion)
> - return nfserr_deleg_revoked;
> - return nfserr_bad_stateid;
> + return nfserr_deleg_revoked;
> }
> *s = stid;
> return nfs_ok;
> @@ -6478,17 +6470,17 @@ nfs4_find_file(struct nfs4_stid *s, int flags)
> {
> struct nfsd_file *ret = NULL;
>
> - if (!s)
> + if (!s || s->sc_status)
> return NULL;
>
> switch (s->sc_type) {
> - case NFS4_DELEG_STID:
> + case SC_TYPE_DELEG:
> spin_lock(&s->sc_file->fi_lock);
> ret = nfsd_file_get(s->sc_file->fi_deleg_file);
> spin_unlock(&s->sc_file->fi_lock);
> break;
> - case NFS4_OPEN_STID:
> - case NFS4_LOCK_STID:
> + case SC_TYPE_OPEN:
> + case SC_TYPE_LOCK:
> if (flags & RD_STATE)
> ret = find_readable_file(s->sc_file);
> else
> @@ -6601,7 +6593,8 @@ static __be32 find_cpntf_state(struct nfsd_net *nn, stateid_t *st,
> goto out;
>
> *stid = find_stateid_by_type(found, &cps->cp_p_stateid,
> - NFS4_DELEG_STID|NFS4_OPEN_STID|NFS4_LOCK_STID);
> + SC_TYPE_DELEG|SC_TYPE_OPEN|SC_TYPE_LOCK,
> + 0);
> if (*stid)
> status = nfs_ok;
> else
> @@ -6658,8 +6651,8 @@ nfs4_preprocess_stateid_op(struct svc_rqst *rqstp,
> }
>
> status = nfsd4_lookup_stateid(cstate, stateid,
> - NFS4_DELEG_STID|NFS4_OPEN_STID|NFS4_LOCK_STID,
> - &s, nn);
> + SC_TYPE_DELEG|SC_TYPE_OPEN|SC_TYPE_LOCK,
> + 0, &s, nn);
> if (status == nfserr_bad_stateid)
> status = find_cpntf_state(nn, stateid, &s);
> if (status)
> @@ -6670,16 +6663,13 @@ nfs4_preprocess_stateid_op(struct svc_rqst *rqstp,
> goto out;
>
> switch (s->sc_type) {
> - case NFS4_DELEG_STID:
> + case SC_TYPE_DELEG:
> status = nfs4_check_delegmode(delegstateid(s), flags);
> break;
> - case NFS4_OPEN_STID:
> - case NFS4_LOCK_STID:
> + case SC_TYPE_OPEN:
> + case SC_TYPE_LOCK:
> status = nfs4_check_olstateid(openlockstateid(s), flags);
> break;
> - default:
> - status = nfserr_bad_stateid;
> - break;
> }
> if (status)
> goto out;
> @@ -6758,33 +6748,34 @@ nfsd4_free_stateid(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
>
> spin_lock(&cl->cl_lock);
> s = find_stateid_locked(cl, stateid);
> - if (!s)
> + if (!s || s->sc_status & SC_STATUS_CLOSED)
> goto out_unlock;
> spin_lock(&s->sc_lock);
> switch (s->sc_type) {
> - case NFS4_DELEG_STID:
> + case SC_TYPE_DELEG:
> + if (s->sc_status & SC_STATUS_REVOKED) {
> + spin_unlock(&s->sc_lock);
> + dp = delegstateid(s);
> + list_del_init(&dp->dl_recall_lru);
> + spin_unlock(&cl->cl_lock);
> + nfs4_put_stid(s);
> + ret = nfs_ok;
> + goto out;
> + }
> ret = nfserr_locks_held;
> break;
> - case NFS4_OPEN_STID:
> + case SC_TYPE_OPEN:
> ret = check_stateid_generation(stateid, &s->sc_stateid, 1);
> if (ret)
> break;
> ret = nfserr_locks_held;
> break;
> - case NFS4_LOCK_STID:
> + case SC_TYPE_LOCK:
> spin_unlock(&s->sc_lock);
> refcount_inc(&s->sc_count);
> spin_unlock(&cl->cl_lock);
> ret = nfsd4_free_lock_stateid(stateid, s);
> goto out;
> - case NFS4_REVOKED_DELEG_STID:
> - spin_unlock(&s->sc_lock);
> - dp = delegstateid(s);
> - list_del_init(&dp->dl_recall_lru);
> - spin_unlock(&cl->cl_lock);
> - nfs4_put_stid(s);
> - ret = nfs_ok;
> - goto out;
> /* Default falls through and returns nfserr_bad_stateid */
> }
> spin_unlock(&s->sc_lock);
> @@ -6827,6 +6818,7 @@ static __be32 nfs4_seqid_op_checks(struct nfsd4_compound_state *cstate, stateid_
> * @seqid: seqid (provided by client)
> * @stateid: stateid (provided by client)
> * @typemask: mask of allowable types for this operation
> + * @statusmask: mask of allowed states: 0 or STID_CLOSED
> * @stpp: return pointer for the stateid found
> * @nn: net namespace for request
> *
> @@ -6836,7 +6828,8 @@ static __be32 nfs4_seqid_op_checks(struct nfsd4_compound_state *cstate, stateid_
> */
> static __be32
> nfs4_preprocess_seqid_op(struct nfsd4_compound_state *cstate, u32 seqid,
> - stateid_t *stateid, char typemask,
> + stateid_t *stateid,
> + unsigned short typemask, unsigned short statusmask,
> struct nfs4_ol_stateid **stpp,
> struct nfsd_net *nn)
> {
> @@ -6847,7 +6840,8 @@ nfs4_preprocess_seqid_op(struct nfsd4_compound_state *cstate, u32 seqid,
> trace_nfsd_preprocess(seqid, stateid);
>
> *stpp = NULL;
> - status = nfsd4_lookup_stateid(cstate, stateid, typemask, &s, nn);
> + status = nfsd4_lookup_stateid(cstate, stateid,
> + typemask, statusmask, &s, nn);
> if (status)
> return status;
> stp = openlockstateid(s);
> @@ -6869,7 +6863,7 @@ static __be32 nfs4_preprocess_confirmed_seqid_op(struct nfsd4_compound_state *cs
> struct nfs4_ol_stateid *stp;
>
> status = nfs4_preprocess_seqid_op(cstate, seqid, stateid,
> - NFS4_OPEN_STID, &stp, nn);
> + SC_TYPE_OPEN, 0, &stp, nn);
> if (status)
> return status;
> oo = openowner(stp->st_stateowner);
> @@ -6900,8 +6894,8 @@ nfsd4_open_confirm(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
> return status;
>
> status = nfs4_preprocess_seqid_op(cstate,
> - oc->oc_seqid, &oc->oc_req_stateid,
> - NFS4_OPEN_STID, &stp, nn);
> + oc->oc_seqid, &oc->oc_req_stateid,
> + SC_TYPE_OPEN, 0, &stp, nn);
> if (status)
> goto out;
> oo = openowner(stp->st_stateowner);
> @@ -7031,18 +7025,20 @@ nfsd4_close(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
> struct net *net = SVC_NET(rqstp);
> struct nfsd_net *nn = net_generic(net, nfsd_net_id);
>
> - dprintk("NFSD: nfsd4_close on file %pd\n",
> + dprintk("NFSD: nfsd4_close on file %pd\n",
> cstate->current_fh.fh_dentry);
>
> status = nfs4_preprocess_seqid_op(cstate, close->cl_seqid,
> - &close->cl_stateid,
> - NFS4_OPEN_STID|NFS4_CLOSED_STID,
> - &stp, nn);
> + &close->cl_stateid,
> + SC_TYPE_OPEN, SC_STATUS_CLOSED,
> + &stp, nn);
> nfsd4_bump_seqid(cstate, status);
> if (status)
> - goto out;
> + goto out;
>
> - stp->st_stid.sc_type = NFS4_CLOSED_STID;
> + spin_lock(&stp->st_stid.sc_client->cl_lock);
> + stp->st_stid.sc_status |= SC_STATUS_CLOSED;
> + spin_unlock(&stp->st_stid.sc_client->cl_lock);
>
> /*
> * Technically we don't _really_ have to increment or copy it, since
> @@ -7084,7 +7080,7 @@ nfsd4_delegreturn(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
> if ((status = fh_verify(rqstp, &cstate->current_fh, S_IFREG, 0)))
> return status;
>
> - status = nfsd4_lookup_stateid(cstate, stateid, NFS4_DELEG_STID, &s, nn);
> + status = nfsd4_lookup_stateid(cstate, stateid, SC_TYPE_DELEG, 0, &s, nn);
> if (status)
> goto out;
> dp = delegstateid(s);
> @@ -7351,7 +7347,7 @@ init_lock_stateid(struct nfs4_ol_stateid *stp, struct nfs4_lockowner *lo,
> if (retstp)
> goto out_found;
> refcount_inc(&stp->st_stid.sc_count);
> - stp->st_stid.sc_type = NFS4_LOCK_STID;
> + stp->st_stid.sc_type = SC_TYPE_LOCK;
> stp->st_stateowner = nfs4_get_stateowner(&lo->lo_owner);
> get_nfs4_file(fp);
> stp->st_stid.sc_file = fp;
> @@ -7538,9 +7534,10 @@ nfsd4_lock(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
> &lock_stp, &new);
> } else {
> status = nfs4_preprocess_seqid_op(cstate,
> - lock->lk_old_lock_seqid,
> - &lock->lk_old_lock_stateid,
> - NFS4_LOCK_STID, &lock_stp, nn);
> + lock->lk_old_lock_seqid,
> + &lock->lk_old_lock_stateid,
> + SC_TYPE_LOCK, 0, &lock_stp,
> + nn);
> }
> if (status)
> goto out;
> @@ -7853,8 +7850,8 @@ nfsd4_locku(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
> return nfserr_inval;
>
> status = nfs4_preprocess_seqid_op(cstate, locku->lu_seqid,
> - &locku->lu_stateid, NFS4_LOCK_STID,
> - &stp, nn);
> + &locku->lu_stateid, SC_TYPE_LOCK, 0,
> + &stp, nn);
> if (status)
> goto out;
> nf = find_any_file(stp->st_stid.sc_file);
> @@ -8292,7 +8289,7 @@ nfs4_state_shutdown_net(struct net *net)
> spin_lock(&state_lock);
> list_for_each_safe(pos, next, &nn->del_recall_lru) {
> dp = list_entry (pos, struct nfs4_delegation, dl_recall_lru);
> - unhash_delegation_locked(dp, NFS4_CLOSED_DELEG_STID);
> + unhash_delegation_locked(dp, SC_STATUS_CLOSED);
> list_add(&dp->dl_recall_lru, &reaplist);
> }
> spin_unlock(&state_lock);
> diff --git a/fs/nfsd/state.h b/fs/nfsd/state.h
> index 41bdc913fa71..ffc8920d0558 100644
> --- a/fs/nfsd/state.h
> +++ b/fs/nfsd/state.h
> @@ -88,17 +88,33 @@ struct nfsd4_callback_ops {
> */
> struct nfs4_stid {
> refcount_t sc_count;
> -#define NFS4_OPEN_STID 1
> -#define NFS4_LOCK_STID 2
> -#define NFS4_DELEG_STID 4
> -/* For an open stateid kept around *only* to process close replays: */
> -#define NFS4_CLOSED_STID 8
> +
> + /* A new stateid is added to the cl_stateids idr early before it
> + * is fully initialised. Its sc_type is then zero. After
> + * initialisation the sc_type it set under cl_lock, and then
> + * never changes.
> + */
> +#define SC_TYPE_OPEN BIT(0)
> +#define SC_TYPE_LOCK BIT(1)
> +#define SC_TYPE_DELEG BIT(2)
> +#define SC_TYPE_LAYOUT BIT(3)
> + unsigned short sc_type;
> +
> +/* state_lock protects sc_status for delegation stateids.
> + * ->cl_lock protects sc_status for open and lock stateids.
> + * ->st_mutex also protect sc_status for open stateids.
> + * ->ls_lock protects sc_status for layout stateids.
> + */
> +/*
> + * For an open stateid kept around *only* to process close replays.
> + * For deleg stateid, kept in idr until last reference is dropped.
> + */
> +#define SC_STATUS_CLOSED BIT(0)
> /* For a deleg stateid kept around only to process free_stateid's: */
> -#define NFS4_REVOKED_DELEG_STID 16
> -#define NFS4_CLOSED_DELEG_STID 32
> -#define NFS4_LAYOUT_STID 64
> +#define SC_STATUS_REVOKED BIT(1)
> + unsigned short sc_status;
> +
> struct list_head sc_cp_list;
> - unsigned char sc_type;
> stateid_t sc_stateid;
> spinlock_t sc_lock;
> struct nfs4_client *sc_client;
> @@ -672,15 +688,15 @@ extern __be32 nfs4_preprocess_stateid_op(struct svc_rqst *rqstp,
> stateid_t *stateid, int flags, struct nfsd_file **filp,
> struct nfs4_stid **cstid);
> __be32 nfsd4_lookup_stateid(struct nfsd4_compound_state *cstate,
> - stateid_t *stateid, unsigned char typemask,
> - struct nfs4_stid **s, struct nfsd_net *nn);
> + stateid_t *stateid, unsigned short typemask,
> + unsigned short statusmask,
> + struct nfs4_stid **s, struct nfsd_net *nn);
> struct nfs4_stid *nfs4_alloc_stid(struct nfs4_client *cl, struct kmem_cache *slab,
> void (*sc_free)(struct nfs4_stid *));
> int nfs4_init_copy_state(struct nfsd_net *nn, struct nfsd4_copy *copy);
> void nfs4_free_copy_state(struct nfsd4_copy *copy);
> struct nfs4_cpntf_state *nfs4_alloc_init_cpntf_state(struct nfsd_net *nn,
> struct nfs4_stid *p_stid);
> -void nfs4_unhash_stid(struct nfs4_stid *s);
> void nfs4_put_stid(struct nfs4_stid *s);
> void nfs4_inc_and_copy_stateid(stateid_t *dst, struct nfs4_stid *stid);
> void nfs4_remove_reclaim_record(struct nfs4_client_reclaim *, struct nfsd_net *);
> diff --git a/fs/nfsd/trace.h b/fs/nfsd/trace.h
> index d1e8cf079b0f..fe08ca18b647 100644
> --- a/fs/nfsd/trace.h
> +++ b/fs/nfsd/trace.h
> @@ -641,23 +641,17 @@ DEFINE_EVENT(nfsd_stateseqid_class, nfsd_##name, \
> DEFINE_STATESEQID_EVENT(preprocess);
> DEFINE_STATESEQID_EVENT(open_confirm);
>
> -TRACE_DEFINE_ENUM(NFS4_OPEN_STID);
> -TRACE_DEFINE_ENUM(NFS4_LOCK_STID);
> -TRACE_DEFINE_ENUM(NFS4_DELEG_STID);
> -TRACE_DEFINE_ENUM(NFS4_CLOSED_STID);
> -TRACE_DEFINE_ENUM(NFS4_REVOKED_DELEG_STID);
> -TRACE_DEFINE_ENUM(NFS4_CLOSED_DELEG_STID);
> -TRACE_DEFINE_ENUM(NFS4_LAYOUT_STID);
> -
> #define show_stid_type(x) \
> __print_flags(x, "|", \
> - { NFS4_OPEN_STID, "OPEN" }, \
> - { NFS4_LOCK_STID, "LOCK" }, \
> - { NFS4_DELEG_STID, "DELEG" }, \
> - { NFS4_CLOSED_STID, "CLOSED" }, \
> - { NFS4_REVOKED_DELEG_STID, "REVOKED" }, \
> - { NFS4_CLOSED_DELEG_STID, "CLOSED_DELEG" }, \
> - { NFS4_LAYOUT_STID, "LAYOUT" })
> + { SC_TYPE_OPEN, "OPEN" }, \
> + { SC_TYPE_LOCK, "LOCK" }, \
> + { SC_TYPE_DELEG, "DELEG" }, \
> + { SC_TYPE_LAYOUT, "LAYOUT" })
> +
> +#define show_stid_status(x) \
> + __print_flags(x, "|", \
> + { SC_STATUS_CLOSED, "CLOSED" }, \
> + { SC_STATUS_REVOKED, "REVOKED" }) \
>
> DECLARE_EVENT_CLASS(nfsd_stid_class,
> TP_PROTO(
> @@ -666,6 +660,7 @@ DECLARE_EVENT_CLASS(nfsd_stid_class,
> TP_ARGS(stid),
> TP_STRUCT__entry(
> __field(unsigned long, sc_type)
> + __field(unsigned long, sc_status)
> __field(int, sc_count)
> __field(u32, cl_boot)
> __field(u32, cl_id)
> @@ -676,16 +671,18 @@ DECLARE_EVENT_CLASS(nfsd_stid_class,
> const stateid_t *stp = &stid->sc_stateid;
>
> __entry->sc_type = stid->sc_type;
> + __entry->sc_status = stid->sc_status;
> __entry->sc_count = refcount_read(&stid->sc_count);
> __entry->cl_boot = stp->si_opaque.so_clid.cl_boot;
> __entry->cl_id = stp->si_opaque.so_clid.cl_id;
> __entry->si_id = stp->si_opaque.so_id;
> __entry->si_generation = stp->si_generation;
> ),
> - TP_printk("client %08x:%08x stateid %08x:%08x ref=%d type=%s",
> + TP_printk("client %08x:%08x stateid %08x:%08x ref=%d type=%s state=%s",
> __entry->cl_boot, __entry->cl_id,
> __entry->si_id, __entry->si_generation,
> - __entry->sc_count, show_stid_type(__entry->sc_type)
> + __entry->sc_count, show_stid_type(__entry->sc_type),
> + show_stid_status(__entry->sc_status)
> )
> );
>
Reviewed-by: Jeff Layton <jlayton@kernel.org>
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PATCH 05/13] nfsd: split sc_status out of sc_type
2024-01-29 3:29 ` [PATCH 05/13] nfsd: split sc_status out of sc_type NeilBrown
2024-01-29 12:21 ` Jeff Layton
@ 2024-01-29 14:04 ` Chuck Lever
1 sibling, 0 replies; 27+ messages in thread
From: Chuck Lever @ 2024-01-29 14:04 UTC (permalink / raw)
To: NeilBrown
Cc: Jeff Layton, linux-nfs, Olga Kornievskaia, Dai Ngo, Tom Talpey,
Christoph Hellwig, Tom Haynes
On Mon, Jan 29, 2024 at 02:29:27PM +1100, NeilBrown wrote:
> sc_type identifies the type of a state - open, lock, deleg, layout - and
> also the status of a state - closed or revoked.
>
> This is a bit untidy and could get worse when "admin-revoked" states are
> added. So clean it up.
>
> With this patch, the type is now all that is stored in sc_type. This is
> zero when the state is first added to ->cl_stateids (causing it to be
> ignored), and is then set appropriately once it is fully initialised.
> It is set under ->cl_lock to ensure atomicity w.r.t lookup. It is now
> never cleared.
>
> sc_type is still a bit-set even though at most one bit is set. This allows
> lookup functions to be given a bitmap of acceptable types.
>
> sc_type is now an unsigned short rather than char. There is no value in
> restricting to just 8 bits.
>
> All the constants now start SC_TYPE_ matching the field in which they
> are stored. Keeping the existing names and ensuring clear separation
> from non-type flags would have required something like
> NFS4_STID_TYPE_CLOSED which is cumbersome. The "NFS4" prefix is
> redundant was they only appear in NFS4 code, so remove that and change
> STID to SC to match the field.
>
> The status is stored in a separate unsigned short named "sc_status". It
> has two flags: SC_STATUS_CLOSED and SC_STATUS_REVOKED.
> CLOSED combines NFS4_CLOSED_STID, NFS4_CLOSED_DELEG_STID, and is used
> for SC_TYPE_LOCK and SC_TYPE_LAYOUT instead of setting the sc_type to zero.
> These flags are only ever set, never cleared.
> For deleg stateids they are set under the global state_lock.
> For open and lock stateids they are set under ->cl_lock.
> For layout stateids they are set under ->ls_lock
>
> nfs4_unhash_stid() has been removed, and we never set sc_type = 0. This
> was only used for LOCK and LAYOUT stids and they now use
> SC_STATUS_CLOSED.
>
> Also TRACE_DEFINE_NUM() calls for the various STID #define have been
> removed because these things are not enums, and so that call is
> incorrect.
>
> Signed-off-by: NeilBrown <neilb@suse.de>
> ---
> fs/nfsd/nfs4layouts.c | 14 +--
> fs/nfsd/nfs4state.c | 207 +++++++++++++++++++++---------------------
> fs/nfsd/state.h | 40 +++++---
> fs/nfsd/trace.h | 31 +++----
> 4 files changed, 151 insertions(+), 141 deletions(-)
This one doesn't apply cleanly to nfsd-next. Can you do a quick
rebase onto nfsd-next and send a fresh version of the series?
> diff --git a/fs/nfsd/nfs4layouts.c b/fs/nfsd/nfs4layouts.c
> index 5e8096bc5eaa..857b822450b4 100644
> --- a/fs/nfsd/nfs4layouts.c
> +++ b/fs/nfsd/nfs4layouts.c
> @@ -236,7 +236,7 @@ nfsd4_alloc_layout_stateid(struct nfsd4_compound_state *cstate,
> nfsd4_init_cb(&ls->ls_recall, clp, &nfsd4_cb_layout_ops,
> NFSPROC4_CLNT_CB_LAYOUT);
>
> - if (parent->sc_type == NFS4_DELEG_STID)
> + if (parent->sc_type == SC_TYPE_DELEG)
> ls->ls_file = nfsd_file_get(fp->fi_deleg_file);
> else
> ls->ls_file = find_any_file(fp);
> @@ -250,7 +250,7 @@ nfsd4_alloc_layout_stateid(struct nfsd4_compound_state *cstate,
> }
>
> spin_lock(&clp->cl_lock);
> - stp->sc_type = NFS4_LAYOUT_STID;
> + stp->sc_type = SC_TYPE_LAYOUT;
> list_add(&ls->ls_perclnt, &clp->cl_lo_states);
> spin_unlock(&clp->cl_lock);
>
> @@ -269,13 +269,13 @@ nfsd4_preprocess_layout_stateid(struct svc_rqst *rqstp,
> {
> struct nfs4_layout_stateid *ls;
> struct nfs4_stid *stid;
> - unsigned char typemask = NFS4_LAYOUT_STID;
> + unsigned short typemask = SC_TYPE_LAYOUT;
> __be32 status;
>
> if (create)
> - typemask |= (NFS4_OPEN_STID | NFS4_LOCK_STID | NFS4_DELEG_STID);
> + typemask |= (SC_TYPE_OPEN | SC_TYPE_LOCK | SC_TYPE_DELEG);
>
> - status = nfsd4_lookup_stateid(cstate, stateid, typemask, &stid,
> + status = nfsd4_lookup_stateid(cstate, stateid, typemask, 0, &stid,
> net_generic(SVC_NET(rqstp), nfsd_net_id));
> if (status)
> goto out;
> @@ -286,7 +286,7 @@ nfsd4_preprocess_layout_stateid(struct svc_rqst *rqstp,
> goto out_put_stid;
> }
>
> - if (stid->sc_type != NFS4_LAYOUT_STID) {
> + if (stid->sc_type != SC_TYPE_LAYOUT) {
> ls = nfsd4_alloc_layout_stateid(cstate, stid, layout_type);
> nfs4_put_stid(stid);
>
> @@ -518,7 +518,7 @@ nfsd4_return_file_layouts(struct svc_rqst *rqstp,
> lrp->lrs_present = true;
> } else {
> trace_nfsd_layoutstate_unhash(&ls->ls_stid.sc_stateid);
> - nfs4_unhash_stid(&ls->ls_stid);
> + ls->ls_stid.sc_status |= SC_STATUS_CLOSED;
> lrp->lrs_present = false;
> }
> spin_unlock(&ls->ls_lock);
> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
> index dbf9ed84610e..6bccdd0af814 100644
> --- a/fs/nfsd/nfs4state.c
> +++ b/fs/nfsd/nfs4state.c
> @@ -1260,11 +1260,6 @@ static void destroy_unhashed_deleg(struct nfs4_delegation *dp)
> nfs4_put_stid(&dp->dl_stid);
> }
>
> -void nfs4_unhash_stid(struct nfs4_stid *s)
> -{
> - s->sc_type = 0;
> -}
> -
> /**
> * nfs4_delegation_exists - Discover if this delegation already exists
> * @clp: a pointer to the nfs4_client we're granting a delegation to
> @@ -1317,7 +1312,7 @@ hash_delegation_locked(struct nfs4_delegation *dp, struct nfs4_file *fp)
> if (nfs4_delegation_exists(clp, fp))
> return -EAGAIN;
> refcount_inc(&dp->dl_stid.sc_count);
> - dp->dl_stid.sc_type = NFS4_DELEG_STID;
> + dp->dl_stid.sc_type = SC_TYPE_DELEG;
> list_add(&dp->dl_perfile, &fp->fi_delegations);
> list_add(&dp->dl_perclnt, &clp->cl_delegations);
> return 0;
> @@ -1329,7 +1324,7 @@ static bool delegation_hashed(struct nfs4_delegation *dp)
> }
>
> static bool
> -unhash_delegation_locked(struct nfs4_delegation *dp, unsigned char type)
> +unhash_delegation_locked(struct nfs4_delegation *dp, unsigned short statusmask)
> {
> struct nfs4_file *fp = dp->dl_stid.sc_file;
>
> @@ -1339,8 +1334,9 @@ unhash_delegation_locked(struct nfs4_delegation *dp, unsigned char type)
> return false;
>
> if (dp->dl_stid.sc_client->cl_minorversion == 0)
> - type = NFS4_CLOSED_DELEG_STID;
> - dp->dl_stid.sc_type = type;
> + statusmask = SC_STATUS_CLOSED;
> + dp->dl_stid.sc_status |= statusmask;
> +
> /* Ensure that deleg break won't try to requeue it */
> ++dp->dl_time;
> spin_lock(&fp->fi_lock);
> @@ -1356,7 +1352,7 @@ static void destroy_delegation(struct nfs4_delegation *dp)
> bool unhashed;
>
> spin_lock(&state_lock);
> - unhashed = unhash_delegation_locked(dp, NFS4_CLOSED_DELEG_STID);
> + unhashed = unhash_delegation_locked(dp, SC_STATUS_CLOSED);
> spin_unlock(&state_lock);
> if (unhashed)
> destroy_unhashed_deleg(dp);
> @@ -1370,7 +1366,7 @@ static void revoke_delegation(struct nfs4_delegation *dp)
>
> trace_nfsd_stid_revoke(&dp->dl_stid);
>
> - if (dp->dl_stid.sc_type == NFS4_REVOKED_DELEG_STID) {
> + if (dp->dl_stid.sc_status & SC_STATUS_REVOKED) {
> spin_lock(&clp->cl_lock);
> refcount_inc(&dp->dl_stid.sc_count);
> list_add(&dp->dl_recall_lru, &clp->cl_revoked);
> @@ -1379,8 +1375,8 @@ static void revoke_delegation(struct nfs4_delegation *dp)
> destroy_unhashed_deleg(dp);
> }
>
> -/*
> - * SETCLIENTID state
> +/*
> + * SETCLIENTID state
> */
>
> static unsigned int clientid_hashval(u32 id)
> @@ -1543,7 +1539,7 @@ static bool unhash_lock_stateid(struct nfs4_ol_stateid *stp)
> if (!unhash_ol_stateid(stp))
> return false;
> list_del_init(&stp->st_locks);
> - nfs4_unhash_stid(&stp->st_stid);
> + stp->st_stid.sc_status |= SC_STATUS_CLOSED;
> return true;
> }
>
> @@ -1622,6 +1618,7 @@ static void release_open_stateid(struct nfs4_ol_stateid *stp)
> LIST_HEAD(reaplist);
>
> spin_lock(&stp->st_stid.sc_client->cl_lock);
> + stp->st_stid.sc_status |= SC_STATUS_CLOSED;
> if (unhash_open_stateid(stp, &reaplist))
> put_ol_stateid_locked(stp, &reaplist);
> spin_unlock(&stp->st_stid.sc_client->cl_lock);
> @@ -2230,7 +2227,7 @@ __destroy_client(struct nfs4_client *clp)
> spin_lock(&state_lock);
> while (!list_empty(&clp->cl_delegations)) {
> dp = list_entry(clp->cl_delegations.next, struct nfs4_delegation, dl_perclnt);
> - unhash_delegation_locked(dp, NFS4_CLOSED_DELEG_STID);
> + unhash_delegation_locked(dp, SC_STATUS_CLOSED);
> list_add(&dp->dl_recall_lru, &reaplist);
> }
> spin_unlock(&state_lock);
> @@ -2462,14 +2459,16 @@ find_stateid_locked(struct nfs4_client *cl, stateid_t *t)
> }
>
> static struct nfs4_stid *
> -find_stateid_by_type(struct nfs4_client *cl, stateid_t *t, char typemask)
> +find_stateid_by_type(struct nfs4_client *cl, stateid_t *t,
> + unsigned short typemask, unsigned short ok_states)
> {
> struct nfs4_stid *s;
>
> spin_lock(&cl->cl_lock);
> s = find_stateid_locked(cl, t);
> if (s != NULL) {
> - if (typemask & s->sc_type)
> + if ((s->sc_status & ~ok_states) == 0 &&
> + (typemask & s->sc_type))
> refcount_inc(&s->sc_count);
> else
> s = NULL;
> @@ -2622,7 +2621,7 @@ 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)
> + if (st->sc_type != SC_TYPE_OPEN && st->sc_type != SC_TYPE_LOCK)
> return 0; /* XXX: or SEQ_SKIP? */
> ols = openlockstateid(st);
> oo = ols->st_stateowner;
> @@ -2754,13 +2753,13 @@ static int states_show(struct seq_file *s, void *v)
> struct nfs4_stid *st = v;
>
> switch (st->sc_type) {
> - case NFS4_OPEN_STID:
> + case SC_TYPE_OPEN:
> return nfs4_show_open(s, st);
> - case NFS4_LOCK_STID:
> + case SC_TYPE_LOCK:
> return nfs4_show_lock(s, st);
> - case NFS4_DELEG_STID:
> + case SC_TYPE_DELEG:
> return nfs4_show_deleg(s, st);
> - case NFS4_LAYOUT_STID:
> + case SC_TYPE_LAYOUT:
> return nfs4_show_layout(s, st);
> default:
> return 0; /* XXX: or SEQ_SKIP? */
> @@ -4532,7 +4531,8 @@ nfsd4_find_existing_open(struct nfs4_file *fp, struct nfsd4_open *open)
> continue;
> if (local->st_stateowner != &oo->oo_owner)
> continue;
> - if (local->st_stid.sc_type == NFS4_OPEN_STID) {
> + if (local->st_stid.sc_type == SC_TYPE_OPEN &&
> + !local->st_stid.sc_status) {
> ret = local;
> refcount_inc(&ret->st_stid.sc_count);
> break;
> @@ -4546,17 +4546,10 @@ nfsd4_verify_open_stid(struct nfs4_stid *s)
> {
> __be32 ret = nfs_ok;
>
> - switch (s->sc_type) {
> - default:
> - break;
> - case 0:
> - case NFS4_CLOSED_STID:
> - case NFS4_CLOSED_DELEG_STID:
> - ret = nfserr_bad_stateid;
> - break;
> - case NFS4_REVOKED_DELEG_STID:
> + if (s->sc_status & SC_STATUS_REVOKED)
> ret = nfserr_deleg_revoked;
> - }
> + else if (s->sc_status & SC_STATUS_CLOSED)
> + ret = nfserr_bad_stateid;
> return ret;
> }
>
> @@ -4642,7 +4635,7 @@ init_open_stateid(struct nfs4_file *fp, struct nfsd4_open *open)
>
> open->op_stp = NULL;
> refcount_inc(&stp->st_stid.sc_count);
> - stp->st_stid.sc_type = NFS4_OPEN_STID;
> + stp->st_stid.sc_type = SC_TYPE_OPEN;
> INIT_LIST_HEAD(&stp->st_locks);
> stp->st_stateowner = nfs4_get_stateowner(&oo->oo_owner);
> get_nfs4_file(fp);
> @@ -4869,9 +4862,9 @@ static int nfsd4_cb_recall_done(struct nfsd4_callback *cb,
>
> trace_nfsd_cb_recall_done(&dp->dl_stid.sc_stateid, task);
>
> - if (dp->dl_stid.sc_type == NFS4_CLOSED_DELEG_STID ||
> - dp->dl_stid.sc_type == NFS4_REVOKED_DELEG_STID)
> - return 1;
> + if (dp->dl_stid.sc_status)
> + /* CLOSED or REVOKED */
> + return 1;
>
> switch (task->tk_status) {
> case 0:
> @@ -5116,12 +5109,12 @@ static int share_access_to_flags(u32 share_access)
> return share_access == NFS4_SHARE_ACCESS_READ ? RD_STATE : WR_STATE;
> }
>
> -static struct nfs4_delegation *find_deleg_stateid(struct nfs4_client *cl, stateid_t *s)
> +static struct nfs4_delegation *find_deleg_stateid(struct nfs4_client *cl,
> + stateid_t *s)
> {
> struct nfs4_stid *ret;
>
> - ret = find_stateid_by_type(cl, s,
> - NFS4_DELEG_STID|NFS4_REVOKED_DELEG_STID);
> + ret = find_stateid_by_type(cl, s, SC_TYPE_DELEG, SC_STATUS_REVOKED);
> if (!ret)
> return NULL;
> return delegstateid(ret);
> @@ -5144,7 +5137,7 @@ 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_type == NFS4_REVOKED_DELEG_STID) {
> + if (deleg->dl_stid.sc_status & SC_STATUS_REVOKED) {
> nfs4_put_stid(&deleg->dl_stid);
> status = nfserr_deleg_revoked;
> goto out;
> @@ -5777,7 +5770,6 @@ nfsd4_process_open2(struct svc_rqst *rqstp, struct svc_fh *current_fh, struct nf
> } else {
> status = nfs4_get_vfs_file(rqstp, fp, current_fh, stp, open, true);
> if (status) {
> - stp->st_stid.sc_type = NFS4_CLOSED_STID;
> release_open_stateid(stp);
> mutex_unlock(&stp->st_mutex);
> goto out;
> @@ -6169,7 +6161,7 @@ nfs4_laundromat(struct nfsd_net *nn)
> dp = list_entry (pos, struct nfs4_delegation, dl_recall_lru);
> if (!state_expired(<, dp->dl_time))
> break;
> - unhash_delegation_locked(dp, NFS4_REVOKED_DELEG_STID);
> + unhash_delegation_locked(dp, SC_STATUS_REVOKED);
> list_add(&dp->dl_recall_lru, &reaplist);
> }
> spin_unlock(&state_lock);
> @@ -6408,22 +6400,20 @@ static __be32 nfsd4_validate_stateid(struct nfs4_client *cl, stateid_t *stateid)
> status = nfsd4_stid_check_stateid_generation(stateid, s, 1);
> if (status)
> goto out_unlock;
> + status = nfsd4_verify_open_stid(s);
> + if (status)
> + goto out_unlock;
> +
> switch (s->sc_type) {
> - case NFS4_DELEG_STID:
> + case SC_TYPE_DELEG:
> status = nfs_ok;
> break;
> - case NFS4_REVOKED_DELEG_STID:
> - status = nfserr_deleg_revoked;
> - break;
> - case NFS4_OPEN_STID:
> - case NFS4_LOCK_STID:
> + case SC_TYPE_OPEN:
> + case SC_TYPE_LOCK:
> status = nfsd4_check_openowner_confirmed(openlockstateid(s));
> break;
> default:
> printk("unknown stateid type %x\n", s->sc_type);
> - fallthrough;
> - case NFS4_CLOSED_STID:
> - case NFS4_CLOSED_DELEG_STID:
> status = nfserr_bad_stateid;
> }
> out_unlock:
> @@ -6433,7 +6423,8 @@ static __be32 nfsd4_validate_stateid(struct nfs4_client *cl, stateid_t *stateid)
>
> __be32
> nfsd4_lookup_stateid(struct nfsd4_compound_state *cstate,
> - stateid_t *stateid, unsigned char typemask,
> + stateid_t *stateid,
> + unsigned short typemask, unsigned short statusmask,
> struct nfs4_stid **s, struct nfsd_net *nn)
> {
> __be32 status;
> @@ -6444,10 +6435,13 @@ nfsd4_lookup_stateid(struct nfsd4_compound_state *cstate,
> * only return revoked delegations if explicitly asked.
> * otherwise we report revoked or bad_stateid status.
> */
> - if (typemask & NFS4_REVOKED_DELEG_STID)
> + if (statusmask & SC_STATUS_REVOKED)
> return_revoked = true;
> - else if (typemask & NFS4_DELEG_STID)
> - typemask |= NFS4_REVOKED_DELEG_STID;
> + if (typemask & SC_TYPE_DELEG)
> + /* Always allow REVOKED for DELEG so we can
> + * retturn the appropriate error.
> + */
> + statusmask |= SC_STATUS_REVOKED;
>
> if (ZERO_STATEID(stateid) || ONE_STATEID(stateid) ||
> CLOSE_STATEID(stateid))
> @@ -6460,14 +6454,12 @@ nfsd4_lookup_stateid(struct nfsd4_compound_state *cstate,
> }
> if (status)
> return status;
> - stid = find_stateid_by_type(cstate->clp, stateid, typemask);
> + stid = find_stateid_by_type(cstate->clp, stateid, typemask, statusmask);
> if (!stid)
> return nfserr_bad_stateid;
> - if ((stid->sc_type == NFS4_REVOKED_DELEG_STID) && !return_revoked) {
> + if ((stid->sc_status & SC_STATUS_REVOKED) && !return_revoked) {
> nfs4_put_stid(stid);
> - if (cstate->minorversion)
> - return nfserr_deleg_revoked;
> - return nfserr_bad_stateid;
> + return nfserr_deleg_revoked;
> }
> *s = stid;
> return nfs_ok;
> @@ -6478,17 +6470,17 @@ nfs4_find_file(struct nfs4_stid *s, int flags)
> {
> struct nfsd_file *ret = NULL;
>
> - if (!s)
> + if (!s || s->sc_status)
> return NULL;
>
> switch (s->sc_type) {
> - case NFS4_DELEG_STID:
> + case SC_TYPE_DELEG:
> spin_lock(&s->sc_file->fi_lock);
> ret = nfsd_file_get(s->sc_file->fi_deleg_file);
> spin_unlock(&s->sc_file->fi_lock);
> break;
> - case NFS4_OPEN_STID:
> - case NFS4_LOCK_STID:
> + case SC_TYPE_OPEN:
> + case SC_TYPE_LOCK:
> if (flags & RD_STATE)
> ret = find_readable_file(s->sc_file);
> else
> @@ -6601,7 +6593,8 @@ static __be32 find_cpntf_state(struct nfsd_net *nn, stateid_t *st,
> goto out;
>
> *stid = find_stateid_by_type(found, &cps->cp_p_stateid,
> - NFS4_DELEG_STID|NFS4_OPEN_STID|NFS4_LOCK_STID);
> + SC_TYPE_DELEG|SC_TYPE_OPEN|SC_TYPE_LOCK,
> + 0);
> if (*stid)
> status = nfs_ok;
> else
> @@ -6658,8 +6651,8 @@ nfs4_preprocess_stateid_op(struct svc_rqst *rqstp,
> }
>
> status = nfsd4_lookup_stateid(cstate, stateid,
> - NFS4_DELEG_STID|NFS4_OPEN_STID|NFS4_LOCK_STID,
> - &s, nn);
> + SC_TYPE_DELEG|SC_TYPE_OPEN|SC_TYPE_LOCK,
> + 0, &s, nn);
> if (status == nfserr_bad_stateid)
> status = find_cpntf_state(nn, stateid, &s);
> if (status)
> @@ -6670,16 +6663,13 @@ nfs4_preprocess_stateid_op(struct svc_rqst *rqstp,
> goto out;
>
> switch (s->sc_type) {
> - case NFS4_DELEG_STID:
> + case SC_TYPE_DELEG:
> status = nfs4_check_delegmode(delegstateid(s), flags);
> break;
> - case NFS4_OPEN_STID:
> - case NFS4_LOCK_STID:
> + case SC_TYPE_OPEN:
> + case SC_TYPE_LOCK:
> status = nfs4_check_olstateid(openlockstateid(s), flags);
> break;
> - default:
> - status = nfserr_bad_stateid;
> - break;
> }
> if (status)
> goto out;
> @@ -6758,33 +6748,34 @@ nfsd4_free_stateid(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
>
> spin_lock(&cl->cl_lock);
> s = find_stateid_locked(cl, stateid);
> - if (!s)
> + if (!s || s->sc_status & SC_STATUS_CLOSED)
> goto out_unlock;
> spin_lock(&s->sc_lock);
> switch (s->sc_type) {
> - case NFS4_DELEG_STID:
> + case SC_TYPE_DELEG:
> + if (s->sc_status & SC_STATUS_REVOKED) {
> + spin_unlock(&s->sc_lock);
> + dp = delegstateid(s);
> + list_del_init(&dp->dl_recall_lru);
> + spin_unlock(&cl->cl_lock);
> + nfs4_put_stid(s);
> + ret = nfs_ok;
> + goto out;
> + }
> ret = nfserr_locks_held;
> break;
> - case NFS4_OPEN_STID:
> + case SC_TYPE_OPEN:
> ret = check_stateid_generation(stateid, &s->sc_stateid, 1);
> if (ret)
> break;
> ret = nfserr_locks_held;
> break;
> - case NFS4_LOCK_STID:
> + case SC_TYPE_LOCK:
> spin_unlock(&s->sc_lock);
> refcount_inc(&s->sc_count);
> spin_unlock(&cl->cl_lock);
> ret = nfsd4_free_lock_stateid(stateid, s);
> goto out;
> - case NFS4_REVOKED_DELEG_STID:
> - spin_unlock(&s->sc_lock);
> - dp = delegstateid(s);
> - list_del_init(&dp->dl_recall_lru);
> - spin_unlock(&cl->cl_lock);
> - nfs4_put_stid(s);
> - ret = nfs_ok;
> - goto out;
> /* Default falls through and returns nfserr_bad_stateid */
> }
> spin_unlock(&s->sc_lock);
> @@ -6827,6 +6818,7 @@ static __be32 nfs4_seqid_op_checks(struct nfsd4_compound_state *cstate, stateid_
> * @seqid: seqid (provided by client)
> * @stateid: stateid (provided by client)
> * @typemask: mask of allowable types for this operation
> + * @statusmask: mask of allowed states: 0 or STID_CLOSED
> * @stpp: return pointer for the stateid found
> * @nn: net namespace for request
> *
> @@ -6836,7 +6828,8 @@ static __be32 nfs4_seqid_op_checks(struct nfsd4_compound_state *cstate, stateid_
> */
> static __be32
> nfs4_preprocess_seqid_op(struct nfsd4_compound_state *cstate, u32 seqid,
> - stateid_t *stateid, char typemask,
> + stateid_t *stateid,
> + unsigned short typemask, unsigned short statusmask,
> struct nfs4_ol_stateid **stpp,
> struct nfsd_net *nn)
> {
> @@ -6847,7 +6840,8 @@ nfs4_preprocess_seqid_op(struct nfsd4_compound_state *cstate, u32 seqid,
> trace_nfsd_preprocess(seqid, stateid);
>
> *stpp = NULL;
> - status = nfsd4_lookup_stateid(cstate, stateid, typemask, &s, nn);
> + status = nfsd4_lookup_stateid(cstate, stateid,
> + typemask, statusmask, &s, nn);
> if (status)
> return status;
> stp = openlockstateid(s);
> @@ -6869,7 +6863,7 @@ static __be32 nfs4_preprocess_confirmed_seqid_op(struct nfsd4_compound_state *cs
> struct nfs4_ol_stateid *stp;
>
> status = nfs4_preprocess_seqid_op(cstate, seqid, stateid,
> - NFS4_OPEN_STID, &stp, nn);
> + SC_TYPE_OPEN, 0, &stp, nn);
> if (status)
> return status;
> oo = openowner(stp->st_stateowner);
> @@ -6900,8 +6894,8 @@ nfsd4_open_confirm(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
> return status;
>
> status = nfs4_preprocess_seqid_op(cstate,
> - oc->oc_seqid, &oc->oc_req_stateid,
> - NFS4_OPEN_STID, &stp, nn);
> + oc->oc_seqid, &oc->oc_req_stateid,
> + SC_TYPE_OPEN, 0, &stp, nn);
> if (status)
> goto out;
> oo = openowner(stp->st_stateowner);
> @@ -7031,18 +7025,20 @@ nfsd4_close(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
> struct net *net = SVC_NET(rqstp);
> struct nfsd_net *nn = net_generic(net, nfsd_net_id);
>
> - dprintk("NFSD: nfsd4_close on file %pd\n",
> + dprintk("NFSD: nfsd4_close on file %pd\n",
> cstate->current_fh.fh_dentry);
>
> status = nfs4_preprocess_seqid_op(cstate, close->cl_seqid,
> - &close->cl_stateid,
> - NFS4_OPEN_STID|NFS4_CLOSED_STID,
> - &stp, nn);
> + &close->cl_stateid,
> + SC_TYPE_OPEN, SC_STATUS_CLOSED,
> + &stp, nn);
> nfsd4_bump_seqid(cstate, status);
> if (status)
> - goto out;
> + goto out;
>
> - stp->st_stid.sc_type = NFS4_CLOSED_STID;
> + spin_lock(&stp->st_stid.sc_client->cl_lock);
> + stp->st_stid.sc_status |= SC_STATUS_CLOSED;
> + spin_unlock(&stp->st_stid.sc_client->cl_lock);
>
> /*
> * Technically we don't _really_ have to increment or copy it, since
> @@ -7084,7 +7080,7 @@ nfsd4_delegreturn(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
> if ((status = fh_verify(rqstp, &cstate->current_fh, S_IFREG, 0)))
> return status;
>
> - status = nfsd4_lookup_stateid(cstate, stateid, NFS4_DELEG_STID, &s, nn);
> + status = nfsd4_lookup_stateid(cstate, stateid, SC_TYPE_DELEG, 0, &s, nn);
> if (status)
> goto out;
> dp = delegstateid(s);
> @@ -7351,7 +7347,7 @@ init_lock_stateid(struct nfs4_ol_stateid *stp, struct nfs4_lockowner *lo,
> if (retstp)
> goto out_found;
> refcount_inc(&stp->st_stid.sc_count);
> - stp->st_stid.sc_type = NFS4_LOCK_STID;
> + stp->st_stid.sc_type = SC_TYPE_LOCK;
> stp->st_stateowner = nfs4_get_stateowner(&lo->lo_owner);
> get_nfs4_file(fp);
> stp->st_stid.sc_file = fp;
> @@ -7538,9 +7534,10 @@ nfsd4_lock(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
> &lock_stp, &new);
> } else {
> status = nfs4_preprocess_seqid_op(cstate,
> - lock->lk_old_lock_seqid,
> - &lock->lk_old_lock_stateid,
> - NFS4_LOCK_STID, &lock_stp, nn);
> + lock->lk_old_lock_seqid,
> + &lock->lk_old_lock_stateid,
> + SC_TYPE_LOCK, 0, &lock_stp,
> + nn);
> }
> if (status)
> goto out;
> @@ -7853,8 +7850,8 @@ nfsd4_locku(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
> return nfserr_inval;
>
> status = nfs4_preprocess_seqid_op(cstate, locku->lu_seqid,
> - &locku->lu_stateid, NFS4_LOCK_STID,
> - &stp, nn);
> + &locku->lu_stateid, SC_TYPE_LOCK, 0,
> + &stp, nn);
> if (status)
> goto out;
> nf = find_any_file(stp->st_stid.sc_file);
> @@ -8292,7 +8289,7 @@ nfs4_state_shutdown_net(struct net *net)
> spin_lock(&state_lock);
> list_for_each_safe(pos, next, &nn->del_recall_lru) {
> dp = list_entry (pos, struct nfs4_delegation, dl_recall_lru);
> - unhash_delegation_locked(dp, NFS4_CLOSED_DELEG_STID);
> + unhash_delegation_locked(dp, SC_STATUS_CLOSED);
> list_add(&dp->dl_recall_lru, &reaplist);
> }
> spin_unlock(&state_lock);
> diff --git a/fs/nfsd/state.h b/fs/nfsd/state.h
> index 41bdc913fa71..ffc8920d0558 100644
> --- a/fs/nfsd/state.h
> +++ b/fs/nfsd/state.h
> @@ -88,17 +88,33 @@ struct nfsd4_callback_ops {
> */
> struct nfs4_stid {
> refcount_t sc_count;
> -#define NFS4_OPEN_STID 1
> -#define NFS4_LOCK_STID 2
> -#define NFS4_DELEG_STID 4
> -/* For an open stateid kept around *only* to process close replays: */
> -#define NFS4_CLOSED_STID 8
> +
> + /* A new stateid is added to the cl_stateids idr early before it
> + * is fully initialised. Its sc_type is then zero. After
> + * initialisation the sc_type it set under cl_lock, and then
> + * never changes.
> + */
> +#define SC_TYPE_OPEN BIT(0)
> +#define SC_TYPE_LOCK BIT(1)
> +#define SC_TYPE_DELEG BIT(2)
> +#define SC_TYPE_LAYOUT BIT(3)
> + unsigned short sc_type;
> +
> +/* state_lock protects sc_status for delegation stateids.
> + * ->cl_lock protects sc_status for open and lock stateids.
> + * ->st_mutex also protect sc_status for open stateids.
> + * ->ls_lock protects sc_status for layout stateids.
> + */
> +/*
> + * For an open stateid kept around *only* to process close replays.
> + * For deleg stateid, kept in idr until last reference is dropped.
> + */
> +#define SC_STATUS_CLOSED BIT(0)
> /* For a deleg stateid kept around only to process free_stateid's: */
> -#define NFS4_REVOKED_DELEG_STID 16
> -#define NFS4_CLOSED_DELEG_STID 32
> -#define NFS4_LAYOUT_STID 64
> +#define SC_STATUS_REVOKED BIT(1)
> + unsigned short sc_status;
> +
> struct list_head sc_cp_list;
> - unsigned char sc_type;
> stateid_t sc_stateid;
> spinlock_t sc_lock;
> struct nfs4_client *sc_client;
> @@ -672,15 +688,15 @@ extern __be32 nfs4_preprocess_stateid_op(struct svc_rqst *rqstp,
> stateid_t *stateid, int flags, struct nfsd_file **filp,
> struct nfs4_stid **cstid);
> __be32 nfsd4_lookup_stateid(struct nfsd4_compound_state *cstate,
> - stateid_t *stateid, unsigned char typemask,
> - struct nfs4_stid **s, struct nfsd_net *nn);
> + stateid_t *stateid, unsigned short typemask,
> + unsigned short statusmask,
> + struct nfs4_stid **s, struct nfsd_net *nn);
> struct nfs4_stid *nfs4_alloc_stid(struct nfs4_client *cl, struct kmem_cache *slab,
> void (*sc_free)(struct nfs4_stid *));
> int nfs4_init_copy_state(struct nfsd_net *nn, struct nfsd4_copy *copy);
> void nfs4_free_copy_state(struct nfsd4_copy *copy);
> struct nfs4_cpntf_state *nfs4_alloc_init_cpntf_state(struct nfsd_net *nn,
> struct nfs4_stid *p_stid);
> -void nfs4_unhash_stid(struct nfs4_stid *s);
> void nfs4_put_stid(struct nfs4_stid *s);
> void nfs4_inc_and_copy_stateid(stateid_t *dst, struct nfs4_stid *stid);
> void nfs4_remove_reclaim_record(struct nfs4_client_reclaim *, struct nfsd_net *);
> diff --git a/fs/nfsd/trace.h b/fs/nfsd/trace.h
> index d1e8cf079b0f..fe08ca18b647 100644
> --- a/fs/nfsd/trace.h
> +++ b/fs/nfsd/trace.h
> @@ -641,23 +641,17 @@ DEFINE_EVENT(nfsd_stateseqid_class, nfsd_##name, \
> DEFINE_STATESEQID_EVENT(preprocess);
> DEFINE_STATESEQID_EVENT(open_confirm);
>
> -TRACE_DEFINE_ENUM(NFS4_OPEN_STID);
> -TRACE_DEFINE_ENUM(NFS4_LOCK_STID);
> -TRACE_DEFINE_ENUM(NFS4_DELEG_STID);
> -TRACE_DEFINE_ENUM(NFS4_CLOSED_STID);
> -TRACE_DEFINE_ENUM(NFS4_REVOKED_DELEG_STID);
> -TRACE_DEFINE_ENUM(NFS4_CLOSED_DELEG_STID);
> -TRACE_DEFINE_ENUM(NFS4_LAYOUT_STID);
> -
> #define show_stid_type(x) \
> __print_flags(x, "|", \
> - { NFS4_OPEN_STID, "OPEN" }, \
> - { NFS4_LOCK_STID, "LOCK" }, \
> - { NFS4_DELEG_STID, "DELEG" }, \
> - { NFS4_CLOSED_STID, "CLOSED" }, \
> - { NFS4_REVOKED_DELEG_STID, "REVOKED" }, \
> - { NFS4_CLOSED_DELEG_STID, "CLOSED_DELEG" }, \
> - { NFS4_LAYOUT_STID, "LAYOUT" })
> + { SC_TYPE_OPEN, "OPEN" }, \
> + { SC_TYPE_LOCK, "LOCK" }, \
> + { SC_TYPE_DELEG, "DELEG" }, \
> + { SC_TYPE_LAYOUT, "LAYOUT" })
> +
> +#define show_stid_status(x) \
> + __print_flags(x, "|", \
> + { SC_STATUS_CLOSED, "CLOSED" }, \
> + { SC_STATUS_REVOKED, "REVOKED" }) \
>
> DECLARE_EVENT_CLASS(nfsd_stid_class,
> TP_PROTO(
> @@ -666,6 +660,7 @@ DECLARE_EVENT_CLASS(nfsd_stid_class,
> TP_ARGS(stid),
> TP_STRUCT__entry(
> __field(unsigned long, sc_type)
> + __field(unsigned long, sc_status)
> __field(int, sc_count)
> __field(u32, cl_boot)
> __field(u32, cl_id)
> @@ -676,16 +671,18 @@ DECLARE_EVENT_CLASS(nfsd_stid_class,
> const stateid_t *stp = &stid->sc_stateid;
>
> __entry->sc_type = stid->sc_type;
> + __entry->sc_status = stid->sc_status;
> __entry->sc_count = refcount_read(&stid->sc_count);
> __entry->cl_boot = stp->si_opaque.so_clid.cl_boot;
> __entry->cl_id = stp->si_opaque.so_clid.cl_id;
> __entry->si_id = stp->si_opaque.so_id;
> __entry->si_generation = stp->si_generation;
> ),
> - TP_printk("client %08x:%08x stateid %08x:%08x ref=%d type=%s",
> + TP_printk("client %08x:%08x stateid %08x:%08x ref=%d type=%s state=%s",
> __entry->cl_boot, __entry->cl_id,
> __entry->si_id, __entry->si_generation,
> - __entry->sc_count, show_stid_type(__entry->sc_type)
> + __entry->sc_count, show_stid_type(__entry->sc_type),
> + show_stid_status(__entry->sc_status)
> )
> );
>
> --
> 2.43.0
>
--
Chuck Lever
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH 06/13] nfsd: prepare for supporting admin-revocation of state
2024-01-29 3:29 [PATCH 00/13 v4] nfsd: support admin-revocation of v4 state NeilBrown
` (4 preceding siblings ...)
2024-01-29 3:29 ` [PATCH 05/13] nfsd: split sc_status out of sc_type NeilBrown
@ 2024-01-29 3:29 ` NeilBrown
2024-01-29 12:22 ` Jeff Layton
2024-01-29 3:29 ` [PATCH 07/13] nfsd: allow state with no file to appear in /proc/fs/nfsd/clients/*/states NeilBrown
` (6 subsequent siblings)
12 siblings, 1 reply; 27+ messages in thread
From: NeilBrown @ 2024-01-29 3:29 UTC (permalink / raw)
To: Chuck Lever, Jeff Layton
Cc: linux-nfs, Olga Kornievskaia, Dai Ngo, Tom Talpey,
Christoph Hellwig, Tom Haynes
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 SC_STATUS_ADMIN_REVOKED
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 <neilb@suse.de>
---
fs/nfsd/nfs4state.c | 85 ++++++++++++++++++++++++++++++++++++++++++++-
fs/nfsd/nfsctl.c | 1 +
fs/nfsd/nfsd.h | 1 +
fs/nfsd/state.h | 10 ++++++
fs/nfsd/trace.h | 3 +-
5 files changed, 98 insertions(+), 2 deletions(-)
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index 6bccdd0af814..8db224906864 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -1210,6 +1210,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 & SC_STATUS_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);
@@ -1529,6 +1531,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 & SC_STATUS_ADMIN_REVOKED)
+ atomic_dec(&s->sc_client->cl_admin_revoked);
list_add(&stp->st_locks, reaplist);
}
@@ -1674,6 +1678,68 @@ 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 - revoke all nfsv4 states associated with given filesystem
+ * @net - used to identify instance of nfsd (there is one per net namespace)
+ * @sb - super_block used to identify target filesystem
+ *
+ * All nfs4 states (open, lock, delegation, layout) held by the server instance
+ * and associated with a file on the given filesystem will be revoked resulting
+ * in any files being closed and so all references from nfsd to the filesystem
+ * being released. Thus nfsd will no longer prevent the filesystem from being
+ * unmounted.
+ *
+ * The clients which own the states will subsequently being notified that the
+ * states have been "admin-revoked".
+ */
+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)
{
@@ -2545,6 +2611,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;
@@ -4058,6 +4126,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);
@@ -4546,7 +4616,9 @@ nfsd4_verify_open_stid(struct nfs4_stid *s)
{
__be32 ret = nfs_ok;
- if (s->sc_status & SC_STATUS_REVOKED)
+ if (s->sc_status & SC_STATUS_ADMIN_REVOKED)
+ ret = nfserr_admin_revoked;
+ else if (s->sc_status & SC_STATUS_REVOKED)
ret = nfserr_deleg_revoked;
else if (s->sc_status & SC_STATUS_CLOSED)
ret = nfserr_bad_stateid;
@@ -5137,6 +5209,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 & SC_STATUS_ADMIN_REVOKED) {
+ nfs4_put_stid(&deleg->dl_stid);
+ status = nfserr_admin_revoked;
+ goto out;
+ }
if (deleg->dl_stid.sc_status & SC_STATUS_REVOKED) {
nfs4_put_stid(&deleg->dl_stid);
status = nfserr_deleg_revoked;
@@ -6443,6 +6520,8 @@ nfsd4_lookup_stateid(struct nfsd4_compound_state *cstate,
*/
statusmask |= SC_STATUS_REVOKED;
+ statusmask |= SC_STATUS_ADMIN_REVOKED;
+
if (ZERO_STATEID(stateid) || ONE_STATEID(stateid) ||
CLOSE_STATEID(stateid))
return nfserr_bad_stateid;
@@ -6461,6 +6540,10 @@ nfsd4_lookup_stateid(struct nfsd4_compound_state *cstate,
nfs4_put_stid(stid);
return nfserr_deleg_revoked;
}
+ if (stid->sc_status & SC_STATUS_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 f206ca32e7f5..4bae65e8d28a 100644
--- a/fs/nfsd/nfsctl.c
+++ b/fs/nfsd/nfsctl.c
@@ -281,6 +281,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 304e9728b929..9a86fe8a39ef 100644
--- a/fs/nfsd/nfsd.h
+++ b/fs/nfsd/nfsd.h
@@ -274,6 +274,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 ffc8920d0558..7fa83265ad9a 100644
--- a/fs/nfsd/state.h
+++ b/fs/nfsd/state.h
@@ -112,6 +112,7 @@ struct nfs4_stid {
#define SC_STATUS_CLOSED BIT(0)
/* For a deleg stateid kept around only to process free_stateid's: */
#define SC_STATUS_REVOKED BIT(1)
+#define SC_STATUS_ADMIN_REVOKED BIT(2)
unsigned short sc_status;
struct list_head sc_cp_list;
@@ -367,6 +368,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;
@@ -730,6 +732,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 fe08ca18b647..5c58da9f86b7 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, "|", \
{ SC_STATUS_CLOSED, "CLOSED" }, \
- { SC_STATUS_REVOKED, "REVOKED" }) \
+ { SC_STATUS_REVOKED, "REVOKED" }, \
+ { SC_STATUS_ADMIN_REVOKED, "ADMIN_REVOKED" })
DECLARE_EVENT_CLASS(nfsd_stid_class,
TP_PROTO(
--
2.43.0
^ permalink raw reply related [flat|nested] 27+ messages in thread* Re: [PATCH 06/13] nfsd: prepare for supporting admin-revocation of state
2024-01-29 3:29 ` [PATCH 06/13] nfsd: prepare for supporting admin-revocation of state NeilBrown
@ 2024-01-29 12:22 ` Jeff Layton
0 siblings, 0 replies; 27+ messages in thread
From: Jeff Layton @ 2024-01-29 12:22 UTC (permalink / raw)
To: NeilBrown, Chuck Lever
Cc: linux-nfs, Olga Kornievskaia, Dai Ngo, Tom Talpey,
Christoph Hellwig, Tom Haynes
On Mon, 2024-01-29 at 14:29 +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 SC_STATUS_ADMIN_REVOKED
> 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 <neilb@suse.de>
> ---
> fs/nfsd/nfs4state.c | 85 ++++++++++++++++++++++++++++++++++++++++++++-
> fs/nfsd/nfsctl.c | 1 +
> fs/nfsd/nfsd.h | 1 +
> fs/nfsd/state.h | 10 ++++++
> fs/nfsd/trace.h | 3 +-
> 5 files changed, 98 insertions(+), 2 deletions(-)
>
> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
> index 6bccdd0af814..8db224906864 100644
> --- a/fs/nfsd/nfs4state.c
> +++ b/fs/nfsd/nfs4state.c
> @@ -1210,6 +1210,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 & SC_STATUS_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);
> @@ -1529,6 +1531,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 & SC_STATUS_ADMIN_REVOKED)
> + atomic_dec(&s->sc_client->cl_admin_revoked);
> list_add(&stp->st_locks, reaplist);
> }
>
> @@ -1674,6 +1678,68 @@ 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 - revoke all nfsv4 states associated with given filesystem
> + * @net - used to identify instance of nfsd (there is one per net namespace)
> + * @sb - super_block used to identify target filesystem
> + *
> + * All nfs4 states (open, lock, delegation, layout) held by the server instance
> + * and associated with a file on the given filesystem will be revoked resulting
> + * in any files being closed and so all references from nfsd to the filesystem
> + * being released. Thus nfsd will no longer prevent the filesystem from being
> + * unmounted.
> + *
> + * The clients which own the states will subsequently being notified that the
> + * states have been "admin-revoked".
> + */
> +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)
> {
> @@ -2545,6 +2611,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;
> @@ -4058,6 +4126,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);
> @@ -4546,7 +4616,9 @@ nfsd4_verify_open_stid(struct nfs4_stid *s)
> {
> __be32 ret = nfs_ok;
>
> - if (s->sc_status & SC_STATUS_REVOKED)
> + if (s->sc_status & SC_STATUS_ADMIN_REVOKED)
> + ret = nfserr_admin_revoked;
> + else if (s->sc_status & SC_STATUS_REVOKED)
> ret = nfserr_deleg_revoked;
> else if (s->sc_status & SC_STATUS_CLOSED)
> ret = nfserr_bad_stateid;
> @@ -5137,6 +5209,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 & SC_STATUS_ADMIN_REVOKED) {
> + nfs4_put_stid(&deleg->dl_stid);
> + status = nfserr_admin_revoked;
> + goto out;
> + }
> if (deleg->dl_stid.sc_status & SC_STATUS_REVOKED) {
> nfs4_put_stid(&deleg->dl_stid);
> status = nfserr_deleg_revoked;
> @@ -6443,6 +6520,8 @@ nfsd4_lookup_stateid(struct nfsd4_compound_state *cstate,
> */
> statusmask |= SC_STATUS_REVOKED;
>
> + statusmask |= SC_STATUS_ADMIN_REVOKED;
> +
> if (ZERO_STATEID(stateid) || ONE_STATEID(stateid) ||
> CLOSE_STATEID(stateid))
> return nfserr_bad_stateid;
> @@ -6461,6 +6540,10 @@ nfsd4_lookup_stateid(struct nfsd4_compound_state *cstate,
> nfs4_put_stid(stid);
> return nfserr_deleg_revoked;
> }
> + if (stid->sc_status & SC_STATUS_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 f206ca32e7f5..4bae65e8d28a 100644
> --- a/fs/nfsd/nfsctl.c
> +++ b/fs/nfsd/nfsctl.c
> @@ -281,6 +281,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 304e9728b929..9a86fe8a39ef 100644
> --- a/fs/nfsd/nfsd.h
> +++ b/fs/nfsd/nfsd.h
> @@ -274,6 +274,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 ffc8920d0558..7fa83265ad9a 100644
> --- a/fs/nfsd/state.h
> +++ b/fs/nfsd/state.h
> @@ -112,6 +112,7 @@ struct nfs4_stid {
> #define SC_STATUS_CLOSED BIT(0)
> /* For a deleg stateid kept around only to process free_stateid's: */
> #define SC_STATUS_REVOKED BIT(1)
> +#define SC_STATUS_ADMIN_REVOKED BIT(2)
> unsigned short sc_status;
>
> struct list_head sc_cp_list;
> @@ -367,6 +368,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;
> @@ -730,6 +732,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 fe08ca18b647..5c58da9f86b7 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, "|", \
> { SC_STATUS_CLOSED, "CLOSED" }, \
> - { SC_STATUS_REVOKED, "REVOKED" }) \
> + { SC_STATUS_REVOKED, "REVOKED" }, \
> + { SC_STATUS_ADMIN_REVOKED, "ADMIN_REVOKED" })
>
> DECLARE_EVENT_CLASS(nfsd_stid_class,
> TP_PROTO(
Reviewed-by: Jeff Layton <jlayton@kernel.org>
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH 07/13] nfsd: allow state with no file to appear in /proc/fs/nfsd/clients/*/states
2024-01-29 3:29 [PATCH 00/13 v4] nfsd: support admin-revocation of v4 state NeilBrown
` (5 preceding siblings ...)
2024-01-29 3:29 ` [PATCH 06/13] nfsd: prepare for supporting admin-revocation of state NeilBrown
@ 2024-01-29 3:29 ` NeilBrown
2024-01-29 12:23 ` Jeff Layton
2024-01-29 3:29 ` [PATCH 08/13] nfsd: report in /proc/fs/nfsd/clients/*/states when state is admin-revoke NeilBrown
` (5 subsequent siblings)
12 siblings, 1 reply; 27+ messages in thread
From: NeilBrown @ 2024-01-29 3:29 UTC (permalink / raw)
To: Chuck Lever, Jeff Layton
Cc: linux-nfs, Olga Kornievskaia, Dai Ngo, Tom Talpey,
Christoph Hellwig, Tom Haynes
Change the "show" functions to show some content even if a file cannot
be found. This is the case for admin-revoked state.
This is primarily useful for debugging - to ensure states are being
removed eventually.
So change several seq_printf() to seq_puts(). Some of these are needed
to keep checkpatch happy. Others were done for consistency.
Signed-off-by: NeilBrown <neilb@suse.de>
---
fs/nfsd/nfs4state.c | 118 ++++++++++++++++++++++----------------------
1 file changed, 58 insertions(+), 60 deletions(-)
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index 8db224906864..ef4ec23f7c0d 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -2554,9 +2554,9 @@ static struct nfs4_client *get_nfsdfs_clp(struct inode *inode)
static void seq_quote_mem(struct seq_file *m, char *data, int len)
{
- seq_printf(m, "\"");
+ seq_puts(m, "\"");
seq_escape_mem(m, data, len, ESCAPE_HEX | ESCAPE_NAP | ESCAPE_APPEND, "\"\\");
- seq_printf(m, "\"");
+ seq_puts(m, "\"");
}
static const char *cb_state2str(int state)
@@ -2597,14 +2597,14 @@ static int client_info_show(struct seq_file *m, void *v)
seq_puts(m, "status: unconfirmed\n");
seq_printf(m, "seconds from last renew: %lld\n",
ktime_get_boottime_seconds() - clp->cl_time);
- seq_printf(m, "name: ");
+ seq_puts(m, "name: ");
seq_quote_mem(m, clp->cl_name.data, clp->cl_name.len);
seq_printf(m, "\nminor version: %d\n", clp->cl_minorversion);
if (clp->cl_nii_domain.data) {
- seq_printf(m, "Implementation domain: ");
+ seq_puts(m, "Implementation domain: ");
seq_quote_mem(m, clp->cl_nii_domain.data,
clp->cl_nii_domain.len);
- seq_printf(m, "\nImplementation name: ");
+ seq_puts(m, "\nImplementation name: ");
seq_quote_mem(m, clp->cl_nii_name.data, clp->cl_nii_name.len);
seq_printf(m, "\nImplementation time: [%lld, %ld]\n",
clp->cl_nii_time.tv_sec, clp->cl_nii_time.tv_nsec);
@@ -2671,7 +2671,7 @@ static void nfs4_show_superblock(struct seq_file *s, struct nfsd_file *f)
static void nfs4_show_owner(struct seq_file *s, struct nfs4_stateowner *oo)
{
- seq_printf(s, "owner: ");
+ seq_puts(s, "owner: ");
seq_quote_mem(s, oo->so_owner.data, oo->so_owner.len);
}
@@ -2689,20 +2689,13 @@ 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 != SC_TYPE_OPEN && st->sc_type != SC_TYPE_LOCK)
- 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, "- ");
+ seq_puts(s, "- ");
nfs4_show_stateid(s, &st->sc_stateid);
- seq_printf(s, ": { type: open, ");
+ seq_puts(s, ": { type: open, ");
access = bmap_to_share_mode(ols->st_access_bmap);
deny = bmap_to_share_mode(ols->st_deny_bmap);
@@ -2714,14 +2707,17 @@ 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, ", ");
- nfs4_show_owner(s, oo);
- seq_printf(s, " }\n");
-out:
+ 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);
+ seq_puts(s, " }\n");
return 0;
}
@@ -2735,30 +2731,29 @@ 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, "- ");
+ seq_puts(s, "- ");
nfs4_show_stateid(s, &st->sc_stateid);
- seq_printf(s, ": { type: lock, ");
+ seq_puts(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);
- seq_printf(s, " }\n");
-out:
+ seq_puts(s, " }\n");
spin_unlock(&nf->fi_lock);
return 0;
}
@@ -2771,25 +2766,25 @@ 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, "- ");
+ seq_puts(s, "- ");
nfs4_show_stateid(s, &st->sc_stateid);
- seq_printf(s, ": { type: deleg, ");
+ seq_puts(s, ": { type: deleg, ");
- 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);
+ }
+ seq_puts(s, " }\n");
spin_unlock(&nf->fi_lock);
return 0;
}
@@ -2802,16 +2797,19 @@ static int nfs4_show_layout(struct seq_file *s, struct nfs4_stid *st)
ls = container_of(st, struct nfs4_layout_stateid, ls_stid);
file = ls->ls_file;
- seq_printf(s, "- ");
+ seq_puts(s, "- ");
nfs4_show_stateid(s, &st->sc_stateid);
- seq_printf(s, ": { type: layout, ");
+ seq_puts(s, ": { type: layout");
/* XXX: What else would be useful? */
- nfs4_show_superblock(s, file);
- seq_printf(s, ", ");
- nfs4_show_fname(s, file);
- seq_printf(s, " }\n");
+ if (file) {
+ seq_puts(s, ", ");
+ nfs4_show_superblock(s, file);
+ seq_puts(s, ", ");
+ nfs4_show_fname(s, file);
+ }
+ seq_puts(s, " }\n");
return 0;
}
--
2.43.0
^ permalink raw reply related [flat|nested] 27+ messages in thread* Re: [PATCH 07/13] nfsd: allow state with no file to appear in /proc/fs/nfsd/clients/*/states
2024-01-29 3:29 ` [PATCH 07/13] nfsd: allow state with no file to appear in /proc/fs/nfsd/clients/*/states NeilBrown
@ 2024-01-29 12:23 ` Jeff Layton
0 siblings, 0 replies; 27+ messages in thread
From: Jeff Layton @ 2024-01-29 12:23 UTC (permalink / raw)
To: NeilBrown, Chuck Lever
Cc: linux-nfs, Olga Kornievskaia, Dai Ngo, Tom Talpey,
Christoph Hellwig, Tom Haynes
On Mon, 2024-01-29 at 14:29 +1100, NeilBrown wrote:
> Change the "show" functions to show some content even if a file cannot
> be found. This is the case for admin-revoked state.
> This is primarily useful for debugging - to ensure states are being
> removed eventually.
>
> So change several seq_printf() to seq_puts(). Some of these are needed
> to keep checkpatch happy. Others were done for consistency.
>
> Signed-off-by: NeilBrown <neilb@suse.de>
> ---
> fs/nfsd/nfs4state.c | 118 ++++++++++++++++++++++----------------------
> 1 file changed, 58 insertions(+), 60 deletions(-)
>
> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
> index 8db224906864..ef4ec23f7c0d 100644
> --- a/fs/nfsd/nfs4state.c
> +++ b/fs/nfsd/nfs4state.c
> @@ -2554,9 +2554,9 @@ static struct nfs4_client *get_nfsdfs_clp(struct inode *inode)
>
> static void seq_quote_mem(struct seq_file *m, char *data, int len)
> {
> - seq_printf(m, "\"");
> + seq_puts(m, "\"");
> seq_escape_mem(m, data, len, ESCAPE_HEX | ESCAPE_NAP | ESCAPE_APPEND, "\"\\");
> - seq_printf(m, "\"");
> + seq_puts(m, "\"");
> }
>
> static const char *cb_state2str(int state)
> @@ -2597,14 +2597,14 @@ static int client_info_show(struct seq_file *m, void *v)
> seq_puts(m, "status: unconfirmed\n");
> seq_printf(m, "seconds from last renew: %lld\n",
> ktime_get_boottime_seconds() - clp->cl_time);
> - seq_printf(m, "name: ");
> + seq_puts(m, "name: ");
> seq_quote_mem(m, clp->cl_name.data, clp->cl_name.len);
> seq_printf(m, "\nminor version: %d\n", clp->cl_minorversion);
> if (clp->cl_nii_domain.data) {
> - seq_printf(m, "Implementation domain: ");
> + seq_puts(m, "Implementation domain: ");
> seq_quote_mem(m, clp->cl_nii_domain.data,
> clp->cl_nii_domain.len);
> - seq_printf(m, "\nImplementation name: ");
> + seq_puts(m, "\nImplementation name: ");
> seq_quote_mem(m, clp->cl_nii_name.data, clp->cl_nii_name.len);
> seq_printf(m, "\nImplementation time: [%lld, %ld]\n",
> clp->cl_nii_time.tv_sec, clp->cl_nii_time.tv_nsec);
> @@ -2671,7 +2671,7 @@ static void nfs4_show_superblock(struct seq_file *s, struct nfsd_file *f)
>
> static void nfs4_show_owner(struct seq_file *s, struct nfs4_stateowner *oo)
> {
> - seq_printf(s, "owner: ");
> + seq_puts(s, "owner: ");
> seq_quote_mem(s, oo->so_owner.data, oo->so_owner.len);
> }
>
> @@ -2689,20 +2689,13 @@ 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 != SC_TYPE_OPEN && st->sc_type != SC_TYPE_LOCK)
> - 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, "- ");
> + seq_puts(s, "- ");
> nfs4_show_stateid(s, &st->sc_stateid);
> - seq_printf(s, ": { type: open, ");
> + seq_puts(s, ": { type: open, ");
>
> access = bmap_to_share_mode(ols->st_access_bmap);
> deny = bmap_to_share_mode(ols->st_deny_bmap);
> @@ -2714,14 +2707,17 @@ 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, ", ");
> - nfs4_show_owner(s, oo);
> - seq_printf(s, " }\n");
> -out:
> + 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);
> + seq_puts(s, " }\n");
> return 0;
> }
>
> @@ -2735,30 +2731,29 @@ 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, "- ");
> + seq_puts(s, "- ");
> nfs4_show_stateid(s, &st->sc_stateid);
> - seq_printf(s, ": { type: lock, ");
> + seq_puts(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);
> - seq_printf(s, " }\n");
> -out:
> + seq_puts(s, " }\n");
> spin_unlock(&nf->fi_lock);
> return 0;
> }
> @@ -2771,25 +2766,25 @@ 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, "- ");
> + seq_puts(s, "- ");
> nfs4_show_stateid(s, &st->sc_stateid);
> - seq_printf(s, ": { type: deleg, ");
> + seq_puts(s, ": { type: deleg, ");
>
> - 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);
> + }
> + seq_puts(s, " }\n");
> spin_unlock(&nf->fi_lock);
> return 0;
> }
> @@ -2802,16 +2797,19 @@ static int nfs4_show_layout(struct seq_file *s, struct nfs4_stid *st)
> ls = container_of(st, struct nfs4_layout_stateid, ls_stid);
> file = ls->ls_file;
>
> - seq_printf(s, "- ");
> + seq_puts(s, "- ");
> nfs4_show_stateid(s, &st->sc_stateid);
> - seq_printf(s, ": { type: layout, ");
> + seq_puts(s, ": { type: layout");
>
> /* XXX: What else would be useful? */
>
> - nfs4_show_superblock(s, file);
> - seq_printf(s, ", ");
> - nfs4_show_fname(s, file);
> - seq_printf(s, " }\n");
> + if (file) {
> + seq_puts(s, ", ");
> + nfs4_show_superblock(s, file);
> + seq_puts(s, ", ");
> + nfs4_show_fname(s, file);
> + }
> + seq_puts(s, " }\n");
>
> return 0;
> }
Reviewed-by: Jeff Layton <jlayton@kernel.org>
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH 08/13] nfsd: report in /proc/fs/nfsd/clients/*/states when state is admin-revoke
2024-01-29 3:29 [PATCH 00/13 v4] nfsd: support admin-revocation of v4 state NeilBrown
` (6 preceding siblings ...)
2024-01-29 3:29 ` [PATCH 07/13] nfsd: allow state with no file to appear in /proc/fs/nfsd/clients/*/states NeilBrown
@ 2024-01-29 3:29 ` NeilBrown
2024-01-29 12:24 ` Jeff Layton
2024-01-29 3:29 ` [PATCH 09/13] nfsd: allow admin-revoked NFSv4.0 state to be freed NeilBrown
` (4 subsequent siblings)
12 siblings, 1 reply; 27+ messages in thread
From: NeilBrown @ 2024-01-29 3:29 UTC (permalink / raw)
To: Chuck Lever, Jeff Layton
Cc: linux-nfs, Olga Kornievskaia, Dai Ngo, Tom Talpey,
Christoph Hellwig, Tom Haynes
Add "admin-revoked" to the status information for any states that have
been admin-revoked. This can be useful for confirming correct
behaviour.
Signed-off-by: NeilBrown <neilb@suse.de>
---
fs/nfsd/nfs4state.c | 10 +++++++++-
1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index ef4ec23f7c0d..e1492ca7c75c 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -2717,6 +2717,8 @@ static int nfs4_show_open(struct seq_file *s, struct nfs4_stid *st)
}
spin_unlock(&nf->fi_lock);
nfs4_show_owner(s, oo);
+ if (st->sc_status & SC_STATUS_ADMIN_REVOKED)
+ seq_puts(s, ", admin-revoked");
seq_puts(s, " }\n");
return 0;
}
@@ -2753,6 +2755,8 @@ static int nfs4_show_lock(struct seq_file *s, struct nfs4_stid *st)
seq_puts(s, ", ");
}
nfs4_show_owner(s, oo);
+ if (st->sc_status & SC_STATUS_ADMIN_REVOKED)
+ seq_puts(s, ", admin-revoked");
seq_puts(s, " }\n");
spin_unlock(&nf->fi_lock);
return 0;
@@ -2784,8 +2788,10 @@ static int nfs4_show_deleg(struct seq_file *s, struct nfs4_stid *st)
seq_puts(s, ", ");
nfs4_show_fname(s, file);
}
- seq_puts(s, " }\n");
spin_unlock(&nf->fi_lock);
+ if (st->sc_status & SC_STATUS_ADMIN_REVOKED)
+ seq_puts(s, ", admin-revoked");
+ seq_puts(s, " }\n");
return 0;
}
@@ -2809,6 +2815,8 @@ static int nfs4_show_layout(struct seq_file *s, struct nfs4_stid *st)
seq_puts(s, ", ");
nfs4_show_fname(s, file);
}
+ if (st->sc_status & SC_STATUS_ADMIN_REVOKED)
+ seq_puts(s, ", admin-revoked");
seq_puts(s, " }\n");
return 0;
--
2.43.0
^ permalink raw reply related [flat|nested] 27+ messages in thread* Re: [PATCH 08/13] nfsd: report in /proc/fs/nfsd/clients/*/states when state is admin-revoke
2024-01-29 3:29 ` [PATCH 08/13] nfsd: report in /proc/fs/nfsd/clients/*/states when state is admin-revoke NeilBrown
@ 2024-01-29 12:24 ` Jeff Layton
0 siblings, 0 replies; 27+ messages in thread
From: Jeff Layton @ 2024-01-29 12:24 UTC (permalink / raw)
To: NeilBrown, Chuck Lever
Cc: linux-nfs, Olga Kornievskaia, Dai Ngo, Tom Talpey,
Christoph Hellwig, Tom Haynes
On Mon, 2024-01-29 at 14:29 +1100, NeilBrown wrote:
> Add "admin-revoked" to the status information for any states that have
> been admin-revoked. This can be useful for confirming correct
> behaviour.
>
> Signed-off-by: NeilBrown <neilb@suse.de>
> ---
> fs/nfsd/nfs4state.c | 10 +++++++++-
> 1 file changed, 9 insertions(+), 1 deletion(-)
>
> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
> index ef4ec23f7c0d..e1492ca7c75c 100644
> --- a/fs/nfsd/nfs4state.c
> +++ b/fs/nfsd/nfs4state.c
> @@ -2717,6 +2717,8 @@ static int nfs4_show_open(struct seq_file *s, struct nfs4_stid *st)
> }
> spin_unlock(&nf->fi_lock);
> nfs4_show_owner(s, oo);
> + if (st->sc_status & SC_STATUS_ADMIN_REVOKED)
> + seq_puts(s, ", admin-revoked");
> seq_puts(s, " }\n");
> return 0;
> }
> @@ -2753,6 +2755,8 @@ static int nfs4_show_lock(struct seq_file *s, struct nfs4_stid *st)
> seq_puts(s, ", ");
> }
> nfs4_show_owner(s, oo);
> + if (st->sc_status & SC_STATUS_ADMIN_REVOKED)
> + seq_puts(s, ", admin-revoked");
> seq_puts(s, " }\n");
> spin_unlock(&nf->fi_lock);
> return 0;
> @@ -2784,8 +2788,10 @@ static int nfs4_show_deleg(struct seq_file *s, struct nfs4_stid *st)
> seq_puts(s, ", ");
> nfs4_show_fname(s, file);
> }
> - seq_puts(s, " }\n");
> spin_unlock(&nf->fi_lock);
> + if (st->sc_status & SC_STATUS_ADMIN_REVOKED)
> + seq_puts(s, ", admin-revoked");
> + seq_puts(s, " }\n");
> return 0;
> }
>
> @@ -2809,6 +2815,8 @@ static int nfs4_show_layout(struct seq_file *s, struct nfs4_stid *st)
> seq_puts(s, ", ");
> nfs4_show_fname(s, file);
> }
> + if (st->sc_status & SC_STATUS_ADMIN_REVOKED)
> + seq_puts(s, ", admin-revoked");
> seq_puts(s, " }\n");
>
> return 0;
Reviewed-by: Jeff Layton <jlayton@kernel.org>
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH 09/13] nfsd: allow admin-revoked NFSv4.0 state to be freed.
2024-01-29 3:29 [PATCH 00/13 v4] nfsd: support admin-revocation of v4 state NeilBrown
` (7 preceding siblings ...)
2024-01-29 3:29 ` [PATCH 08/13] nfsd: report in /proc/fs/nfsd/clients/*/states when state is admin-revoke NeilBrown
@ 2024-01-29 3:29 ` NeilBrown
2024-01-29 12:29 ` Jeff Layton
2024-01-29 3:29 ` [PATCH 10/13] nfsd: allow lock state ids to be revoked and then freed NeilBrown
` (3 subsequent siblings)
12 siblings, 1 reply; 27+ messages in thread
From: NeilBrown @ 2024-01-29 3:29 UTC (permalink / raw)
To: Chuck Lever, Jeff Layton
Cc: linux-nfs, Olga Kornievskaia, Dai Ngo, Tom Talpey,
Christoph Hellwig, Tom Haynes
For NFSv4.1 and later the client easily discovers if there is any
admin-revoked state and will then find and explicitly free it.
For NFSv4.0 there is no such mechanism. The client can only find that
state is admin-revoked if it tries to use that state, and there is no
way for it to explicitly free the state. So the server must hold on to
the stateid (at least) for an indefinite amount of time. A
RELEASE_LOCKOWNER request might justify forgetting some of these
stateids, as would the whole clients lease lapsing, but these are not
reliable.
This patch takes two approaches.
Whenever a client uses an revoked stateid, that stateid is then
discarded and will not be recognised again. This might confuse a client
which expect to get NFS4ERR_ADMIN_REVOKED consistently once it get it at
all, but should mostly work. Hopefully one error will lead to other
resources being closed (e.g. process exits), which will result in more
stateid being freed when a CLOSE attempt gets NFS4ERR_ADMIN_REVOKED.
Also, any admin-revoked stateids that have been that way for more than
one lease time are periodically revoke.
No actual freeing of state happens in this patch. That will come in
future patches which handle the different sorts of revoked state.
Signed-off-by: NeilBrown <neilb@suse.de>
---
fs/nfsd/netns.h | 4 ++
fs/nfsd/nfs4state.c | 98 ++++++++++++++++++++++++++++++++++++++++++++-
2 files changed, 101 insertions(+), 1 deletion(-)
diff --git a/fs/nfsd/netns.h b/fs/nfsd/netns.h
index 74b4360779a1..b35bdd5c10de 100644
--- a/fs/nfsd/netns.h
+++ b/fs/nfsd/netns.h
@@ -192,6 +192,10 @@ struct nfsd_net {
atomic_t nfsd_courtesy_clients;
struct shrinker *nfsd_client_shrinker;
struct work_struct nfsd_shrinker_work;
+
+ /* last time an admin-revoke happened for NFSv4.0 */
+ time64_t nfs40_last_revoke;
+
};
/* Simple check to find out if a given net was properly initialized */
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index e1492ca7c75c..900d295bd570 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -1733,6 +1733,14 @@ void nfsd4_revoke_states(struct net *net, struct super_block *sb)
}
nfs4_put_stid(stid);
spin_lock(&nn->client_lock);
+ if (clp->cl_minorversion == 0)
+ /* Allow cleanup after a lease period.
+ * store_release ensures cleanup will
+ * see any newly revoked states if it
+ * sees the time updated.
+ */
+ nn->nfs40_last_revoke =
+ ktime_get_boottime_seconds();
goto retry;
}
}
@@ -4617,6 +4625,40 @@ nfsd4_find_existing_open(struct nfs4_file *fp, struct nfsd4_open *open)
return ret;
}
+static void nfsd4_drop_revoked_stid(struct nfs4_stid *s)
+ __releases(&s->sc_client->cl_lock)
+{
+ struct nfs4_client *cl = s->sc_client;
+
+ switch (s->sc_type) {
+ default:
+ spin_unlock(&cl->cl_lock);
+ }
+}
+
+static void nfsd40_drop_revoked_stid(struct nfs4_client *cl,
+ stateid_t *stid)
+{
+ /* NFSv4.0 has no way for the client to tell the server
+ * that it can forget an admin-revoked stateid.
+ * So we keep it around until the first time that the
+ * client uses it, and drop it the first time
+ * nfserr_admin_revoked is returned.
+ * For v4.1 and later we wait until explicitly told
+ * to free the stateid.
+ */
+ if (cl->cl_minorversion == 0) {
+ struct nfs4_stid *st;
+
+ spin_lock(&cl->cl_lock);
+ st = find_stateid_locked(cl, stid);
+ if (st)
+ nfsd4_drop_revoked_stid(st);
+ else
+ spin_unlock(&cl->cl_lock);
+ }
+}
+
static __be32
nfsd4_verify_open_stid(struct nfs4_stid *s)
{
@@ -4639,6 +4681,10 @@ nfsd4_lock_ol_stateid(struct nfs4_ol_stateid *stp)
mutex_lock_nested(&stp->st_mutex, LOCK_STATEID_MUTEX);
ret = nfsd4_verify_open_stid(&stp->st_stid);
+ if (ret == nfserr_admin_revoked)
+ nfsd40_drop_revoked_stid(stp->st_stid.sc_client,
+ &stp->st_stid.sc_stateid);
+
if (ret != nfs_ok)
mutex_unlock(&stp->st_mutex);
return ret;
@@ -5222,6 +5268,7 @@ nfs4_check_deleg(struct nfs4_client *cl, struct nfsd4_open *open,
}
if (deleg->dl_stid.sc_status & SC_STATUS_REVOKED) {
nfs4_put_stid(&deleg->dl_stid);
+ nfsd40_drop_revoked_stid(cl, &open->op_delegate_stateid);
status = nfserr_deleg_revoked;
goto out;
}
@@ -6206,6 +6253,43 @@ nfs4_process_client_reaplist(struct list_head *reaplist)
}
}
+static void nfs40_clean_admin_revoked(struct nfsd_net *nn,
+ struct laundry_time *lt)
+{
+ struct nfs4_client *clp;
+
+ spin_lock(&nn->client_lock);
+ if (nn->nfs40_last_revoke == 0 ||
+ nn->nfs40_last_revoke > lt->cutoff) {
+ spin_unlock(&nn->client_lock);
+ return;
+ }
+ nn->nfs40_last_revoke = 0;
+
+retry:
+ list_for_each_entry(clp, &nn->client_lru, cl_lru) {
+ unsigned long id, tmp;
+ struct nfs4_stid *stid;
+
+ if (atomic_read(&clp->cl_admin_revoked) == 0)
+ continue;
+
+ spin_lock(&clp->cl_lock);
+ idr_for_each_entry_ul(&clp->cl_stateids, stid, tmp, id)
+ if (stid->sc_status & SC_STATUS_ADMIN_REVOKED) {
+ refcount_inc(&stid->sc_count);
+ spin_unlock(&nn->client_lock);
+ /* this function drops ->cl_lock */
+ nfsd4_drop_revoked_stid(stid);
+ nfs4_put_stid(stid);
+ spin_lock(&nn->client_lock);
+ goto retry;
+ }
+ spin_unlock(&clp->cl_lock);
+ }
+ spin_unlock(&nn->client_lock);
+}
+
static time64_t
nfs4_laundromat(struct nfsd_net *nn)
{
@@ -6239,6 +6323,8 @@ nfs4_laundromat(struct nfsd_net *nn)
nfs4_get_client_reaplist(nn, &reaplist, <);
nfs4_process_client_reaplist(&reaplist);
+ nfs40_clean_admin_revoked(nn, <);
+
spin_lock(&state_lock);
list_for_each_safe(pos, next, &nn->del_recall_lru) {
dp = list_entry (pos, struct nfs4_delegation, dl_recall_lru);
@@ -6457,6 +6543,9 @@ static __be32 nfsd4_stid_check_stateid_generation(stateid_t *in, struct nfs4_sti
if (ret == nfs_ok)
ret = check_stateid_generation(in, &s->sc_stateid, has_session);
spin_unlock(&s->sc_lock);
+ if (ret == nfserr_admin_revoked)
+ nfsd40_drop_revoked_stid(s->sc_client,
+ &s->sc_stateid);
return ret;
}
@@ -6501,6 +6590,8 @@ static __be32 nfsd4_validate_stateid(struct nfs4_client *cl, stateid_t *stateid)
}
out_unlock:
spin_unlock(&cl->cl_lock);
+ if (status == nfserr_admin_revoked)
+ nfsd40_drop_revoked_stid(cl, stateid);
return status;
}
@@ -6547,6 +6638,7 @@ nfsd4_lookup_stateid(struct nfsd4_compound_state *cstate,
return nfserr_deleg_revoked;
}
if (stid->sc_status & SC_STATUS_ADMIN_REVOKED) {
+ nfsd40_drop_revoked_stid(cstate->clp, stateid);
nfs4_put_stid(stid);
return nfserr_admin_revoked;
}
@@ -6839,6 +6931,11 @@ nfsd4_free_stateid(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
s = find_stateid_locked(cl, stateid);
if (!s || s->sc_status & SC_STATUS_CLOSED)
goto out_unlock;
+ if (s->sc_status & SC_STATUS_ADMIN_REVOKED) {
+ nfsd4_drop_revoked_stid(s);
+ ret = nfs_ok;
+ goto out;
+ }
spin_lock(&s->sc_lock);
switch (s->sc_type) {
case SC_TYPE_DELEG:
@@ -6865,7 +6962,6 @@ nfsd4_free_stateid(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
spin_unlock(&cl->cl_lock);
ret = nfsd4_free_lock_stateid(stateid, s);
goto out;
- /* Default falls through and returns nfserr_bad_stateid */
}
spin_unlock(&s->sc_lock);
out_unlock:
--
2.43.0
^ permalink raw reply related [flat|nested] 27+ messages in thread* Re: [PATCH 09/13] nfsd: allow admin-revoked NFSv4.0 state to be freed.
2024-01-29 3:29 ` [PATCH 09/13] nfsd: allow admin-revoked NFSv4.0 state to be freed NeilBrown
@ 2024-01-29 12:29 ` Jeff Layton
0 siblings, 0 replies; 27+ messages in thread
From: Jeff Layton @ 2024-01-29 12:29 UTC (permalink / raw)
To: NeilBrown, Chuck Lever
Cc: linux-nfs, Olga Kornievskaia, Dai Ngo, Tom Talpey,
Christoph Hellwig, Tom Haynes
On Mon, 2024-01-29 at 14:29 +1100, NeilBrown wrote:
> For NFSv4.1 and later the client easily discovers if there is any
> admin-revoked state and will then find and explicitly free it.
>
> For NFSv4.0 there is no such mechanism. The client can only find that
> state is admin-revoked if it tries to use that state, and there is no
> way for it to explicitly free the state. So the server must hold on to
> the stateid (at least) for an indefinite amount of time. A
> RELEASE_LOCKOWNER request might justify forgetting some of these
> stateids, as would the whole clients lease lapsing, but these are not
> reliable.
>
> This patch takes two approaches.
>
> Whenever a client uses an revoked stateid, that stateid is then
> discarded and will not be recognised again. This might confuse a client
> which expect to get NFS4ERR_ADMIN_REVOKED consistently once it get it at
> all, but should mostly work. Hopefully one error will lead to other
> resources being closed (e.g. process exits), which will result in more
> stateid being freed when a CLOSE attempt gets NFS4ERR_ADMIN_REVOKED.
>
> Also, any admin-revoked stateids that have been that way for more than
> one lease time are periodically revoke.
>
> No actual freeing of state happens in this patch. That will come in
> future patches which handle the different sorts of revoked state.
>
> Signed-off-by: NeilBrown <neilb@suse.de>
> ---
> fs/nfsd/netns.h | 4 ++
> fs/nfsd/nfs4state.c | 98 ++++++++++++++++++++++++++++++++++++++++++++-
> 2 files changed, 101 insertions(+), 1 deletion(-)
>
> diff --git a/fs/nfsd/netns.h b/fs/nfsd/netns.h
> index 74b4360779a1..b35bdd5c10de 100644
> --- a/fs/nfsd/netns.h
> +++ b/fs/nfsd/netns.h
> @@ -192,6 +192,10 @@ struct nfsd_net {
> atomic_t nfsd_courtesy_clients;
> struct shrinker *nfsd_client_shrinker;
> struct work_struct nfsd_shrinker_work;
> +
> + /* last time an admin-revoke happened for NFSv4.0 */
> + time64_t nfs40_last_revoke;
> +
> };
>
> /* Simple check to find out if a given net was properly initialized */
> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
> index e1492ca7c75c..900d295bd570 100644
> --- a/fs/nfsd/nfs4state.c
> +++ b/fs/nfsd/nfs4state.c
> @@ -1733,6 +1733,14 @@ void nfsd4_revoke_states(struct net *net, struct super_block *sb)
> }
> nfs4_put_stid(stid);
> spin_lock(&nn->client_lock);
> + if (clp->cl_minorversion == 0)
> + /* Allow cleanup after a lease period.
> + * store_release ensures cleanup will
> + * see any newly revoked states if it
> + * sees the time updated.
> + */
> + nn->nfs40_last_revoke =
> + ktime_get_boottime_seconds();
> goto retry;
> }
> }
> @@ -4617,6 +4625,40 @@ nfsd4_find_existing_open(struct nfs4_file *fp, struct nfsd4_open *open)
> return ret;
> }
>
> +static void nfsd4_drop_revoked_stid(struct nfs4_stid *s)
> + __releases(&s->sc_client->cl_lock)
> +{
> + struct nfs4_client *cl = s->sc_client;
> +
> + switch (s->sc_type) {
> + default:
> + spin_unlock(&cl->cl_lock);
> + }
> +}
> +
> +static void nfsd40_drop_revoked_stid(struct nfs4_client *cl,
> + stateid_t *stid)
> +{
> + /* NFSv4.0 has no way for the client to tell the server
> + * that it can forget an admin-revoked stateid.
> + * So we keep it around until the first time that the
> + * client uses it, and drop it the first time
> + * nfserr_admin_revoked is returned.
> + * For v4.1 and later we wait until explicitly told
> + * to free the stateid.
> + */
> + if (cl->cl_minorversion == 0) {
> + struct nfs4_stid *st;
> +
> + spin_lock(&cl->cl_lock);
> + st = find_stateid_locked(cl, stid);
> + if (st)
> + nfsd4_drop_revoked_stid(st);
> + else
> + spin_unlock(&cl->cl_lock);
> + }
> +}
> +
> static __be32
> nfsd4_verify_open_stid(struct nfs4_stid *s)
> {
> @@ -4639,6 +4681,10 @@ nfsd4_lock_ol_stateid(struct nfs4_ol_stateid *stp)
>
> mutex_lock_nested(&stp->st_mutex, LOCK_STATEID_MUTEX);
> ret = nfsd4_verify_open_stid(&stp->st_stid);
> + if (ret == nfserr_admin_revoked)
> + nfsd40_drop_revoked_stid(stp->st_stid.sc_client,
> + &stp->st_stid.sc_stateid);
> +
> if (ret != nfs_ok)
> mutex_unlock(&stp->st_mutex);
> return ret;
> @@ -5222,6 +5268,7 @@ nfs4_check_deleg(struct nfs4_client *cl, struct nfsd4_open *open,
> }
> if (deleg->dl_stid.sc_status & SC_STATUS_REVOKED) {
> nfs4_put_stid(&deleg->dl_stid);
> + nfsd40_drop_revoked_stid(cl, &open->op_delegate_stateid);
> status = nfserr_deleg_revoked;
> goto out;
> }
> @@ -6206,6 +6253,43 @@ nfs4_process_client_reaplist(struct list_head *reaplist)
> }
> }
>
> +static void nfs40_clean_admin_revoked(struct nfsd_net *nn,
> + struct laundry_time *lt)
> +{
> + struct nfs4_client *clp;
> +
> + spin_lock(&nn->client_lock);
> + if (nn->nfs40_last_revoke == 0 ||
> + nn->nfs40_last_revoke > lt->cutoff) {
> + spin_unlock(&nn->client_lock);
> + return;
> + }
> + nn->nfs40_last_revoke = 0;
> +
> +retry:
> + list_for_each_entry(clp, &nn->client_lru, cl_lru) {
> + unsigned long id, tmp;
> + struct nfs4_stid *stid;
> +
> + if (atomic_read(&clp->cl_admin_revoked) == 0)
> + continue;
> +
> + spin_lock(&clp->cl_lock);
> + idr_for_each_entry_ul(&clp->cl_stateids, stid, tmp, id)
> + if (stid->sc_status & SC_STATUS_ADMIN_REVOKED) {
> + refcount_inc(&stid->sc_count);
> + spin_unlock(&nn->client_lock);
> + /* this function drops ->cl_lock */
> + nfsd4_drop_revoked_stid(stid);
> + nfs4_put_stid(stid);
> + spin_lock(&nn->client_lock);
> + goto retry;
> + }
> + spin_unlock(&clp->cl_lock);
> + }
> + spin_unlock(&nn->client_lock);
> +}
> +
> static time64_t
> nfs4_laundromat(struct nfsd_net *nn)
> {
> @@ -6239,6 +6323,8 @@ nfs4_laundromat(struct nfsd_net *nn)
> nfs4_get_client_reaplist(nn, &reaplist, <);
> nfs4_process_client_reaplist(&reaplist);
>
> + nfs40_clean_admin_revoked(nn, <);
> +
> spin_lock(&state_lock);
> list_for_each_safe(pos, next, &nn->del_recall_lru) {
> dp = list_entry (pos, struct nfs4_delegation, dl_recall_lru);
> @@ -6457,6 +6543,9 @@ static __be32 nfsd4_stid_check_stateid_generation(stateid_t *in, struct nfs4_sti
> if (ret == nfs_ok)
> ret = check_stateid_generation(in, &s->sc_stateid, has_session);
> spin_unlock(&s->sc_lock);
> + if (ret == nfserr_admin_revoked)
> + nfsd40_drop_revoked_stid(s->sc_client,
> + &s->sc_stateid);
> return ret;
> }
>
> @@ -6501,6 +6590,8 @@ static __be32 nfsd4_validate_stateid(struct nfs4_client *cl, stateid_t *stateid)
> }
> out_unlock:
> spin_unlock(&cl->cl_lock);
> + if (status == nfserr_admin_revoked)
> + nfsd40_drop_revoked_stid(cl, stateid);
> return status;
> }
>
> @@ -6547,6 +6638,7 @@ nfsd4_lookup_stateid(struct nfsd4_compound_state *cstate,
> return nfserr_deleg_revoked;
> }
> if (stid->sc_status & SC_STATUS_ADMIN_REVOKED) {
> + nfsd40_drop_revoked_stid(cstate->clp, stateid);
> nfs4_put_stid(stid);
> return nfserr_admin_revoked;
> }
> @@ -6839,6 +6931,11 @@ nfsd4_free_stateid(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
> s = find_stateid_locked(cl, stateid);
> if (!s || s->sc_status & SC_STATUS_CLOSED)
> goto out_unlock;
> + if (s->sc_status & SC_STATUS_ADMIN_REVOKED) {
> + nfsd4_drop_revoked_stid(s);
> + ret = nfs_ok;
> + goto out;
> + }
> spin_lock(&s->sc_lock);
> switch (s->sc_type) {
> case SC_TYPE_DELEG:
> @@ -6865,7 +6962,6 @@ nfsd4_free_stateid(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
> spin_unlock(&cl->cl_lock);
> ret = nfsd4_free_lock_stateid(stateid, s);
> goto out;
> - /* Default falls through and returns nfserr_bad_stateid */
> }
> spin_unlock(&s->sc_lock);
> out_unlock:
Reviewed-by: Jeff Layton <jlayton@kernel.org>
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH 10/13] nfsd: allow lock state ids to be revoked and then freed
2024-01-29 3:29 [PATCH 00/13 v4] nfsd: support admin-revocation of v4 state NeilBrown
` (8 preceding siblings ...)
2024-01-29 3:29 ` [PATCH 09/13] nfsd: allow admin-revoked NFSv4.0 state to be freed NeilBrown
@ 2024-01-29 3:29 ` NeilBrown
2024-01-29 12:30 ` Jeff Layton
2024-01-29 3:29 ` [PATCH 11/13] nfsd: allow open " NeilBrown
` (2 subsequent siblings)
12 siblings, 1 reply; 27+ messages in thread
From: NeilBrown @ 2024-01-29 3:29 UTC (permalink / raw)
To: Chuck Lever, Jeff Layton
Cc: linux-nfs, Olga Kornievskaia, Dai Ngo, Tom Talpey,
Christoph Hellwig, Tom Haynes
Revoking state through 'unlock_filesystem' now revokes any lock states
found. When the stateids are then freed by the client, the revoked
stateids will be cleaned up correctly.
Signed-off-by: NeilBrown <neilb@suse.de>
---
fs/nfsd/nfs4state.c | 40 +++++++++++++++++++++++++++++++++++++++-
1 file changed, 39 insertions(+), 1 deletion(-)
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index 900d295bd570..a5c17dab8bdb 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -1717,7 +1717,7 @@ void nfsd4_revoke_states(struct net *net, struct super_block *sb)
unsigned int idhashval;
unsigned int sc_types;
- sc_types = 0;
+ sc_types = SC_TYPE_LOCK;
spin_lock(&nn->client_lock);
for (idhashval = 0; idhashval < CLIENT_HASH_MASK; idhashval++) {
@@ -1728,8 +1728,36 @@ void nfsd4_revoke_states(struct net *net, struct super_block *sb)
struct nfs4_stid *stid = find_one_sb_stid(clp, sb,
sc_types);
if (stid) {
+ struct nfs4_ol_stateid *stp;
+
spin_unlock(&nn->client_lock);
switch (stid->sc_type) {
+ case SC_TYPE_LOCK:
+ stp = openlockstateid(stid);
+ mutex_lock_nested(&stp->st_mutex,
+ LOCK_STATEID_MUTEX);
+ spin_lock(&clp->cl_lock);
+ if (stid->sc_status == 0) {
+ struct nfs4_lockowner *lo =
+ lockowner(stp->st_stateowner);
+ struct nfsd_file *nf;
+
+ stid->sc_status |=
+ SC_STATUS_ADMIN_REVOKED;
+ atomic_inc(&clp->cl_admin_revoked);
+ spin_unlock(&clp->cl_lock);
+ nf = find_any_file(stp->st_stid.sc_file);
+ if (nf) {
+ get_file(nf->nf_file);
+ filp_close(nf->nf_file,
+ (fl_owner_t)lo);
+ nfsd_file_put(nf);
+ }
+ release_all_access(stp);
+ } else
+ spin_unlock(&clp->cl_lock);
+ mutex_unlock(&stp->st_mutex);
+ break;
}
nfs4_put_stid(stid);
spin_lock(&nn->client_lock);
@@ -4629,8 +4657,18 @@ static void nfsd4_drop_revoked_stid(struct nfs4_stid *s)
__releases(&s->sc_client->cl_lock)
{
struct nfs4_client *cl = s->sc_client;
+ LIST_HEAD(reaplist);
+ struct nfs4_ol_stateid *stp;
+ bool unhashed;
switch (s->sc_type) {
+ case SC_TYPE_LOCK:
+ stp = openlockstateid(s);
+ unhashed = unhash_lock_stateid(stp);
+ spin_unlock(&cl->cl_lock);
+ if (unhashed)
+ nfs4_put_stid(s);
+ break;
default:
spin_unlock(&cl->cl_lock);
}
--
2.43.0
^ permalink raw reply related [flat|nested] 27+ messages in thread* Re: [PATCH 10/13] nfsd: allow lock state ids to be revoked and then freed
2024-01-29 3:29 ` [PATCH 10/13] nfsd: allow lock state ids to be revoked and then freed NeilBrown
@ 2024-01-29 12:30 ` Jeff Layton
0 siblings, 0 replies; 27+ messages in thread
From: Jeff Layton @ 2024-01-29 12:30 UTC (permalink / raw)
To: NeilBrown, Chuck Lever
Cc: linux-nfs, Olga Kornievskaia, Dai Ngo, Tom Talpey,
Christoph Hellwig, Tom Haynes
On Mon, 2024-01-29 at 14:29 +1100, NeilBrown wrote:
> Revoking state through 'unlock_filesystem' now revokes any lock states
> found. When the stateids are then freed by the client, the revoked
> stateids will be cleaned up correctly.
>
> Signed-off-by: NeilBrown <neilb@suse.de>
> ---
> fs/nfsd/nfs4state.c | 40 +++++++++++++++++++++++++++++++++++++++-
> 1 file changed, 39 insertions(+), 1 deletion(-)
>
> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
> index 900d295bd570..a5c17dab8bdb 100644
> --- a/fs/nfsd/nfs4state.c
> +++ b/fs/nfsd/nfs4state.c
> @@ -1717,7 +1717,7 @@ void nfsd4_revoke_states(struct net *net, struct super_block *sb)
> unsigned int idhashval;
> unsigned int sc_types;
>
> - sc_types = 0;
> + sc_types = SC_TYPE_LOCK;
>
> spin_lock(&nn->client_lock);
> for (idhashval = 0; idhashval < CLIENT_HASH_MASK; idhashval++) {
> @@ -1728,8 +1728,36 @@ void nfsd4_revoke_states(struct net *net, struct super_block *sb)
> struct nfs4_stid *stid = find_one_sb_stid(clp, sb,
> sc_types);
> if (stid) {
> + struct nfs4_ol_stateid *stp;
> +
> spin_unlock(&nn->client_lock);
> switch (stid->sc_type) {
> + case SC_TYPE_LOCK:
> + stp = openlockstateid(stid);
> + mutex_lock_nested(&stp->st_mutex,
> + LOCK_STATEID_MUTEX);
> + spin_lock(&clp->cl_lock);
> + if (stid->sc_status == 0) {
> + struct nfs4_lockowner *lo =
> + lockowner(stp->st_stateowner);
> + struct nfsd_file *nf;
> +
> + stid->sc_status |=
> + SC_STATUS_ADMIN_REVOKED;
> + atomic_inc(&clp->cl_admin_revoked);
> + spin_unlock(&clp->cl_lock);
> + nf = find_any_file(stp->st_stid.sc_file);
> + if (nf) {
> + get_file(nf->nf_file);
> + filp_close(nf->nf_file,
> + (fl_owner_t)lo);
> + nfsd_file_put(nf);
> + }
> + release_all_access(stp);
> + } else
> + spin_unlock(&clp->cl_lock);
> + mutex_unlock(&stp->st_mutex);
> + break;
> }
> nfs4_put_stid(stid);
> spin_lock(&nn->client_lock);
> @@ -4629,8 +4657,18 @@ static void nfsd4_drop_revoked_stid(struct nfs4_stid *s)
> __releases(&s->sc_client->cl_lock)
> {
> struct nfs4_client *cl = s->sc_client;
> + LIST_HEAD(reaplist);
> + struct nfs4_ol_stateid *stp;
> + bool unhashed;
>
> switch (s->sc_type) {
> + case SC_TYPE_LOCK:
> + stp = openlockstateid(s);
> + unhashed = unhash_lock_stateid(stp);
> + spin_unlock(&cl->cl_lock);
> + if (unhashed)
> + nfs4_put_stid(s);
> + break;
> default:
> spin_unlock(&cl->cl_lock);
> }
Reviewed-by: Jeff Layton <jlayton@kernel.org>
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH 11/13] nfsd: allow open state ids to be revoked and then freed
2024-01-29 3:29 [PATCH 00/13 v4] nfsd: support admin-revocation of v4 state NeilBrown
` (9 preceding siblings ...)
2024-01-29 3:29 ` [PATCH 10/13] nfsd: allow lock state ids to be revoked and then freed NeilBrown
@ 2024-01-29 3:29 ` NeilBrown
2024-01-29 12:31 ` Jeff Layton
2024-01-29 3:29 ` [PATCH 12/13] nfsd: allow delegation " NeilBrown
2024-01-29 3:29 ` [PATCH 13/13] nfsd: allow layout state to be admin-revoked NeilBrown
12 siblings, 1 reply; 27+ messages in thread
From: NeilBrown @ 2024-01-29 3:29 UTC (permalink / raw)
To: Chuck Lever, Jeff Layton
Cc: linux-nfs, Olga Kornievskaia, Dai Ngo, Tom Talpey,
Christoph Hellwig, Tom Haynes
Revoking state through 'unlock_filesystem' now revokes any open states
found. When the stateids are then freed by the client, the revoked
stateids will be cleaned up correctly.
Possibly the related lock states should be revoked too, but a
subsequent patch will do that for all lock state on the superblock.
Signed-off-by: NeilBrown <neilb@suse.de>
---
fs/nfsd/nfs4state.c | 25 ++++++++++++++++++++++++-
1 file changed, 24 insertions(+), 1 deletion(-)
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index a5c17dab8bdb..5dc8f60e18dc 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -1717,7 +1717,7 @@ void nfsd4_revoke_states(struct net *net, struct super_block *sb)
unsigned int idhashval;
unsigned int sc_types;
- sc_types = SC_TYPE_LOCK;
+ sc_types = SC_TYPE_OPEN | SC_TYPE_LOCK;
spin_lock(&nn->client_lock);
for (idhashval = 0; idhashval < CLIENT_HASH_MASK; idhashval++) {
@@ -1732,6 +1732,22 @@ void nfsd4_revoke_states(struct net *net, struct super_block *sb)
spin_unlock(&nn->client_lock);
switch (stid->sc_type) {
+ case SC_TYPE_OPEN:
+ stp = openlockstateid(stid);
+ mutex_lock_nested(&stp->st_mutex,
+ OPEN_STATEID_MUTEX);
+
+ spin_lock(&clp->cl_lock);
+ if (stid->sc_status == 0) {
+ stid->sc_status |=
+ SC_STATUS_ADMIN_REVOKED;
+ atomic_inc(&clp->cl_admin_revoked);
+ spin_unlock(&clp->cl_lock);
+ release_all_access(stp);
+ } else
+ spin_unlock(&clp->cl_lock);
+ mutex_unlock(&stp->st_mutex);
+ break;
case SC_TYPE_LOCK:
stp = openlockstateid(stid);
mutex_lock_nested(&stp->st_mutex,
@@ -4662,6 +4678,13 @@ static void nfsd4_drop_revoked_stid(struct nfs4_stid *s)
bool unhashed;
switch (s->sc_type) {
+ case SC_TYPE_OPEN:
+ stp = openlockstateid(s);
+ if (unhash_open_stateid(stp, &reaplist))
+ put_ol_stateid_locked(stp, &reaplist);
+ spin_unlock(&cl->cl_lock);
+ free_ol_stateid_reaplist(&reaplist);
+ break;
case SC_TYPE_LOCK:
stp = openlockstateid(s);
unhashed = unhash_lock_stateid(stp);
--
2.43.0
^ permalink raw reply related [flat|nested] 27+ messages in thread* Re: [PATCH 11/13] nfsd: allow open state ids to be revoked and then freed
2024-01-29 3:29 ` [PATCH 11/13] nfsd: allow open " NeilBrown
@ 2024-01-29 12:31 ` Jeff Layton
0 siblings, 0 replies; 27+ messages in thread
From: Jeff Layton @ 2024-01-29 12:31 UTC (permalink / raw)
To: NeilBrown, Chuck Lever
Cc: linux-nfs, Olga Kornievskaia, Dai Ngo, Tom Talpey,
Christoph Hellwig, Tom Haynes
On Mon, 2024-01-29 at 14:29 +1100, NeilBrown wrote:
> Revoking state through 'unlock_filesystem' now revokes any open states
> found. When the stateids are then freed by the client, the revoked
> stateids will be cleaned up correctly.
>
> Possibly the related lock states should be revoked too, but a
> subsequent patch will do that for all lock state on the superblock.
>
> Signed-off-by: NeilBrown <neilb@suse.de>
> ---
> fs/nfsd/nfs4state.c | 25 ++++++++++++++++++++++++-
> 1 file changed, 24 insertions(+), 1 deletion(-)
>
> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
> index a5c17dab8bdb..5dc8f60e18dc 100644
> --- a/fs/nfsd/nfs4state.c
> +++ b/fs/nfsd/nfs4state.c
> @@ -1717,7 +1717,7 @@ void nfsd4_revoke_states(struct net *net, struct super_block *sb)
> unsigned int idhashval;
> unsigned int sc_types;
>
> - sc_types = SC_TYPE_LOCK;
> + sc_types = SC_TYPE_OPEN | SC_TYPE_LOCK;
>
> spin_lock(&nn->client_lock);
> for (idhashval = 0; idhashval < CLIENT_HASH_MASK; idhashval++) {
> @@ -1732,6 +1732,22 @@ void nfsd4_revoke_states(struct net *net, struct super_block *sb)
>
> spin_unlock(&nn->client_lock);
> switch (stid->sc_type) {
> + case SC_TYPE_OPEN:
> + stp = openlockstateid(stid);
> + mutex_lock_nested(&stp->st_mutex,
> + OPEN_STATEID_MUTEX);
> +
> + spin_lock(&clp->cl_lock);
> + if (stid->sc_status == 0) {
> + stid->sc_status |=
> + SC_STATUS_ADMIN_REVOKED;
> + atomic_inc(&clp->cl_admin_revoked);
> + spin_unlock(&clp->cl_lock);
> + release_all_access(stp);
> + } else
> + spin_unlock(&clp->cl_lock);
> + mutex_unlock(&stp->st_mutex);
> + break;
> case SC_TYPE_LOCK:
> stp = openlockstateid(stid);
> mutex_lock_nested(&stp->st_mutex,
> @@ -4662,6 +4678,13 @@ static void nfsd4_drop_revoked_stid(struct nfs4_stid *s)
> bool unhashed;
>
> switch (s->sc_type) {
> + case SC_TYPE_OPEN:
> + stp = openlockstateid(s);
> + if (unhash_open_stateid(stp, &reaplist))
> + put_ol_stateid_locked(stp, &reaplist);
> + spin_unlock(&cl->cl_lock);
> + free_ol_stateid_reaplist(&reaplist);
> + break;
> case SC_TYPE_LOCK:
> stp = openlockstateid(s);
> unhashed = unhash_lock_stateid(stp);
Reviewed-by: Jeff Layton <jlayton@kernel.org>
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH 12/13] nfsd: allow delegation state ids to be revoked and then freed
2024-01-29 3:29 [PATCH 00/13 v4] nfsd: support admin-revocation of v4 state NeilBrown
` (10 preceding siblings ...)
2024-01-29 3:29 ` [PATCH 11/13] nfsd: allow open " NeilBrown
@ 2024-01-29 3:29 ` NeilBrown
2024-01-29 12:32 ` Jeff Layton
2024-01-29 3:29 ` [PATCH 13/13] nfsd: allow layout state to be admin-revoked NeilBrown
12 siblings, 1 reply; 27+ messages in thread
From: NeilBrown @ 2024-01-29 3:29 UTC (permalink / raw)
To: Chuck Lever, Jeff Layton
Cc: linux-nfs, Olga Kornievskaia, Dai Ngo, Tom Talpey,
Christoph Hellwig, Tom Haynes
Revoking state through 'unlock_filesystem' now revokes any delegation
states found. When the stateids are then freed by the client, the
revoked stateids will be cleaned up correctly.
As there is already support for revoking delegations, we build on that
for admin-revoking.
Signed-off-by: NeilBrown <neilb@suse.de>
---
fs/nfsd/nfs4state.c | 28 +++++++++++++++++++++++++---
1 file changed, 25 insertions(+), 3 deletions(-)
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index 5dc8f60e18dc..e749d5c0e23a 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -1335,9 +1335,12 @@ unhash_delegation_locked(struct nfs4_delegation *dp, unsigned short statusmask)
if (!delegation_hashed(dp))
return false;
- if (dp->dl_stid.sc_client->cl_minorversion == 0)
+ if (statusmask == SC_STATUS_REVOKED &&
+ dp->dl_stid.sc_client->cl_minorversion == 0)
statusmask = SC_STATUS_CLOSED;
dp->dl_stid.sc_status |= statusmask;
+ if (statusmask & SC_STATUS_ADMIN_REVOKED)
+ atomic_inc(&dp->dl_stid.sc_client->cl_admin_revoked);
/* Ensure that deleg break won't try to requeue it */
++dp->dl_time;
@@ -1368,7 +1371,8 @@ static void revoke_delegation(struct nfs4_delegation *dp)
trace_nfsd_stid_revoke(&dp->dl_stid);
- if (dp->dl_stid.sc_status & SC_STATUS_REVOKED) {
+ if (dp->dl_stid.sc_status &
+ (SC_STATUS_REVOKED | SC_STATUS_ADMIN_REVOKED)) {
spin_lock(&clp->cl_lock);
refcount_inc(&dp->dl_stid.sc_count);
list_add(&dp->dl_recall_lru, &clp->cl_revoked);
@@ -1717,7 +1721,7 @@ void nfsd4_revoke_states(struct net *net, struct super_block *sb)
unsigned int idhashval;
unsigned int sc_types;
- sc_types = SC_TYPE_OPEN | SC_TYPE_LOCK;
+ sc_types = SC_TYPE_OPEN | SC_TYPE_LOCK | SC_TYPE_DELEG;
spin_lock(&nn->client_lock);
for (idhashval = 0; idhashval < CLIENT_HASH_MASK; idhashval++) {
@@ -1729,6 +1733,7 @@ void nfsd4_revoke_states(struct net *net, struct super_block *sb)
sc_types);
if (stid) {
struct nfs4_ol_stateid *stp;
+ struct nfs4_delegation *dp;
spin_unlock(&nn->client_lock);
switch (stid->sc_type) {
@@ -1774,6 +1779,16 @@ void nfsd4_revoke_states(struct net *net, struct super_block *sb)
spin_unlock(&clp->cl_lock);
mutex_unlock(&stp->st_mutex);
break;
+ case SC_TYPE_DELEG:
+ dp = delegstateid(stid);
+ spin_lock(&state_lock);
+ if (!unhash_delegation_locked(
+ dp, SC_STATUS_ADMIN_REVOKED))
+ dp = NULL;
+ spin_unlock(&state_lock);
+ if (dp)
+ revoke_delegation(dp);
+ break;
}
nfs4_put_stid(stid);
spin_lock(&nn->client_lock);
@@ -4675,6 +4690,7 @@ static void nfsd4_drop_revoked_stid(struct nfs4_stid *s)
struct nfs4_client *cl = s->sc_client;
LIST_HEAD(reaplist);
struct nfs4_ol_stateid *stp;
+ struct nfs4_delegation *dp;
bool unhashed;
switch (s->sc_type) {
@@ -4692,6 +4708,12 @@ static void nfsd4_drop_revoked_stid(struct nfs4_stid *s)
if (unhashed)
nfs4_put_stid(s);
break;
+ case SC_TYPE_DELEG:
+ dp = delegstateid(s);
+ list_del_init(&dp->dl_recall_lru);
+ spin_unlock(&cl->cl_lock);
+ nfs4_put_stid(s);
+ break;
default:
spin_unlock(&cl->cl_lock);
}
--
2.43.0
^ permalink raw reply related [flat|nested] 27+ messages in thread* Re: [PATCH 12/13] nfsd: allow delegation state ids to be revoked and then freed
2024-01-29 3:29 ` [PATCH 12/13] nfsd: allow delegation " NeilBrown
@ 2024-01-29 12:32 ` Jeff Layton
0 siblings, 0 replies; 27+ messages in thread
From: Jeff Layton @ 2024-01-29 12:32 UTC (permalink / raw)
To: NeilBrown, Chuck Lever
Cc: linux-nfs, Olga Kornievskaia, Dai Ngo, Tom Talpey,
Christoph Hellwig, Tom Haynes
On Mon, 2024-01-29 at 14:29 +1100, NeilBrown wrote:
> Revoking state through 'unlock_filesystem' now revokes any delegation
> states found. When the stateids are then freed by the client, the
> revoked stateids will be cleaned up correctly.
>
> As there is already support for revoking delegations, we build on that
> for admin-revoking.
>
> Signed-off-by: NeilBrown <neilb@suse.de>
> ---
> fs/nfsd/nfs4state.c | 28 +++++++++++++++++++++++++---
> 1 file changed, 25 insertions(+), 3 deletions(-)
>
> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
> index 5dc8f60e18dc..e749d5c0e23a 100644
> --- a/fs/nfsd/nfs4state.c
> +++ b/fs/nfsd/nfs4state.c
> @@ -1335,9 +1335,12 @@ unhash_delegation_locked(struct nfs4_delegation *dp, unsigned short statusmask)
> if (!delegation_hashed(dp))
> return false;
>
> - if (dp->dl_stid.sc_client->cl_minorversion == 0)
> + if (statusmask == SC_STATUS_REVOKED &&
> + dp->dl_stid.sc_client->cl_minorversion == 0)
> statusmask = SC_STATUS_CLOSED;
> dp->dl_stid.sc_status |= statusmask;
> + if (statusmask & SC_STATUS_ADMIN_REVOKED)
> + atomic_inc(&dp->dl_stid.sc_client->cl_admin_revoked);
>
> /* Ensure that deleg break won't try to requeue it */
> ++dp->dl_time;
> @@ -1368,7 +1371,8 @@ static void revoke_delegation(struct nfs4_delegation *dp)
>
> trace_nfsd_stid_revoke(&dp->dl_stid);
>
> - if (dp->dl_stid.sc_status & SC_STATUS_REVOKED) {
> + if (dp->dl_stid.sc_status &
> + (SC_STATUS_REVOKED | SC_STATUS_ADMIN_REVOKED)) {
> spin_lock(&clp->cl_lock);
> refcount_inc(&dp->dl_stid.sc_count);
> list_add(&dp->dl_recall_lru, &clp->cl_revoked);
> @@ -1717,7 +1721,7 @@ void nfsd4_revoke_states(struct net *net, struct super_block *sb)
> unsigned int idhashval;
> unsigned int sc_types;
>
> - sc_types = SC_TYPE_OPEN | SC_TYPE_LOCK;
> + sc_types = SC_TYPE_OPEN | SC_TYPE_LOCK | SC_TYPE_DELEG;
>
> spin_lock(&nn->client_lock);
> for (idhashval = 0; idhashval < CLIENT_HASH_MASK; idhashval++) {
> @@ -1729,6 +1733,7 @@ void nfsd4_revoke_states(struct net *net, struct super_block *sb)
> sc_types);
> if (stid) {
> struct nfs4_ol_stateid *stp;
> + struct nfs4_delegation *dp;
>
> spin_unlock(&nn->client_lock);
> switch (stid->sc_type) {
> @@ -1774,6 +1779,16 @@ void nfsd4_revoke_states(struct net *net, struct super_block *sb)
> spin_unlock(&clp->cl_lock);
> mutex_unlock(&stp->st_mutex);
> break;
> + case SC_TYPE_DELEG:
> + dp = delegstateid(stid);
> + spin_lock(&state_lock);
> + if (!unhash_delegation_locked(
> + dp, SC_STATUS_ADMIN_REVOKED))
> + dp = NULL;
> + spin_unlock(&state_lock);
> + if (dp)
> + revoke_delegation(dp);
> + break;
> }
> nfs4_put_stid(stid);
> spin_lock(&nn->client_lock);
> @@ -4675,6 +4690,7 @@ static void nfsd4_drop_revoked_stid(struct nfs4_stid *s)
> struct nfs4_client *cl = s->sc_client;
> LIST_HEAD(reaplist);
> struct nfs4_ol_stateid *stp;
> + struct nfs4_delegation *dp;
> bool unhashed;
>
> switch (s->sc_type) {
> @@ -4692,6 +4708,12 @@ static void nfsd4_drop_revoked_stid(struct nfs4_stid *s)
> if (unhashed)
> nfs4_put_stid(s);
> break;
> + case SC_TYPE_DELEG:
> + dp = delegstateid(s);
> + list_del_init(&dp->dl_recall_lru);
> + spin_unlock(&cl->cl_lock);
> + nfs4_put_stid(s);
> + break;
> default:
> spin_unlock(&cl->cl_lock);
> }
Reviewed-by: Jeff Layton <jlayton@kernel.org>
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH 13/13] nfsd: allow layout state to be admin-revoked.
2024-01-29 3:29 [PATCH 00/13 v4] nfsd: support admin-revocation of v4 state NeilBrown
` (11 preceding siblings ...)
2024-01-29 3:29 ` [PATCH 12/13] nfsd: allow delegation " NeilBrown
@ 2024-01-29 3:29 ` NeilBrown
2024-01-29 12:38 ` Jeff Layton
12 siblings, 1 reply; 27+ messages in thread
From: NeilBrown @ 2024-01-29 3:29 UTC (permalink / raw)
To: Chuck Lever, Jeff Layton
Cc: linux-nfs, Olga Kornievskaia, Dai Ngo, Tom Talpey,
Christoph Hellwig, Tom Haynes
When there is layout state on a filesystem that is being "unlocked" that
is now revoked, which involves closing the nfsd_file and releasing the
vfs lease.
To avoid races, ->ls_file can now be accessed either:
- under ->fi_lock for the state's sc_file or
- under rcu_read_lock() if nfsd_file_get() is used.
To support this, ->fence_client and nfsd4_cb_layout_fail() now take a
second argument being the nfsd_file.
Signed-off-by: NeilBrown <neilb@suse.de>
---
fs/nfsd/blocklayout.c | 4 ++--
fs/nfsd/nfs4layouts.c | 43 ++++++++++++++++++++++++++++++++-----------
fs/nfsd/nfs4state.c | 11 +++++++++--
fs/nfsd/pnfs.h | 8 +++++++-
4 files changed, 50 insertions(+), 16 deletions(-)
diff --git a/fs/nfsd/blocklayout.c b/fs/nfsd/blocklayout.c
index 46fd74d91ea9..3c040c81c77d 100644
--- a/fs/nfsd/blocklayout.c
+++ b/fs/nfsd/blocklayout.c
@@ -328,10 +328,10 @@ nfsd4_scsi_proc_layoutcommit(struct inode *inode,
}
static void
-nfsd4_scsi_fence_client(struct nfs4_layout_stateid *ls)
+nfsd4_scsi_fence_client(struct nfs4_layout_stateid *ls, struct nfsd_file *file)
{
struct nfs4_client *clp = ls->ls_stid.sc_client;
- struct block_device *bdev = ls->ls_file->nf_file->f_path.mnt->mnt_sb->s_bdev;
+ struct block_device *bdev = file->nf_file->f_path.mnt->mnt_sb->s_bdev;
bdev->bd_disk->fops->pr_ops->pr_preempt(bdev, NFSD_MDS_PR_KEY,
nfsd4_scsi_pr_key(clp), 0, true);
diff --git a/fs/nfsd/nfs4layouts.c b/fs/nfsd/nfs4layouts.c
index 857b822450b4..1cfd61db2472 100644
--- a/fs/nfsd/nfs4layouts.c
+++ b/fs/nfsd/nfs4layouts.c
@@ -152,6 +152,23 @@ void nfsd4_setup_layout_type(struct svc_export *exp)
#endif
}
+void nfsd4_close_layout(struct nfs4_layout_stateid *ls)
+{
+ struct nfsd_file *fl;
+
+ spin_lock(&ls->ls_stid.sc_file->fi_lock);
+ fl = ls->ls_file;
+ ls->ls_file = NULL;
+ spin_unlock(&ls->ls_stid.sc_file->fi_lock);
+
+ if (fl) {
+ if (!nfsd4_layout_ops[ls->ls_layout_type]->disable_recalls)
+ vfs_setlease(fl->nf_file, F_UNLCK, NULL,
+ (void **)&ls);
+ nfsd_file_put(fl);
+ }
+}
+
static void
nfsd4_free_layout_stateid(struct nfs4_stid *stid)
{
@@ -169,9 +186,7 @@ nfsd4_free_layout_stateid(struct nfs4_stid *stid)
list_del_init(&ls->ls_perfile);
spin_unlock(&fp->fi_lock);
- if (!nfsd4_layout_ops[ls->ls_layout_type]->disable_recalls)
- vfs_setlease(ls->ls_file->nf_file, F_UNLCK, NULL, (void **)&ls);
- nfsd_file_put(ls->ls_file);
+ nfsd4_close_layout(ls);
if (ls->ls_recalled)
atomic_dec(&ls->ls_stid.sc_file->fi_lo_recalls);
@@ -605,7 +620,7 @@ nfsd4_return_all_file_layouts(struct nfs4_client *clp, struct nfs4_file *fp)
}
static void
-nfsd4_cb_layout_fail(struct nfs4_layout_stateid *ls)
+nfsd4_cb_layout_fail(struct nfs4_layout_stateid *ls, struct nfsd_file *file)
{
struct nfs4_client *clp = ls->ls_stid.sc_client;
char addr_str[INET6_ADDRSTRLEN];
@@ -627,7 +642,7 @@ nfsd4_cb_layout_fail(struct nfs4_layout_stateid *ls)
argv[0] = (char *)nfsd_recall_failed;
argv[1] = addr_str;
- argv[2] = ls->ls_file->nf_file->f_path.mnt->mnt_sb->s_id;
+ argv[2] = file->nf_file->f_path.mnt->mnt_sb->s_id;
argv[3] = NULL;
error = call_usermodehelper(nfsd_recall_failed, argv, envp,
@@ -657,6 +672,7 @@ nfsd4_cb_layout_done(struct nfsd4_callback *cb, struct rpc_task *task)
struct nfsd_net *nn;
ktime_t now, cutoff;
const struct nfsd4_layout_ops *ops;
+ struct nfsd_file *fl;
trace_nfsd_cb_layout_done(&ls->ls_stid.sc_stateid, task);
switch (task->tk_status) {
@@ -688,12 +704,17 @@ nfsd4_cb_layout_done(struct nfsd4_callback *cb, struct rpc_task *task)
* Unknown error or non-responding client, we'll need to fence.
*/
trace_nfsd_layout_recall_fail(&ls->ls_stid.sc_stateid);
-
- ops = nfsd4_layout_ops[ls->ls_layout_type];
- if (ops->fence_client)
- ops->fence_client(ls);
- else
- nfsd4_cb_layout_fail(ls);
+ rcu_read_lock();
+ fl = nfsd_file_get(ls->ls_file);
+ rcu_read_unlock();
+ if (fl) {
+ ops = nfsd4_layout_ops[ls->ls_layout_type];
+ if (ops->fence_client)
+ ops->fence_client(ls, fl);
+ else
+ nfsd4_cb_layout_fail(ls, fl);
+ nfsd_file_put(fl);
+ }
return 1;
case -NFS4ERR_NOMATCHING_LAYOUT:
trace_nfsd_layout_recall_done(&ls->ls_stid.sc_stateid);
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index e749d5c0e23a..268b47a6f3b6 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -1721,7 +1721,7 @@ void nfsd4_revoke_states(struct net *net, struct super_block *sb)
unsigned int idhashval;
unsigned int sc_types;
- sc_types = SC_TYPE_OPEN | SC_TYPE_LOCK | SC_TYPE_DELEG;
+ sc_types = SC_TYPE_OPEN | SC_TYPE_LOCK | SC_TYPE_DELEG | SC_TYPE_LAYOUT;
spin_lock(&nn->client_lock);
for (idhashval = 0; idhashval < CLIENT_HASH_MASK; idhashval++) {
@@ -1734,6 +1734,7 @@ void nfsd4_revoke_states(struct net *net, struct super_block *sb)
if (stid) {
struct nfs4_ol_stateid *stp;
struct nfs4_delegation *dp;
+ struct nfs4_layout_stateid *ls;
spin_unlock(&nn->client_lock);
switch (stid->sc_type) {
@@ -1789,6 +1790,10 @@ void nfsd4_revoke_states(struct net *net, struct super_block *sb)
if (dp)
revoke_delegation(dp);
break;
+ case SC_TYPE_LAYOUT:
+ ls = layoutstateid(stid);
+ nfsd4_close_layout(ls);
+ break;
}
nfs4_put_stid(stid);
spin_lock(&nn->client_lock);
@@ -2868,7 +2873,6 @@ static int nfs4_show_layout(struct seq_file *s, struct nfs4_stid *st)
struct nfsd_file *file;
ls = container_of(st, struct nfs4_layout_stateid, ls_stid);
- file = ls->ls_file;
seq_puts(s, "- ");
nfs4_show_stateid(s, &st->sc_stateid);
@@ -2876,12 +2880,15 @@ static int nfs4_show_layout(struct seq_file *s, struct nfs4_stid *st)
/* XXX: What else would be useful? */
+ spin_lock(&ls->ls_stid.sc_file->fi_lock);
+ file = ls->ls_file;
if (file) {
seq_puts(s, ", ");
nfs4_show_superblock(s, file);
seq_puts(s, ", ");
nfs4_show_fname(s, file);
}
+ spin_unlock(&ls->ls_stid.sc_file->fi_lock);
if (st->sc_status & SC_STATUS_ADMIN_REVOKED)
seq_puts(s, ", admin-revoked");
seq_puts(s, " }\n");
diff --git a/fs/nfsd/pnfs.h b/fs/nfsd/pnfs.h
index de1e0dfed06a..925817f66917 100644
--- a/fs/nfsd/pnfs.h
+++ b/fs/nfsd/pnfs.h
@@ -37,7 +37,8 @@ struct nfsd4_layout_ops {
__be32 (*proc_layoutcommit)(struct inode *inode,
struct nfsd4_layoutcommit *lcp);
- void (*fence_client)(struct nfs4_layout_stateid *ls);
+ void (*fence_client)(struct nfs4_layout_stateid *ls,
+ struct nfsd_file *file);
};
extern const struct nfsd4_layout_ops *nfsd4_layout_ops[];
@@ -72,11 +73,13 @@ void nfsd4_setup_layout_type(struct svc_export *exp);
void nfsd4_return_all_client_layouts(struct nfs4_client *);
void nfsd4_return_all_file_layouts(struct nfs4_client *clp,
struct nfs4_file *fp);
+void nfsd4_close_layout(struct nfs4_layout_stateid *ls);
int nfsd4_init_pnfs(void);
void nfsd4_exit_pnfs(void);
#else
struct nfs4_client;
struct nfs4_file;
+struct nfs4_layout_stateid;
static inline void nfsd4_setup_layout_type(struct svc_export *exp)
{
@@ -89,6 +92,9 @@ static inline void nfsd4_return_all_file_layouts(struct nfs4_client *clp,
struct nfs4_file *fp)
{
}
+static inline void nfsd4_close_layout(struct nfs4_layout_stateid *ls)
+{
+}
static inline void nfsd4_exit_pnfs(void)
{
}
--
2.43.0
^ permalink raw reply related [flat|nested] 27+ messages in thread* Re: [PATCH 13/13] nfsd: allow layout state to be admin-revoked.
2024-01-29 3:29 ` [PATCH 13/13] nfsd: allow layout state to be admin-revoked NeilBrown
@ 2024-01-29 12:38 ` Jeff Layton
2024-01-30 1:07 ` NeilBrown
0 siblings, 1 reply; 27+ messages in thread
From: Jeff Layton @ 2024-01-29 12:38 UTC (permalink / raw)
To: NeilBrown, Chuck Lever
Cc: linux-nfs, Olga Kornievskaia, Dai Ngo, Tom Talpey,
Christoph Hellwig, Tom Haynes
On Mon, 2024-01-29 at 14:29 +1100, NeilBrown wrote:
> When there is layout state on a filesystem that is being "unlocked" that
> is now revoked, which involves closing the nfsd_file and releasing the
> vfs lease.
>
> To avoid races, ->ls_file can now be accessed either:
> - under ->fi_lock for the state's sc_file or
> - under rcu_read_lock() if nfsd_file_get() is used.
> To support this, ->fence_client and nfsd4_cb_layout_fail() now take a
> second argument being the nfsd_file.
>
> Signed-off-by: NeilBrown <neilb@suse.de>
> ---
> fs/nfsd/blocklayout.c | 4 ++--
> fs/nfsd/nfs4layouts.c | 43 ++++++++++++++++++++++++++++++++-----------
> fs/nfsd/nfs4state.c | 11 +++++++++--
> fs/nfsd/pnfs.h | 8 +++++++-
> 4 files changed, 50 insertions(+), 16 deletions(-)
>
> diff --git a/fs/nfsd/blocklayout.c b/fs/nfsd/blocklayout.c
> index 46fd74d91ea9..3c040c81c77d 100644
> --- a/fs/nfsd/blocklayout.c
> +++ b/fs/nfsd/blocklayout.c
> @@ -328,10 +328,10 @@ nfsd4_scsi_proc_layoutcommit(struct inode *inode,
> }
>
> static void
> -nfsd4_scsi_fence_client(struct nfs4_layout_stateid *ls)
> +nfsd4_scsi_fence_client(struct nfs4_layout_stateid *ls, struct nfsd_file *file)
> {
> struct nfs4_client *clp = ls->ls_stid.sc_client;
> - struct block_device *bdev = ls->ls_file->nf_file->f_path.mnt->mnt_sb->s_bdev;
> + struct block_device *bdev = file->nf_file->f_path.mnt->mnt_sb->s_bdev;
>
> bdev->bd_disk->fops->pr_ops->pr_preempt(bdev, NFSD_MDS_PR_KEY,
> nfsd4_scsi_pr_key(clp), 0, true);
> diff --git a/fs/nfsd/nfs4layouts.c b/fs/nfsd/nfs4layouts.c
> index 857b822450b4..1cfd61db2472 100644
> --- a/fs/nfsd/nfs4layouts.c
> +++ b/fs/nfsd/nfs4layouts.c
> @@ -152,6 +152,23 @@ void nfsd4_setup_layout_type(struct svc_export *exp)
> #endif
> }
>
> +void nfsd4_close_layout(struct nfs4_layout_stateid *ls)
> +{
> + struct nfsd_file *fl;
> +
> + spin_lock(&ls->ls_stid.sc_file->fi_lock);
> + fl = ls->ls_file;
> + ls->ls_file = NULL;
> + spin_unlock(&ls->ls_stid.sc_file->fi_lock);
> +
> + if (fl) {
> + if (!nfsd4_layout_ops[ls->ls_layout_type]->disable_recalls)
> + vfs_setlease(fl->nf_file, F_UNLCK, NULL,
> + (void **)&ls);
> + nfsd_file_put(fl);
> + }
> +}
> +
> static void
> nfsd4_free_layout_stateid(struct nfs4_stid *stid)
> {
> @@ -169,9 +186,7 @@ nfsd4_free_layout_stateid(struct nfs4_stid *stid)
> list_del_init(&ls->ls_perfile);
> spin_unlock(&fp->fi_lock);
>
> - if (!nfsd4_layout_ops[ls->ls_layout_type]->disable_recalls)
> - vfs_setlease(ls->ls_file->nf_file, F_UNLCK, NULL, (void **)&ls);
> - nfsd_file_put(ls->ls_file);
> + nfsd4_close_layout(ls);
>
> if (ls->ls_recalled)
> atomic_dec(&ls->ls_stid.sc_file->fi_lo_recalls);
> @@ -605,7 +620,7 @@ nfsd4_return_all_file_layouts(struct nfs4_client *clp, struct nfs4_file *fp)
> }
>
> static void
> -nfsd4_cb_layout_fail(struct nfs4_layout_stateid *ls)
> +nfsd4_cb_layout_fail(struct nfs4_layout_stateid *ls, struct nfsd_file *file)
> {
> struct nfs4_client *clp = ls->ls_stid.sc_client;
> char addr_str[INET6_ADDRSTRLEN];
> @@ -627,7 +642,7 @@ nfsd4_cb_layout_fail(struct nfs4_layout_stateid *ls)
>
> argv[0] = (char *)nfsd_recall_failed;
> argv[1] = addr_str;
> - argv[2] = ls->ls_file->nf_file->f_path.mnt->mnt_sb->s_id;
> + argv[2] = file->nf_file->f_path.mnt->mnt_sb->s_id;
> argv[3] = NULL;
>
> error = call_usermodehelper(nfsd_recall_failed, argv, envp,
> @@ -657,6 +672,7 @@ nfsd4_cb_layout_done(struct nfsd4_callback *cb, struct rpc_task *task)
> struct nfsd_net *nn;
> ktime_t now, cutoff;
> const struct nfsd4_layout_ops *ops;
> + struct nfsd_file *fl;
>
> trace_nfsd_cb_layout_done(&ls->ls_stid.sc_stateid, task);
> switch (task->tk_status) {
> @@ -688,12 +704,17 @@ nfsd4_cb_layout_done(struct nfsd4_callback *cb, struct rpc_task *task)
> * Unknown error or non-responding client, we'll need to fence.
> */
> trace_nfsd_layout_recall_fail(&ls->ls_stid.sc_stateid);
> -
> - ops = nfsd4_layout_ops[ls->ls_layout_type];
> - if (ops->fence_client)
> - ops->fence_client(ls);
> - else
> - nfsd4_cb_layout_fail(ls);
> + rcu_read_lock();
> + fl = nfsd_file_get(ls->ls_file);
> + rcu_read_unlock();
What is the rcu_read_lock protecting here? AFAICT, you don't need it
since you hold a reference to "ls".
> + if (fl) {
> + ops = nfsd4_layout_ops[ls->ls_layout_type];
> + if (ops->fence_client)
> + ops->fence_client(ls, fl);
> + else
> + nfsd4_cb_layout_fail(ls, fl);
> + nfsd_file_put(fl);
> + }
> return 1;
> case -NFS4ERR_NOMATCHING_LAYOUT:
> trace_nfsd_layout_recall_done(&ls->ls_stid.sc_stateid);
> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
> index e749d5c0e23a..268b47a6f3b6 100644
> --- a/fs/nfsd/nfs4state.c
> +++ b/fs/nfsd/nfs4state.c
> @@ -1721,7 +1721,7 @@ void nfsd4_revoke_states(struct net *net, struct super_block *sb)
> unsigned int idhashval;
> unsigned int sc_types;
>
> - sc_types = SC_TYPE_OPEN | SC_TYPE_LOCK | SC_TYPE_DELEG;
> + sc_types = SC_TYPE_OPEN | SC_TYPE_LOCK | SC_TYPE_DELEG | SC_TYPE_LAYOUT;
>
> spin_lock(&nn->client_lock);
> for (idhashval = 0; idhashval < CLIENT_HASH_MASK; idhashval++) {
> @@ -1734,6 +1734,7 @@ void nfsd4_revoke_states(struct net *net, struct super_block *sb)
> if (stid) {
> struct nfs4_ol_stateid *stp;
> struct nfs4_delegation *dp;
> + struct nfs4_layout_stateid *ls;
>
> spin_unlock(&nn->client_lock);
> switch (stid->sc_type) {
> @@ -1789,6 +1790,10 @@ void nfsd4_revoke_states(struct net *net, struct super_block *sb)
> if (dp)
> revoke_delegation(dp);
> break;
> + case SC_TYPE_LAYOUT:
> + ls = layoutstateid(stid);
> + nfsd4_close_layout(ls);
> + break;
> }
> nfs4_put_stid(stid);
> spin_lock(&nn->client_lock);
> @@ -2868,7 +2873,6 @@ static int nfs4_show_layout(struct seq_file *s, struct nfs4_stid *st)
> struct nfsd_file *file;
>
> ls = container_of(st, struct nfs4_layout_stateid, ls_stid);
> - file = ls->ls_file;
>
> seq_puts(s, "- ");
> nfs4_show_stateid(s, &st->sc_stateid);
> @@ -2876,12 +2880,15 @@ static int nfs4_show_layout(struct seq_file *s, struct nfs4_stid *st)
>
> /* XXX: What else would be useful? */
>
> + spin_lock(&ls->ls_stid.sc_file->fi_lock);
> + file = ls->ls_file;
> if (file) {
> seq_puts(s, ", ");
> nfs4_show_superblock(s, file);
> seq_puts(s, ", ");
> nfs4_show_fname(s, file);
> }
> + spin_unlock(&ls->ls_stid.sc_file->fi_lock);
> if (st->sc_status & SC_STATUS_ADMIN_REVOKED)
> seq_puts(s, ", admin-revoked");
> seq_puts(s, " }\n");
> diff --git a/fs/nfsd/pnfs.h b/fs/nfsd/pnfs.h
> index de1e0dfed06a..925817f66917 100644
> --- a/fs/nfsd/pnfs.h
> +++ b/fs/nfsd/pnfs.h
> @@ -37,7 +37,8 @@ struct nfsd4_layout_ops {
> __be32 (*proc_layoutcommit)(struct inode *inode,
> struct nfsd4_layoutcommit *lcp);
>
> - void (*fence_client)(struct nfs4_layout_stateid *ls);
> + void (*fence_client)(struct nfs4_layout_stateid *ls,
> + struct nfsd_file *file);
> };
>
> extern const struct nfsd4_layout_ops *nfsd4_layout_ops[];
> @@ -72,11 +73,13 @@ void nfsd4_setup_layout_type(struct svc_export *exp);
> void nfsd4_return_all_client_layouts(struct nfs4_client *);
> void nfsd4_return_all_file_layouts(struct nfs4_client *clp,
> struct nfs4_file *fp);
> +void nfsd4_close_layout(struct nfs4_layout_stateid *ls);
> int nfsd4_init_pnfs(void);
> void nfsd4_exit_pnfs(void);
> #else
> struct nfs4_client;
> struct nfs4_file;
> +struct nfs4_layout_stateid;
>
> static inline void nfsd4_setup_layout_type(struct svc_export *exp)
> {
> @@ -89,6 +92,9 @@ static inline void nfsd4_return_all_file_layouts(struct nfs4_client *clp,
> struct nfs4_file *fp)
> {
> }
> +static inline void nfsd4_close_layout(struct nfs4_layout_stateid *ls)
> +{
> +}
> static inline void nfsd4_exit_pnfs(void)
> {
> }
--
Jeff Layton <jlayton@kernel.org>
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PATCH 13/13] nfsd: allow layout state to be admin-revoked.
2024-01-29 12:38 ` Jeff Layton
@ 2024-01-30 1:07 ` NeilBrown
0 siblings, 0 replies; 27+ messages in thread
From: NeilBrown @ 2024-01-30 1:07 UTC (permalink / raw)
To: Jeff Layton
Cc: Chuck Lever, linux-nfs, Olga Kornievskaia, Dai Ngo, Tom Talpey,
Christoph Hellwig, Tom Haynes
On Mon, 29 Jan 2024, Jeff Layton wrote:
> On Mon, 2024-01-29 at 14:29 +1100, NeilBrown wrote:
> > When there is layout state on a filesystem that is being "unlocked" that
> > is now revoked, which involves closing the nfsd_file and releasing the
> > vfs lease.
> >
> > To avoid races, ->ls_file can now be accessed either:
> > - under ->fi_lock for the state's sc_file or
> > - under rcu_read_lock() if nfsd_file_get() is used.
> > To support this, ->fence_client and nfsd4_cb_layout_fail() now take a
> > second argument being the nfsd_file.
> >
> > Signed-off-by: NeilBrown <neilb@suse.de>
> > ---
> > fs/nfsd/blocklayout.c | 4 ++--
> > fs/nfsd/nfs4layouts.c | 43 ++++++++++++++++++++++++++++++++-----------
> > fs/nfsd/nfs4state.c | 11 +++++++++--
> > fs/nfsd/pnfs.h | 8 +++++++-
> > 4 files changed, 50 insertions(+), 16 deletions(-)
> >
> > diff --git a/fs/nfsd/blocklayout.c b/fs/nfsd/blocklayout.c
> > index 46fd74d91ea9..3c040c81c77d 100644
> > --- a/fs/nfsd/blocklayout.c
> > +++ b/fs/nfsd/blocklayout.c
> > @@ -328,10 +328,10 @@ nfsd4_scsi_proc_layoutcommit(struct inode *inode,
> > }
> >
> > static void
> > -nfsd4_scsi_fence_client(struct nfs4_layout_stateid *ls)
> > +nfsd4_scsi_fence_client(struct nfs4_layout_stateid *ls, struct nfsd_file *file)
> > {
> > struct nfs4_client *clp = ls->ls_stid.sc_client;
> > - struct block_device *bdev = ls->ls_file->nf_file->f_path.mnt->mnt_sb->s_bdev;
> > + struct block_device *bdev = file->nf_file->f_path.mnt->mnt_sb->s_bdev;
> >
> > bdev->bd_disk->fops->pr_ops->pr_preempt(bdev, NFSD_MDS_PR_KEY,
> > nfsd4_scsi_pr_key(clp), 0, true);
> > diff --git a/fs/nfsd/nfs4layouts.c b/fs/nfsd/nfs4layouts.c
> > index 857b822450b4..1cfd61db2472 100644
> > --- a/fs/nfsd/nfs4layouts.c
> > +++ b/fs/nfsd/nfs4layouts.c
> > @@ -152,6 +152,23 @@ void nfsd4_setup_layout_type(struct svc_export *exp)
> > #endif
> > }
> >
> > +void nfsd4_close_layout(struct nfs4_layout_stateid *ls)
> > +{
> > + struct nfsd_file *fl;
> > +
> > + spin_lock(&ls->ls_stid.sc_file->fi_lock);
> > + fl = ls->ls_file;
> > + ls->ls_file = NULL;
> > + spin_unlock(&ls->ls_stid.sc_file->fi_lock);
> > +
> > + if (fl) {
> > + if (!nfsd4_layout_ops[ls->ls_layout_type]->disable_recalls)
> > + vfs_setlease(fl->nf_file, F_UNLCK, NULL,
> > + (void **)&ls);
> > + nfsd_file_put(fl);
> > + }
> > +}
> > +
> > static void
> > nfsd4_free_layout_stateid(struct nfs4_stid *stid)
> > {
> > @@ -169,9 +186,7 @@ nfsd4_free_layout_stateid(struct nfs4_stid *stid)
> > list_del_init(&ls->ls_perfile);
> > spin_unlock(&fp->fi_lock);
> >
> > - if (!nfsd4_layout_ops[ls->ls_layout_type]->disable_recalls)
> > - vfs_setlease(ls->ls_file->nf_file, F_UNLCK, NULL, (void **)&ls);
> > - nfsd_file_put(ls->ls_file);
> > + nfsd4_close_layout(ls);
> >
> > if (ls->ls_recalled)
> > atomic_dec(&ls->ls_stid.sc_file->fi_lo_recalls);
> > @@ -605,7 +620,7 @@ nfsd4_return_all_file_layouts(struct nfs4_client *clp, struct nfs4_file *fp)
> > }
> >
> > static void
> > -nfsd4_cb_layout_fail(struct nfs4_layout_stateid *ls)
> > +nfsd4_cb_layout_fail(struct nfs4_layout_stateid *ls, struct nfsd_file *file)
> > {
> > struct nfs4_client *clp = ls->ls_stid.sc_client;
> > char addr_str[INET6_ADDRSTRLEN];
> > @@ -627,7 +642,7 @@ nfsd4_cb_layout_fail(struct nfs4_layout_stateid *ls)
> >
> > argv[0] = (char *)nfsd_recall_failed;
> > argv[1] = addr_str;
> > - argv[2] = ls->ls_file->nf_file->f_path.mnt->mnt_sb->s_id;
> > + argv[2] = file->nf_file->f_path.mnt->mnt_sb->s_id;
> > argv[3] = NULL;
> >
> > error = call_usermodehelper(nfsd_recall_failed, argv, envp,
> > @@ -657,6 +672,7 @@ nfsd4_cb_layout_done(struct nfsd4_callback *cb, struct rpc_task *task)
> > struct nfsd_net *nn;
> > ktime_t now, cutoff;
> > const struct nfsd4_layout_ops *ops;
> > + struct nfsd_file *fl;
> >
> > trace_nfsd_cb_layout_done(&ls->ls_stid.sc_stateid, task);
> > switch (task->tk_status) {
> > @@ -688,12 +704,17 @@ nfsd4_cb_layout_done(struct nfsd4_callback *cb, struct rpc_task *task)
> > * Unknown error or non-responding client, we'll need to fence.
> > */
> > trace_nfsd_layout_recall_fail(&ls->ls_stid.sc_stateid);
> > -
> > - ops = nfsd4_layout_ops[ls->ls_layout_type];
> > - if (ops->fence_client)
> > - ops->fence_client(ls);
> > - else
> > - nfsd4_cb_layout_fail(ls);
> > + rcu_read_lock();
> > + fl = nfsd_file_get(ls->ls_file);
> > + rcu_read_unlock();
>
> What is the rcu_read_lock protecting here? AFAICT, you don't need it
> since you hold a reference to "ls".
The new nfsd4_close_layout() can be called asynchronously (by admin
unlocking a filesystem) and will set ->ls_file to NULL, and then
nfsd_file_put() the file.
Without rcu_read_lock(), the pointer we get when dereferencing ls->ls_file
could have been passed to nfsd_file_put() and thence nfsd_file_free()
before nfsd_file_get() is called.
The rcu_read_lock() ensures that ->nf_ref from the pointer is still a valid
refcount and that it is safe for refcount_inc_not_zero() to be called on
it in nfsd_file_get().
Thanks for all you Reviewed-by!
NeilBrown
>
> > + if (fl) {
> > + ops = nfsd4_layout_ops[ls->ls_layout_type];
> > + if (ops->fence_client)
> > + ops->fence_client(ls, fl);
> > + else
> > + nfsd4_cb_layout_fail(ls, fl);
> > + nfsd_file_put(fl);
> > + }
> > return 1;
> > case -NFS4ERR_NOMATCHING_LAYOUT:
> > trace_nfsd_layout_recall_done(&ls->ls_stid.sc_stateid);
> > diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
> > index e749d5c0e23a..268b47a6f3b6 100644
> > --- a/fs/nfsd/nfs4state.c
> > +++ b/fs/nfsd/nfs4state.c
> > @@ -1721,7 +1721,7 @@ void nfsd4_revoke_states(struct net *net, struct super_block *sb)
> > unsigned int idhashval;
> > unsigned int sc_types;
> >
> > - sc_types = SC_TYPE_OPEN | SC_TYPE_LOCK | SC_TYPE_DELEG;
> > + sc_types = SC_TYPE_OPEN | SC_TYPE_LOCK | SC_TYPE_DELEG | SC_TYPE_LAYOUT;
> >
> > spin_lock(&nn->client_lock);
> > for (idhashval = 0; idhashval < CLIENT_HASH_MASK; idhashval++) {
> > @@ -1734,6 +1734,7 @@ void nfsd4_revoke_states(struct net *net, struct super_block *sb)
> > if (stid) {
> > struct nfs4_ol_stateid *stp;
> > struct nfs4_delegation *dp;
> > + struct nfs4_layout_stateid *ls;
> >
> > spin_unlock(&nn->client_lock);
> > switch (stid->sc_type) {
> > @@ -1789,6 +1790,10 @@ void nfsd4_revoke_states(struct net *net, struct super_block *sb)
> > if (dp)
> > revoke_delegation(dp);
> > break;
> > + case SC_TYPE_LAYOUT:
> > + ls = layoutstateid(stid);
> > + nfsd4_close_layout(ls);
> > + break;
> > }
> > nfs4_put_stid(stid);
> > spin_lock(&nn->client_lock);
> > @@ -2868,7 +2873,6 @@ static int nfs4_show_layout(struct seq_file *s, struct nfs4_stid *st)
> > struct nfsd_file *file;
> >
> > ls = container_of(st, struct nfs4_layout_stateid, ls_stid);
> > - file = ls->ls_file;
> >
> > seq_puts(s, "- ");
> > nfs4_show_stateid(s, &st->sc_stateid);
> > @@ -2876,12 +2880,15 @@ static int nfs4_show_layout(struct seq_file *s, struct nfs4_stid *st)
> >
> > /* XXX: What else would be useful? */
> >
> > + spin_lock(&ls->ls_stid.sc_file->fi_lock);
> > + file = ls->ls_file;
> > if (file) {
> > seq_puts(s, ", ");
> > nfs4_show_superblock(s, file);
> > seq_puts(s, ", ");
> > nfs4_show_fname(s, file);
> > }
> > + spin_unlock(&ls->ls_stid.sc_file->fi_lock);
> > if (st->sc_status & SC_STATUS_ADMIN_REVOKED)
> > seq_puts(s, ", admin-revoked");
> > seq_puts(s, " }\n");
> > diff --git a/fs/nfsd/pnfs.h b/fs/nfsd/pnfs.h
> > index de1e0dfed06a..925817f66917 100644
> > --- a/fs/nfsd/pnfs.h
> > +++ b/fs/nfsd/pnfs.h
> > @@ -37,7 +37,8 @@ struct nfsd4_layout_ops {
> > __be32 (*proc_layoutcommit)(struct inode *inode,
> > struct nfsd4_layoutcommit *lcp);
> >
> > - void (*fence_client)(struct nfs4_layout_stateid *ls);
> > + void (*fence_client)(struct nfs4_layout_stateid *ls,
> > + struct nfsd_file *file);
> > };
> >
> > extern const struct nfsd4_layout_ops *nfsd4_layout_ops[];
> > @@ -72,11 +73,13 @@ void nfsd4_setup_layout_type(struct svc_export *exp);
> > void nfsd4_return_all_client_layouts(struct nfs4_client *);
> > void nfsd4_return_all_file_layouts(struct nfs4_client *clp,
> > struct nfs4_file *fp);
> > +void nfsd4_close_layout(struct nfs4_layout_stateid *ls);
> > int nfsd4_init_pnfs(void);
> > void nfsd4_exit_pnfs(void);
> > #else
> > struct nfs4_client;
> > struct nfs4_file;
> > +struct nfs4_layout_stateid;
> >
> > static inline void nfsd4_setup_layout_type(struct svc_export *exp)
> > {
> > @@ -89,6 +92,9 @@ static inline void nfsd4_return_all_file_layouts(struct nfs4_client *clp,
> > struct nfs4_file *fp)
> > {
> > }
> > +static inline void nfsd4_close_layout(struct nfs4_layout_stateid *ls)
> > +{
> > +}
> > static inline void nfsd4_exit_pnfs(void)
> > {
> > }
>
> --
> Jeff Layton <jlayton@kernel.org>
>
>
^ permalink raw reply [flat|nested] 27+ messages in thread