Skip to content

Commit eec7620

Browse files
neilbrownchucklever
authored andcommitted
nfsd: replace rp_mutex to avoid deadlock in move_to_close_lru()
move_to_close_lru() waits for sc_count to become zero while holding rp_mutex. This can deadlock if another thread holds a reference and is waiting for rp_mutex. By the time we get to move_to_close_lru() the openowner is unhashed and cannot be found any more. So code waiting for the mutex can safely retry the lookup if move_to_close_lru() has started. So change rp_mutex to an atomic_t with three states: RP_UNLOCK - state is still hashed, not locked for reply RP_LOCKED - state is still hashed, is locked for reply RP_UNHASHED - state is not hashed, no code can get a lock. Use wait_var_event() to wait for either a lock, or for the owner to be unhashed. In the latter case, retry the lookup. Reviewed-by: Jeff Layton <[email protected]> Signed-off-by: NeilBrown <[email protected]> Signed-off-by: Chuck Lever <[email protected]>
1 parent b3f0373 commit eec7620

File tree

2 files changed

+32
-8
lines changed

2 files changed

+32
-8
lines changed

fs/nfsd/nfs4state.c

Lines changed: 31 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -4650,21 +4650,32 @@ nfsd4_init_leases_net(struct nfsd_net *nn)
46504650
atomic_set(&nn->nfsd_courtesy_clients, 0);
46514651
}
46524652

4653+
enum rp_lock {
4654+
RP_UNLOCKED,
4655+
RP_LOCKED,
4656+
RP_UNHASHED,
4657+
};
4658+
46534659
static void init_nfs4_replay(struct nfs4_replay *rp)
46544660
{
46554661
rp->rp_status = nfserr_serverfault;
46564662
rp->rp_buflen = 0;
46574663
rp->rp_buf = rp->rp_ibuf;
4658-
mutex_init(&rp->rp_mutex);
4664+
atomic_set(&rp->rp_locked, RP_UNLOCKED);
46594665
}
46604666

4661-
static void nfsd4_cstate_assign_replay(struct nfsd4_compound_state *cstate,
4662-
struct nfs4_stateowner *so)
4667+
static int nfsd4_cstate_assign_replay(struct nfsd4_compound_state *cstate,
4668+
struct nfs4_stateowner *so)
46634669
{
46644670
if (!nfsd4_has_session(cstate)) {
4665-
mutex_lock(&so->so_replay.rp_mutex);
4671+
wait_var_event(&so->so_replay.rp_locked,
4672+
atomic_cmpxchg(&so->so_replay.rp_locked,
4673+
RP_UNLOCKED, RP_LOCKED) != RP_LOCKED);
4674+
if (atomic_read(&so->so_replay.rp_locked) == RP_UNHASHED)
4675+
return -EAGAIN;
46664676
cstate->replay_owner = nfs4_get_stateowner(so);
46674677
}
4678+
return 0;
46684679
}
46694680

46704681
void nfsd4_cstate_clear_replay(struct nfsd4_compound_state *cstate)
@@ -4673,7 +4684,8 @@ void nfsd4_cstate_clear_replay(struct nfsd4_compound_state *cstate)
46734684

46744685
if (so != NULL) {
46754686
cstate->replay_owner = NULL;
4676-
mutex_unlock(&so->so_replay.rp_mutex);
4687+
atomic_set(&so->so_replay.rp_locked, RP_UNLOCKED);
4688+
wake_up_var(&so->so_replay.rp_locked);
46774689
nfs4_put_stateowner(so);
46784690
}
46794691
}
@@ -4969,7 +4981,11 @@ move_to_close_lru(struct nfs4_ol_stateid *s, struct net *net)
49694981
* Wait for the refcount to drop to 2. Since it has been unhashed,
49704982
* there should be no danger of the refcount going back up again at
49714983
* this point.
4984+
* Some threads with a reference might be waiting for rp_locked,
4985+
* so tell them to stop waiting.
49724986
*/
4987+
atomic_set(&oo->oo_owner.so_replay.rp_locked, RP_UNHASHED);
4988+
wake_up_var(&oo->oo_owner.so_replay.rp_locked);
49734989
wait_event(close_wq, refcount_read(&s->st_stid.sc_count) == 2);
49744990

49754991
release_all_access(s);
@@ -5342,11 +5358,15 @@ nfsd4_process_open1(struct nfsd4_compound_state *cstate,
53425358
clp = cstate->clp;
53435359

53445360
strhashval = ownerstr_hashval(&open->op_owner);
5361+
retry:
53455362
oo = find_or_alloc_open_stateowner(strhashval, open, cstate);
53465363
open->op_openowner = oo;
53475364
if (!oo)
53485365
return nfserr_jukebox;
5349-
nfsd4_cstate_assign_replay(cstate, &oo->oo_owner);
5366+
if (nfsd4_cstate_assign_replay(cstate, &oo->oo_owner) == -EAGAIN) {
5367+
nfs4_put_stateowner(&oo->oo_owner);
5368+
goto retry;
5369+
}
53505370
status = nfsd4_check_seqid(cstate, &oo->oo_owner, open->op_seqid);
53515371
if (status)
53525372
return status;
@@ -7186,12 +7206,16 @@ nfs4_preprocess_seqid_op(struct nfsd4_compound_state *cstate, u32 seqid,
71867206
trace_nfsd_preprocess(seqid, stateid);
71877207

71887208
*stpp = NULL;
7209+
retry:
71897210
status = nfsd4_lookup_stateid(cstate, stateid,
71907211
typemask, statusmask, &s, nn);
71917212
if (status)
71927213
return status;
71937214
stp = openlockstateid(s);
7194-
nfsd4_cstate_assign_replay(cstate, stp->st_stateowner);
7215+
if (nfsd4_cstate_assign_replay(cstate, stp->st_stateowner) == -EAGAIN) {
7216+
nfs4_put_stateowner(stp->st_stateowner);
7217+
goto retry;
7218+
}
71957219

71967220
status = nfs4_seqid_op_checks(cstate, stateid, seqid, stp);
71977221
if (!status)

fs/nfsd/state.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -486,7 +486,7 @@ struct nfs4_replay {
486486
unsigned int rp_buflen;
487487
char *rp_buf;
488488
struct knfsd_fh rp_openfh;
489-
struct mutex rp_mutex;
489+
atomic_t rp_locked;
490490
char rp_ibuf[NFSD4_REPLAY_ISIZE];
491491
};
492492

0 commit comments

Comments
 (0)