Chuck Lever found this caused a regression, and Trond's fix hasn't hit
Linus's tree yet.
--b.
On Wed, Jan 24, 2018 at 04:14:53AM +0000, Sasha Levin wrote:
> From: Trond Myklebust <trond.myklebust@xxxxxxxxxxxxxxx>
>
> [ Upstream commit 659aefb68eca28ba9aa482a9fc64de107332e256 ]
>
> In order to deal with lookup races, nfsd4_free_lock_stateid() needs
> to be able to signal to other stateful functions that the lock stateid
> is no longer valid. Right now, nfsd_lock() will check whether or not an
> existing stateid is still hashed, but only in the "new lock" path.
>
> To ensure the stateid invalidation is also recognised by the "existing lock"
> path, and also by a second call to nfsd4_free_lock_stateid() itself, we can
> change the type to NFS4_CLOSED_STID under the stp->st_mutex.
>
> Signed-off-by: Trond Myklebust <trond.myklebust@xxxxxxxxxxxxxxx>
> Signed-off-by: J. Bruce Fields <bfields@xxxxxxxxxx>
> Signed-off-by: Sasha Levin <alexander.levin@xxxxxxxxxxxxx>
> ---
> fs/nfsd/nfs4state.c | 19 ++++++++-----------
> 1 file changed, 8 insertions(+), 11 deletions(-)
>
> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
> index 61f38346ce9d..5ec0ca5cbc1e 100644
> --- a/fs/nfsd/nfs4state.c
> +++ b/fs/nfsd/nfs4state.c
> @@ -5099,7 +5099,9 @@ nfsd4_free_lock_stateid(stateid_t *stateid, struct nfs4_stid *s)
> struct nfs4_ol_stateid *stp = openlockstateid(s);
> __be32 ret;
>
> - mutex_lock(&stp->st_mutex);
> + ret = nfsd4_lock_ol_stateid(stp);
> + if (ret)
> + goto out_put_stid;
>
> ret = check_stateid_generation(stateid, &s->sc_stateid, 1);
> if (ret)
> @@ -5110,11 +5112,13 @@ nfsd4_free_lock_stateid(stateid_t *stateid, struct nfs4_stid *s)
> lockowner(stp->st_stateowner)))
> goto out;
>
> + stp->st_stid.sc_type = NFS4_CLOSED_STID;
> release_lock_stateid(stp);
> ret = nfs_ok;
>
> out:
> mutex_unlock(&stp->st_mutex);
> +out_put_stid:
> nfs4_put_stid(s);
> return ret;
> }
> @@ -5683,6 +5687,8 @@ find_lock_stateid(struct nfs4_lockowner *lo, struct nfs4_file *fp)
> lockdep_assert_held(&clp->cl_lock);
>
> list_for_each_entry(lst, &lo->lo_owner.so_stateids, st_perstateowner) {
> + if (lst->st_stid.sc_type != NFS4_LOCK_STID)
> + continue;
> if (lst->st_stid.sc_file == fp) {
> atomic_inc(&lst->st_stid.sc_count);
> return lst;
> @@ -5757,7 +5763,6 @@ lookup_or_create_lock_state(struct nfsd4_compound_state *cstate,
> struct nfs4_lockowner *lo;
> struct nfs4_ol_stateid *lst;
> unsigned int strhashval;
> - bool hashed;
>
> lo = find_lockowner_str(cl, &lock->lk_new_owner);
> if (!lo) {
> @@ -5780,15 +5785,7 @@ retry:
> goto out;
> }
>
> - mutex_lock(&lst->st_mutex);
> -
> - /* See if it's still hashed to avoid race with FREE_STATEID */
> - spin_lock(&cl->cl_lock);
> - hashed = !list_empty(&lst->st_perfile);
> - spin_unlock(&cl->cl_lock);
> -
> - if (!hashed) {
> - mutex_unlock(&lst->st_mutex);
> + if (nfsd4_lock_ol_stateid(lst) != nfs_ok) {
> nfs4_put_stid(&lst->st_stid);
> goto retry;
> }
> --
> 2.11.0