| Message ID | 20220926184102.57254-1-jlayton@kernel.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2,1/2] nfsd: make nfsd4_run_cb a bool return function | expand |
> On Sep 26, 2022, at 2:41 PM, Jeff Layton <jlayton@kernel.org> wrote: > > queue_work can return false and not queue anything, if the work is > already queued. If that happens in the case of a CB_RECALL, we'll have > taken an extra reference to the stid that will never be put. Ensure we > throw a warning in that case. > > Signed-off-by: Jeff Layton <jlayton@kernel.org> > --- > fs/nfsd/nfs4callback.c | 14 ++++++++++++-- > fs/nfsd/nfs4state.c | 5 ++--- > fs/nfsd/state.h | 2 +- > 3 files changed, 15 insertions(+), 6 deletions(-) Thank you, Jeff. Both applied to nfsd's for-next. > diff --git a/fs/nfsd/nfs4callback.c b/fs/nfsd/nfs4callback.c > index 4ce328209f61..c4ee4a820a62 100644 > --- a/fs/nfsd/nfs4callback.c > +++ b/fs/nfsd/nfs4callback.c > @@ -1371,11 +1371,21 @@ void nfsd4_init_cb(struct nfsd4_callback *cb, struct nfs4_client *clp, > cb->cb_holds_slot = false; > } > > -void nfsd4_run_cb(struct nfsd4_callback *cb) > +/** > + * nfsd4_run_cb - queue up a callback job to run > + * @cb: callback to queue > + * > + * Kick off a callback to do its thing. Returns false if it was already > + * on a queue, true otherwise. > + */ > +bool nfsd4_run_cb(struct nfsd4_callback *cb) > { > + bool queued; > struct nfs4_client *clp = cb->cb_clp; > > nfsd41_cb_inflight_begin(clp); > - if (!nfsd4_queue_cb(cb)) > + queued = nfsd4_queue_cb(cb); > + if (!queued) > nfsd41_cb_inflight_end(clp); > + return queued; > } > diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c > index e116f50afcf2..c78c3223161e 100644 > --- a/fs/nfsd/nfs4state.c > +++ b/fs/nfsd/nfs4state.c > @@ -4859,14 +4859,13 @@ static void nfsd_break_one_deleg(struct nfs4_delegation *dp) > * we know it's safe to take a reference. > */ > refcount_inc(&dp->dl_stid.sc_count); > - nfsd4_run_cb(&dp->dl_recall); > + WARN_ON_ONCE(!nfsd4_run_cb(&dp->dl_recall)); > } > > /* Called from break_lease() with flc_lock held. */ > static bool > nfsd_break_deleg_cb(struct file_lock *fl) > { > - bool ret = false; > struct nfs4_delegation *dp = (struct nfs4_delegation *)fl->fl_owner; > struct nfs4_file *fp = dp->dl_stid.sc_file; > struct nfs4_client *clp = dp->dl_stid.sc_client; > @@ -4892,7 +4891,7 @@ nfsd_break_deleg_cb(struct file_lock *fl) > fp->fi_had_conflict = true; > nfsd_break_one_deleg(dp); > spin_unlock(&fp->fi_lock); > - return ret; > + return false; > } > > /** > diff --git a/fs/nfsd/state.h b/fs/nfsd/state.h > index b3477087a9fc..e2daef3cc003 100644 > --- a/fs/nfsd/state.h > +++ b/fs/nfsd/state.h > @@ -692,7 +692,7 @@ extern void nfsd4_probe_callback_sync(struct nfs4_client *clp); > extern void nfsd4_change_callback(struct nfs4_client *clp, struct nfs4_cb_conn *); > extern void nfsd4_init_cb(struct nfsd4_callback *cb, struct nfs4_client *clp, > const struct nfsd4_callback_ops *ops, enum nfsd4_cb_op op); > -extern void nfsd4_run_cb(struct nfsd4_callback *cb); > +extern bool nfsd4_run_cb(struct nfsd4_callback *cb); > extern int nfsd4_create_callback_queue(void); > extern void nfsd4_destroy_callback_queue(void); > extern void nfsd4_shutdown_callback(struct nfs4_client *); > -- > 2.37.3 > -- Chuck Lever
diff --git a/fs/nfsd/nfs4callback.c b/fs/nfsd/nfs4callback.c index 4ce328209f61..c4ee4a820a62 100644 --- a/fs/nfsd/nfs4callback.c +++ b/fs/nfsd/nfs4callback.c @@ -1371,11 +1371,21 @@ void nfsd4_init_cb(struct nfsd4_callback *cb, struct nfs4_client *clp, cb->cb_holds_slot = false; } -void nfsd4_run_cb(struct nfsd4_callback *cb) +/** + * nfsd4_run_cb - queue up a callback job to run + * @cb: callback to queue + * + * Kick off a callback to do its thing. Returns false if it was already + * on a queue, true otherwise. + */ +bool nfsd4_run_cb(struct nfsd4_callback *cb) { + bool queued; struct nfs4_client *clp = cb->cb_clp; nfsd41_cb_inflight_begin(clp); - if (!nfsd4_queue_cb(cb)) + queued = nfsd4_queue_cb(cb); + if (!queued) nfsd41_cb_inflight_end(clp); + return queued; } diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c index e116f50afcf2..c78c3223161e 100644 --- a/fs/nfsd/nfs4state.c +++ b/fs/nfsd/nfs4state.c @@ -4859,14 +4859,13 @@ static void nfsd_break_one_deleg(struct nfs4_delegation *dp) * we know it's safe to take a reference. */ refcount_inc(&dp->dl_stid.sc_count); - nfsd4_run_cb(&dp->dl_recall); + WARN_ON_ONCE(!nfsd4_run_cb(&dp->dl_recall)); } /* Called from break_lease() with flc_lock held. */ static bool nfsd_break_deleg_cb(struct file_lock *fl) { - bool ret = false; struct nfs4_delegation *dp = (struct nfs4_delegation *)fl->fl_owner; struct nfs4_file *fp = dp->dl_stid.sc_file; struct nfs4_client *clp = dp->dl_stid.sc_client; @@ -4892,7 +4891,7 @@ nfsd_break_deleg_cb(struct file_lock *fl) fp->fi_had_conflict = true; nfsd_break_one_deleg(dp); spin_unlock(&fp->fi_lock); - return ret; + return false; } /** diff --git a/fs/nfsd/state.h b/fs/nfsd/state.h index b3477087a9fc..e2daef3cc003 100644 --- a/fs/nfsd/state.h +++ b/fs/nfsd/state.h @@ -692,7 +692,7 @@ extern void nfsd4_probe_callback_sync(struct nfs4_client *clp); extern void nfsd4_change_callback(struct nfs4_client *clp, struct nfs4_cb_conn *); extern void nfsd4_init_cb(struct nfsd4_callback *cb, struct nfs4_client *clp, const struct nfsd4_callback_ops *ops, enum nfsd4_cb_op op); -extern void nfsd4_run_cb(struct nfsd4_callback *cb); +extern bool nfsd4_run_cb(struct nfsd4_callback *cb); extern int nfsd4_create_callback_queue(void); extern void nfsd4_destroy_callback_queue(void); extern void nfsd4_shutdown_callback(struct nfs4_client *);
queue_work can return false and not queue anything, if the work is already queued. If that happens in the case of a CB_RECALL, we'll have taken an extra reference to the stid that will never be put. Ensure we throw a warning in that case. Signed-off-by: Jeff Layton <jlayton@kernel.org> --- fs/nfsd/nfs4callback.c | 14 ++++++++++++-- fs/nfsd/nfs4state.c | 5 ++--- fs/nfsd/state.h | 2 +- 3 files changed, 15 insertions(+), 6 deletions(-)