Skip to content

Commit

Permalink
nfsd: drop st_mutex before calling move_to_close_lru()
Browse files Browse the repository at this point in the history
move_to_close_lru() is currently called with ->st_mutex held.
This can lead to a deadlock as move_to_close_lru() waits for sc_count to
drop to 2, and some threads holding a reference might be waiting for the
mutex.  These references will never be dropped so sc_count will never
reach 2.

There can be no harm in dropping ->st_mutex before
move_to_close_lru() because the only place that takes the mutex is
nfsd4_lock_ol_stateid(), and it quickly aborts if sc_type is
NFS4_CLOSED_STID, which it will be before move_to_close_lru() is called.

See also
 https://lore.kernel.org/lkml/[email protected]/T/
where this problem was raised but not successfully resolved.

Reviewed-by: Jeff Layton <[email protected]>
Signed-off-by: NeilBrown <[email protected]>
Signed-off-by: Chuck Lever <[email protected]>
  • Loading branch information
neilbrown authored and chucklever committed May 6, 2024
1 parent eec7620 commit 56c35f4
Showing 1 changed file with 7 additions and 4 deletions.
11 changes: 7 additions & 4 deletions fs/nfsd/nfs4state.c
Original file line number Diff line number Diff line change
Expand Up @@ -7357,7 +7357,7 @@ nfsd4_open_downgrade(struct svc_rqst *rqstp,
return status;
}

static void nfsd4_close_open_stateid(struct nfs4_ol_stateid *s)
static bool nfsd4_close_open_stateid(struct nfs4_ol_stateid *s)
{
struct nfs4_client *clp = s->st_stid.sc_client;
bool unhashed;
Expand All @@ -7374,11 +7374,11 @@ static void nfsd4_close_open_stateid(struct nfs4_ol_stateid *s)
list_for_each_entry(stp, &reaplist, st_locks)
nfs4_free_cpntf_statelist(clp->net, &stp->st_stid);
free_ol_stateid_reaplist(&reaplist);
return false;
} else {
spin_unlock(&clp->cl_lock);
free_ol_stateid_reaplist(&reaplist);
if (unhashed)
move_to_close_lru(s, clp->net);
return unhashed;
}
}

Expand All @@ -7394,6 +7394,7 @@ nfsd4_close(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
struct nfs4_ol_stateid *stp;
struct net *net = SVC_NET(rqstp);
struct nfsd_net *nn = net_generic(net, nfsd_net_id);
bool need_move_to_close_list;

dprintk("NFSD: nfsd4_close on file %pd\n",
cstate->current_fh.fh_dentry);
Expand All @@ -7418,8 +7419,10 @@ nfsd4_close(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
*/
nfs4_inc_and_copy_stateid(&close->cl_stateid, &stp->st_stid);

nfsd4_close_open_stateid(stp);
need_move_to_close_list = nfsd4_close_open_stateid(stp);
mutex_unlock(&stp->st_mutex);
if (need_move_to_close_list)
move_to_close_lru(stp, net);

/* v4.1+ suggests that we send a special stateid in here, since the
* clients should just ignore this anyway. Since this is not useful
Expand Down

0 comments on commit 56c35f4

Please sign in to comment.