On Wed, 2025-04-23 at 13:59 -0400, Benjamin Coddington wrote:
> A follow-up patch intends to signal the success or failure of FREE_STATEID
> by modifying the nfs4_stateid's type which requires the const qualifier for
> the nfs4_stateid to be dropped. Since it will no longer safe to operate
> directly on shared stateid objects in this path, ensure that callers send a
> copy.
>
> Signed-off-by: Benjamin Coddington <bcodding@xxxxxxxxxx>
> ---
> fs/nfs/nfs4proc.c | 12 +++++++-----
> 1 file changed, 7 insertions(+), 5 deletions(-)
>
> diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
> index 6e95db6c17e9..bfb9e980d662 100644
> --- a/fs/nfs/nfs4proc.c
> +++ b/fs/nfs/nfs4proc.c
> @@ -2990,6 +2990,7 @@ static void nfs41_delegation_recover_stateid(struct nfs4_state *state)
> static int nfs41_check_expired_locks(struct nfs4_state *state)
> {
> int status, ret = NFS_OK;
> + nfs4_stateid stateid;
> struct nfs4_lock_state *lsp, *prev = NULL;
> struct nfs_server *server = NFS_SERVER(state->inode);
>
> @@ -3007,9 +3008,9 @@ static int nfs41_check_expired_locks(struct nfs4_state *state)
> nfs4_put_lock_state(prev);
> prev = lsp;
>
> + nfs4_stateid_copy(&stateid, &lsp->ls_stateid);
> status = nfs41_test_and_free_expired_stateid(server,
> - &lsp->ls_stateid,
> - cred);
> + &stateid, cred);
> trace_nfs4_test_lock_stateid(state, lsp, status);
> if (status == -NFS4ERR_EXPIRED ||
> status == -NFS4ERR_BAD_STATEID) {
> @@ -3042,17 +3043,18 @@ static int nfs41_check_expired_locks(struct nfs4_state *state)
> static int nfs41_check_open_stateid(struct nfs4_state *state)
> {
> struct nfs_server *server = NFS_SERVER(state->inode);
> - nfs4_stateid *stateid = &state->open_stateid;
> + nfs4_stateid stateid;
> const struct cred *cred = state->owner->so_cred;
> int status;
>
> if (test_bit(NFS_OPEN_STATE, &state->flags) == 0)
> return -NFS4ERR_BAD_STATEID;
> - status = nfs41_test_and_free_expired_stateid(server, stateid, cred);
> + nfs4_stateid_copy(&stateid, &state->open_stateid);
> + status = nfs41_test_and_free_expired_stateid(server, &stateid, cred);
> trace_nfs4_test_open_stateid(state, NULL, status);
> if (status == -NFS4ERR_EXPIRED || status == -NFS4ERR_BAD_STATEID) {
> nfs_state_clear_open_state_flags(state);
> - stateid->type = NFS4_INVALID_STATEID_TYPE;
> + state->open_stateid.type = NFS4_INVALID_STATEID_TYPE;
> return status;
> }
> if (nfs_open_stateid_recover_openmode(state))
I don't know that you really need to do this. In the cases where you
end up setting the type to FREED, you will also return NFS4ERR_EXPIRED,
which will make the callers set the type to INVALID.
There will be a brief window where the type will be set to FREED, but
that should be no big deal.
--
Jeff Layton <jlayton@xxxxxxxxxx>
