Skip to content

Commit 1054e8f

Browse files
jtlaytonchucklever
authored andcommitted
nfsd: prevent callback tasks running concurrently
The nfsd4_callback workqueue jobs exist to queue backchannel RPCs to rpciod. Because they run in different workqueue contexts, the rpc_task can run concurrently with the workqueue job itself, should it become requeued. This is problematic as there is no locking when accessing the fields in the nfsd4_callback. Add a new unsigned long to nfsd4_callback and declare a new NFSD4_CALLBACK_RUNNING flag to be set in it. When attempting to run a workqueue job, do a test_and_set_bit() on that flag first, and don't queue the workqueue job if it returns true. Clear NFSD4_CALLBACK_RUNNING in nfsd41_destroy_cb(). This also gives us a more reliable mechanism for handling queueing failures in codepaths where we have to take references under spinlocks. We can now do the test_and_set_bit on NFSD4_CALLBACK_RUNNING first, and only take references to the objects if that returns false. Most of the nfsd4_run_cb() callers are converted to use this new flag or the nfsd4_try_run_cb() wrapper. The main exception is the callback channel probe, which has its own synchronization. Signed-off-by: Jeff Layton <jlayton@kernel.org> Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
1 parent 9254c8a commit 1054e8f

File tree

5 files changed

+26
-8
lines changed

5 files changed

+26
-8
lines changed

fs/nfsd/nfs4callback.c

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1312,6 +1312,7 @@ static void nfsd41_destroy_cb(struct nfsd4_callback *cb)
13121312

13131313
trace_nfsd_cb_destroy(clp, cb);
13141314
nfsd41_cb_release_slot(cb);
1315+
clear_bit(NFSD4_CALLBACK_RUNNING, &cb->cb_flags);
13151316
if (cb->cb_ops && cb->cb_ops->release)
13161317
cb->cb_ops->release(cb);
13171318
nfsd41_cb_inflight_end(clp);
@@ -1632,6 +1633,7 @@ void nfsd4_init_cb(struct nfsd4_callback *cb, struct nfs4_client *clp,
16321633
cb->cb_msg.rpc_proc = &nfs4_cb_procedures[op];
16331634
cb->cb_msg.rpc_argp = cb;
16341635
cb->cb_msg.rpc_resp = cb;
1636+
cb->cb_flags = 0;
16351637
cb->cb_ops = ops;
16361638
INIT_WORK(&cb->cb_work, nfsd4_run_cb_work);
16371639
cb->cb_status = 0;

fs/nfsd/nfs4layouts.c

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -344,9 +344,10 @@ nfsd4_recall_file_layout(struct nfs4_layout_stateid *ls)
344344
atomic_inc(&ls->ls_stid.sc_file->fi_lo_recalls);
345345
trace_nfsd_layout_recall(&ls->ls_stid.sc_stateid);
346346

347-
refcount_inc(&ls->ls_stid.sc_count);
348-
nfsd4_run_cb(&ls->ls_recall);
349-
347+
if (!test_and_set_bit(NFSD4_CALLBACK_RUNNING, &ls->ls_recall.cb_flags)) {
348+
refcount_inc(&ls->ls_stid.sc_count);
349+
nfsd4_run_cb(&ls->ls_recall);
350+
}
350351
out_unlock:
351352
spin_unlock(&ls->ls_lock);
352353
}

fs/nfsd/nfs4proc.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1847,7 +1847,7 @@ static void nfsd4_send_cb_offload(struct nfsd4_copy *copy)
18471847
NFSPROC4_CLNT_CB_OFFLOAD);
18481848
trace_nfsd_cb_offload(copy->cp_clp, &cbo->co_res.cb_stateid,
18491849
&cbo->co_fh, copy->cp_count, copy->nfserr);
1850-
nfsd4_run_cb(&cbo->co_cb);
1850+
nfsd4_try_run_cb(&cbo->co_cb);
18511851
}
18521852

18531853
/**

fs/nfsd/nfs4state.c

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3232,8 +3232,10 @@ static void nfs4_cb_getattr(struct nfs4_cb_fattr *ncf)
32323232
/* set to proper status when nfsd4_cb_getattr_done runs */
32333233
ncf->ncf_cb_status = NFS4ERR_IO;
32343234

3235-
refcount_inc(&dp->dl_stid.sc_count);
3236-
nfsd4_run_cb(&ncf->ncf_getattr);
3235+
if (!test_and_set_bit(NFSD4_CALLBACK_RUNNING, &ncf->ncf_getattr.cb_flags)) {
3236+
refcount_inc(&dp->dl_stid.sc_count);
3237+
nfsd4_run_cb(&ncf->ncf_getattr);
3238+
}
32373239
}
32383240

32393241
static struct nfs4_client *create_client(struct xdr_netobj name,
@@ -5422,6 +5424,10 @@ static const struct nfsd4_callback_ops nfsd4_cb_recall_ops = {
54225424
static void nfsd_break_one_deleg(struct nfs4_delegation *dp)
54235425
{
54245426
bool queued;
5427+
5428+
if (test_and_set_bit(NFSD4_CALLBACK_RUNNING, &dp->dl_recall.cb_flags))
5429+
return;
5430+
54255431
/*
54265432
* We're assuming the state code never drops its reference
54275433
* without first removing the lease. Since we're in this lease
@@ -6910,7 +6916,7 @@ deleg_reaper(struct nfsd_net *nn)
69106916
clp->cl_ra->ra_bmval[0] = BIT(RCA4_TYPE_MASK_RDATA_DLG) |
69116917
BIT(RCA4_TYPE_MASK_WDATA_DLG);
69126918
trace_nfsd_cb_recall_any(clp->cl_ra);
6913-
nfsd4_run_cb(&clp->cl_ra->ra_cb);
6919+
nfsd4_try_run_cb(&clp->cl_ra->ra_cb);
69146920
}
69156921
}
69166922

@@ -7839,7 +7845,7 @@ nfsd4_lm_notify(struct file_lock *fl)
78397845

78407846
if (queue) {
78417847
trace_nfsd_cb_notify_lock(lo, nbl);
7842-
nfsd4_run_cb(&nbl->nbl_cb);
7848+
nfsd4_try_run_cb(&nbl->nbl_cb);
78437849
}
78447850
}
78457851

fs/nfsd/state.h

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,8 @@ typedef struct {
6767
struct nfsd4_callback {
6868
struct nfs4_client *cb_clp;
6969
struct rpc_message cb_msg;
70+
#define NFSD4_CALLBACK_RUNNING (0)
71+
unsigned long cb_flags;
7072
const struct nfsd4_callback_ops *cb_ops;
7173
struct work_struct cb_work;
7274
int cb_seq_status;
@@ -780,6 +782,13 @@ extern void nfsd4_change_callback(struct nfs4_client *clp, struct nfs4_cb_conn *
780782
extern void nfsd4_init_cb(struct nfsd4_callback *cb, struct nfs4_client *clp,
781783
const struct nfsd4_callback_ops *ops, enum nfsd4_cb_op op);
782784
extern bool nfsd4_run_cb(struct nfsd4_callback *cb);
785+
786+
static inline void nfsd4_try_run_cb(struct nfsd4_callback *cb)
787+
{
788+
if (!test_and_set_bit(NFSD4_CALLBACK_RUNNING, &cb->cb_flags))
789+
WARN_ON_ONCE(!nfsd4_run_cb(cb));
790+
}
791+
783792
extern void nfsd4_shutdown_callback(struct nfs4_client *);
784793
extern void nfsd4_shutdown_copy(struct nfs4_client *clp);
785794
void nfsd4_async_copy_reaper(struct nfsd_net *nn);

0 commit comments

Comments
 (0)