* [PATCH 1/4] nfsd: remove nfsd4_callback.cb_op
2014-09-24 10:19 refactor nfsd callback handling Christoph Hellwig
@ 2014-09-24 10:19 ` Christoph Hellwig
2014-09-24 10:19 ` [PATCH 2/4] nfsd: introduce a generic nfsd4_cb Christoph Hellwig
` (4 subsequent siblings)
5 siblings, 0 replies; 9+ messages in thread
From: Christoph Hellwig @ 2014-09-24 10:19 UTC (permalink / raw)
To: J. Bruce Fields; +Cc: linux-nfs
We can always get at the private data by using container_of, no need for
a void pointer. Also introduce a little to_delegation helper to avoid
opencoding the container_of everywhere.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
fs/nfsd/nfs4callback.c | 17 +++++++++--------
fs/nfsd/state.h | 1 -
2 files changed, 9 insertions(+), 9 deletions(-)
diff --git a/fs/nfsd/nfs4callback.c b/fs/nfsd/nfs4callback.c
index fc07084..06e07e1 100644
--- a/fs/nfsd/nfs4callback.c
+++ b/fs/nfsd/nfs4callback.c
@@ -55,6 +55,9 @@ enum {
NFSPROC4_CLNT_CB_SEQUENCE,
};
+#define to_delegation(cb) \
+ container_of(cb, struct nfs4_delegation, dl_recall)
+
struct nfs4_cb_compound_hdr {
/* args */
u32 ident; /* minorversion 0 only */
@@ -494,7 +497,7 @@ static void nfs4_xdr_enc_cb_null(struct rpc_rqst *req, struct xdr_stream *xdr,
static void nfs4_xdr_enc_cb_recall(struct rpc_rqst *req, struct xdr_stream *xdr,
const struct nfsd4_callback *cb)
{
- const struct nfs4_delegation *args = cb->cb_op;
+ const struct nfs4_delegation *dp = to_delegation(cb);
struct nfs4_cb_compound_hdr hdr = {
.ident = cb->cb_clp->cl_cb_ident,
.minorversion = cb->cb_minorversion,
@@ -502,7 +505,7 @@ static void nfs4_xdr_enc_cb_recall(struct rpc_rqst *req, struct xdr_stream *xdr,
encode_cb_compound4args(xdr, &hdr);
encode_cb_sequence4args(xdr, cb, &hdr);
- encode_cb_recall4args(xdr, args, &hdr);
+ encode_cb_recall4args(xdr, dp, &hdr);
encode_cb_nops(&hdr);
}
@@ -755,7 +758,6 @@ static void do_probe_callback(struct nfs4_client *clp)
{
struct nfsd4_callback *cb = &clp->cl_cb_null;
- cb->cb_op = NULL;
cb->cb_clp = clp;
cb->cb_msg.rpc_proc = &nfs4_cb_procedures[NFSPROC4_CLNT_CB_NULL];
@@ -850,11 +852,10 @@ static void nfsd4_cb_done(struct rpc_task *task, void *calldata)
}
}
-
static void nfsd4_cb_recall_done(struct rpc_task *task, void *calldata)
{
struct nfsd4_callback *cb = calldata;
- struct nfs4_delegation *dp = container_of(cb, struct nfs4_delegation, dl_recall);
+ struct nfs4_delegation *dp = to_delegation(cb);
struct nfs4_client *clp = cb->cb_clp;
struct rpc_clnt *current_rpc_client = clp->cl_cb_client;
@@ -893,9 +894,10 @@ static void nfsd4_cb_recall_release(void *calldata)
{
struct nfsd4_callback *cb = calldata;
struct nfs4_client *clp = cb->cb_clp;
- struct nfs4_delegation *dp = container_of(cb, struct nfs4_delegation, dl_recall);
if (cb->cb_done) {
+ struct nfs4_delegation *dp = to_delegation(cb);
+
spin_lock(&clp->cl_lock);
list_del(&cb->cb_per_client);
spin_unlock(&clp->cl_lock);
@@ -1040,7 +1042,7 @@ nfsd4_run_cb_recall(struct work_struct *w)
struct nfsd4_callback *cb = container_of(w, struct nfsd4_callback,
cb_work);
- nfsd4_prepare_cb_recall(cb->cb_op);
+ nfsd4_prepare_cb_recall(to_delegation(cb));
nfsd4_run_callback_rpc(cb);
}
@@ -1050,7 +1052,6 @@ void nfsd4_cb_recall(struct nfs4_delegation *dp)
struct nfs4_client *clp = dp->dl_stid.sc_client;
dp->dl_retries = 1;
- cb->cb_op = dp;
cb->cb_clp = clp;
cb->cb_msg.rpc_proc = &nfs4_cb_procedures[NFSPROC4_CLNT_CB_RECALL];
cb->cb_msg.rpc_argp = cb;
diff --git a/fs/nfsd/state.h b/fs/nfsd/state.h
index 64f291a..74b0700 100644
--- a/fs/nfsd/state.h
+++ b/fs/nfsd/state.h
@@ -62,7 +62,6 @@ typedef struct {
(s)->si_generation
struct nfsd4_callback {
- void *cb_op;
struct nfs4_client *cb_clp;
struct list_head cb_per_client;
u32 cb_minorversion;
--
1.9.1
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH 2/4] nfsd: introduce a generic nfsd4_cb
2014-09-24 10:19 refactor nfsd callback handling Christoph Hellwig
2014-09-24 10:19 ` [PATCH 1/4] nfsd: remove nfsd4_callback.cb_op Christoph Hellwig
@ 2014-09-24 10:19 ` Christoph Hellwig
2014-09-24 10:19 ` [PATCH 3/4] nfsd: split nfsd4_callback initialization and use Christoph Hellwig
` (3 subsequent siblings)
5 siblings, 0 replies; 9+ messages in thread
From: Christoph Hellwig @ 2014-09-24 10:19 UTC (permalink / raw)
To: J. Bruce Fields; +Cc: linux-nfs
Add a helper to queue up a callback. CB_NULL has a bit of special casing
because it is special in the specification, but all other new callback
operations will be able to share code with this and a few more changes
to refactor the callback code.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
fs/nfsd/nfs4callback.c | 47 ++++++++++++-----------------------------------
fs/nfsd/nfs4state.c | 4 +++-
fs/nfsd/state.h | 10 +++++++++-
3 files changed, 24 insertions(+), 37 deletions(-)
diff --git a/fs/nfsd/nfs4callback.c b/fs/nfsd/nfs4callback.c
index 06e07e1..022edcf 100644
--- a/fs/nfsd/nfs4callback.c
+++ b/fs/nfsd/nfs4callback.c
@@ -49,12 +49,6 @@ static void nfsd4_mark_cb_fault(struct nfs4_client *, int reason);
/* Index of predefined Linux callback client operations */
-enum {
- NFSPROC4_CLNT_CB_NULL = 0,
- NFSPROC4_CLNT_CB_RECALL,
- NFSPROC4_CLNT_CB_SEQUENCE,
-};
-
#define to_delegation(cb) \
container_of(cb, struct nfs4_delegation, dl_recall)
@@ -749,24 +743,9 @@ static const struct rpc_call_ops nfsd4_cb_probe_ops = {
static struct workqueue_struct *callback_wq;
-static void run_nfsd4_cb(struct nfsd4_callback *cb)
-{
- queue_work(callback_wq, &cb->cb_work);
-}
-
static void do_probe_callback(struct nfs4_client *clp)
{
- struct nfsd4_callback *cb = &clp->cl_cb_null;
-
- cb->cb_clp = clp;
-
- cb->cb_msg.rpc_proc = &nfs4_cb_procedures[NFSPROC4_CLNT_CB_NULL];
- cb->cb_msg.rpc_argp = NULL;
- cb->cb_msg.rpc_resp = NULL;
-
- cb->cb_ops = &nfsd4_cb_probe_ops;
-
- run_nfsd4_cb(cb);
+ return nfsd4_cb(&clp->cl_cb_null, clp, NFSPROC4_CLNT_CB_NULL);
}
/*
@@ -1005,7 +984,7 @@ static void nfsd4_process_cb_update(struct nfsd4_callback *cb)
}
/* Yay, the callback channel's back! Restart any callbacks: */
list_for_each_entry(cb, &clp->cl_callbacks, cb_per_client)
- run_nfsd4_cb(cb);
+ queue_work(callback_wq, &cb->cb_work);
}
static void
@@ -1046,21 +1025,19 @@ nfsd4_run_cb_recall(struct work_struct *w)
nfsd4_run_callback_rpc(cb);
}
-void nfsd4_cb_recall(struct nfs4_delegation *dp)
+void nfsd4_cb(struct nfsd4_callback *cb, struct nfs4_client *clp,
+ enum nfsd4_cb_op op)
{
- struct nfsd4_callback *cb = &dp->dl_recall;
- struct nfs4_client *clp = dp->dl_stid.sc_client;
-
- dp->dl_retries = 1;
cb->cb_clp = clp;
- cb->cb_msg.rpc_proc = &nfs4_cb_procedures[NFSPROC4_CLNT_CB_RECALL];
+ cb->cb_msg.rpc_proc = &nfs4_cb_procedures[op];
cb->cb_msg.rpc_argp = cb;
cb->cb_msg.rpc_resp = cb;
+ if (op == NFSPROC4_CLNT_CB_NULL)
+ cb->cb_ops = &nfsd4_cb_probe_ops;
+ else
+ cb->cb_ops = &nfsd4_cb_recall_ops;
+ INIT_LIST_HEAD(&cb->cb_per_client);
+ cb->cb_done = true;
- cb->cb_ops = &nfsd4_cb_recall_ops;
-
- INIT_LIST_HEAD(&cb->cb_per_client);
- cb->cb_done = true;
-
- run_nfsd4_cb(&dp->dl_recall);
+ queue_work(callback_wq, &cb->cb_work);
}
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index 8597fbe..90b909e 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -3383,7 +3383,9 @@ static void nfsd_break_one_deleg(struct nfs4_delegation *dp)
* it's safe to take a reference.
*/
atomic_inc(&dp->dl_stid.sc_count);
- nfsd4_cb_recall(dp);
+ dp->dl_retries = 1;
+ nfsd4_cb(&dp->dl_recall, dp->dl_stid.sc_client,
+ NFSPROC4_CLNT_CB_RECALL);
}
/* Called from break_lease() with i_lock held. */
diff --git a/fs/nfsd/state.h b/fs/nfsd/state.h
index 74b0700..515408b 100644
--- a/fs/nfsd/state.h
+++ b/fs/nfsd/state.h
@@ -515,6 +515,13 @@ static inline struct nfs4_ol_stateid *openlockstateid(struct nfs4_stid *s)
#define RD_STATE 0x00000010
#define WR_STATE 0x00000020
+enum nfsd4_cb_op {
+ NFSPROC4_CLNT_CB_NULL = 0,
+ NFSPROC4_CLNT_CB_RECALL,
+ NFSPROC4_CLNT_CB_SEQUENCE,
+};
+
+
struct nfsd4_compound_state;
struct nfsd_net;
@@ -534,7 +541,8 @@ void nfsd4_run_cb_recall(struct work_struct *w);
extern void nfsd4_probe_callback(struct nfs4_client *clp);
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_cb_recall(struct nfs4_delegation *dp);
+extern void nfsd4_cb(struct nfsd4_callback *cb, struct nfs4_client *clp,
+ enum nfsd4_cb_op op);
extern int nfsd4_create_callback_queue(void);
extern void nfsd4_destroy_callback_queue(void);
extern void nfsd4_shutdown_callback(struct nfs4_client *);
--
1.9.1
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH 3/4] nfsd: split nfsd4_callback initialization and use
2014-09-24 10:19 refactor nfsd callback handling Christoph Hellwig
2014-09-24 10:19 ` [PATCH 1/4] nfsd: remove nfsd4_callback.cb_op Christoph Hellwig
2014-09-24 10:19 ` [PATCH 2/4] nfsd: introduce a generic nfsd4_cb Christoph Hellwig
@ 2014-09-24 10:19 ` Christoph Hellwig
2014-09-24 15:29 ` Jeff Layton
2014-09-24 10:19 ` [PATCH 4/4] nfsd: introduce nfsd4_callback_ops Christoph Hellwig
` (2 subsequent siblings)
5 siblings, 1 reply; 9+ messages in thread
From: Christoph Hellwig @ 2014-09-24 10:19 UTC (permalink / raw)
To: J. Bruce Fields; +Cc: linux-nfs
Split out initializing the nfs4_callback structure from using it. For
the NULL callback this gets rid of tons of pointless re-initializations.
Note that I don't quite understand what protects us from running multiple
NULL callbacks at the same time, but at least this chance doesn't make
it worse..
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
fs/nfsd/nfs4callback.c | 14 ++++++--------
fs/nfsd/nfs4state.c | 8 +++++---
fs/nfsd/state.h | 3 ++-
3 files changed, 13 insertions(+), 12 deletions(-)
diff --git a/fs/nfsd/nfs4callback.c b/fs/nfsd/nfs4callback.c
index 022edcf..6d9d947 100644
--- a/fs/nfsd/nfs4callback.c
+++ b/fs/nfsd/nfs4callback.c
@@ -743,11 +743,6 @@ static const struct rpc_call_ops nfsd4_cb_probe_ops = {
static struct workqueue_struct *callback_wq;
-static void do_probe_callback(struct nfs4_client *clp)
-{
- return nfsd4_cb(&clp->cl_cb_null, clp, NFSPROC4_CLNT_CB_NULL);
-}
-
/*
* Poke the callback thread to process any updates to the callback
* parameters, and send a null probe.
@@ -756,7 +751,7 @@ void nfsd4_probe_callback(struct nfs4_client *clp)
{
clp->cl_cb_state = NFSD4_CB_UNKNOWN;
set_bit(NFSD4_CLIENT_CB_UPDATE, &clp->cl_flags);
- do_probe_callback(clp);
+ nfsd4_run_cb(&clp->cl_cb_null);
}
void nfsd4_probe_callback_sync(struct nfs4_client *clp)
@@ -912,7 +907,7 @@ void nfsd4_shutdown_callback(struct nfs4_client *clp)
* instead, nfsd4_run_cb_null() will detect the killed
* client, destroy the rpc client, and stop:
*/
- do_probe_callback(clp);
+ nfsd4_run_cb(&clp->cl_cb_null);
flush_workqueue(callback_wq);
}
@@ -1025,7 +1020,7 @@ nfsd4_run_cb_recall(struct work_struct *w)
nfsd4_run_callback_rpc(cb);
}
-void nfsd4_cb(struct nfsd4_callback *cb, struct nfs4_client *clp,
+void nfsd4_init_cb(struct nfsd4_callback *cb, struct nfs4_client *clp,
enum nfsd4_cb_op op)
{
cb->cb_clp = clp;
@@ -1038,6 +1033,9 @@ void nfsd4_cb(struct nfsd4_callback *cb, struct nfs4_client *clp,
cb->cb_ops = &nfsd4_cb_recall_ops;
INIT_LIST_HEAD(&cb->cb_per_client);
cb->cb_done = true;
+}
+void nfsd4_run_cb(struct nfsd4_callback *cb)
+{
queue_work(callback_wq, &cb->cb_work);
}
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index 90b909e..5e5012f 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -650,6 +650,9 @@ alloc_init_deleg(struct nfs4_client *clp, struct svc_fh *current_fh)
INIT_LIST_HEAD(&dp->dl_perclnt);
INIT_LIST_HEAD(&dp->dl_recall_lru);
dp->dl_type = NFS4_OPEN_DELEGATE_READ;
+ dp->dl_retries = 1;
+ nfsd4_init_cb(&dp->dl_recall, dp->dl_stid.sc_client,
+ NFSPROC4_CLNT_CB_RECALL);
INIT_WORK(&dp->dl_recall.cb_work, nfsd4_run_cb_recall);
return dp;
out_dec:
@@ -1866,6 +1869,7 @@ static struct nfs4_client *create_client(struct xdr_netobj name,
free_client(clp);
return NULL;
}
+ nfsd4_init_cb(&clp->cl_cb_null, clp, NFSPROC4_CLNT_CB_NULL);
INIT_WORK(&clp->cl_cb_null.cb_work, nfsd4_run_cb_null);
clp->cl_time = get_seconds();
clear_bit(0, &clp->cl_cb_slot_busy);
@@ -3383,9 +3387,7 @@ static void nfsd_break_one_deleg(struct nfs4_delegation *dp)
* it's safe to take a reference.
*/
atomic_inc(&dp->dl_stid.sc_count);
- dp->dl_retries = 1;
- nfsd4_cb(&dp->dl_recall, dp->dl_stid.sc_client,
- NFSPROC4_CLNT_CB_RECALL);
+ nfsd4_run_cb(&dp->dl_recall);
}
/* Called from break_lease() with i_lock held. */
diff --git a/fs/nfsd/state.h b/fs/nfsd/state.h
index 515408b..4d10e1d 100644
--- a/fs/nfsd/state.h
+++ b/fs/nfsd/state.h
@@ -541,8 +541,9 @@ void nfsd4_run_cb_recall(struct work_struct *w);
extern void nfsd4_probe_callback(struct nfs4_client *clp);
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_cb(struct nfsd4_callback *cb, struct nfs4_client *clp,
+extern void nfsd4_init_cb(struct nfsd4_callback *cb, struct nfs4_client *clp,
enum nfsd4_cb_op op);
+extern void 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 *);
--
1.9.1
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [PATCH 3/4] nfsd: split nfsd4_callback initialization and use
2014-09-24 10:19 ` [PATCH 3/4] nfsd: split nfsd4_callback initialization and use Christoph Hellwig
@ 2014-09-24 15:29 ` Jeff Layton
2014-09-24 16:43 ` Christoph Hellwig
0 siblings, 1 reply; 9+ messages in thread
From: Jeff Layton @ 2014-09-24 15:29 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: J. Bruce Fields, linux-nfs
On Wed, 24 Sep 2014 12:19:18 +0200
Christoph Hellwig <hch@lst.de> wrote:
> Split out initializing the nfs4_callback structure from using it. For
> the NULL callback this gets rid of tons of pointless re-initializations.
>
> Note that I don't quite understand what protects us from running multiple
> NULL callbacks at the same time, but at least this chance doesn't make
> it worse..
>
Doesn't the workqueue ensure that? Or am I misunderstanding what you're
saying there?
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
> fs/nfsd/nfs4callback.c | 14 ++++++--------
> fs/nfsd/nfs4state.c | 8 +++++---
> fs/nfsd/state.h | 3 ++-
> 3 files changed, 13 insertions(+), 12 deletions(-)
>
> diff --git a/fs/nfsd/nfs4callback.c b/fs/nfsd/nfs4callback.c
> index 022edcf..6d9d947 100644
> --- a/fs/nfsd/nfs4callback.c
> +++ b/fs/nfsd/nfs4callback.c
> @@ -743,11 +743,6 @@ static const struct rpc_call_ops nfsd4_cb_probe_ops = {
>
> static struct workqueue_struct *callback_wq;
>
> -static void do_probe_callback(struct nfs4_client *clp)
> -{
> - return nfsd4_cb(&clp->cl_cb_null, clp, NFSPROC4_CLNT_CB_NULL);
> -}
> -
> /*
> * Poke the callback thread to process any updates to the callback
> * parameters, and send a null probe.
> @@ -756,7 +751,7 @@ void nfsd4_probe_callback(struct nfs4_client *clp)
> {
> clp->cl_cb_state = NFSD4_CB_UNKNOWN;
> set_bit(NFSD4_CLIENT_CB_UPDATE, &clp->cl_flags);
> - do_probe_callback(clp);
> + nfsd4_run_cb(&clp->cl_cb_null);
> }
>
> void nfsd4_probe_callback_sync(struct nfs4_client *clp)
> @@ -912,7 +907,7 @@ void nfsd4_shutdown_callback(struct nfs4_client *clp)
> * instead, nfsd4_run_cb_null() will detect the killed
> * client, destroy the rpc client, and stop:
> */
> - do_probe_callback(clp);
> + nfsd4_run_cb(&clp->cl_cb_null);
> flush_workqueue(callback_wq);
> }
>
> @@ -1025,7 +1020,7 @@ nfsd4_run_cb_recall(struct work_struct *w)
> nfsd4_run_callback_rpc(cb);
> }
>
> -void nfsd4_cb(struct nfsd4_callback *cb, struct nfs4_client *clp,
> +void nfsd4_init_cb(struct nfsd4_callback *cb, struct nfs4_client *clp,
> enum nfsd4_cb_op op)
> {
> cb->cb_clp = clp;
> @@ -1038,6 +1033,9 @@ void nfsd4_cb(struct nfsd4_callback *cb, struct nfs4_client *clp,
> cb->cb_ops = &nfsd4_cb_recall_ops;
> INIT_LIST_HEAD(&cb->cb_per_client);
> cb->cb_done = true;
> +}
>
> +void nfsd4_run_cb(struct nfsd4_callback *cb)
> +{
> queue_work(callback_wq, &cb->cb_work);
> }
> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
> index 90b909e..5e5012f 100644
> --- a/fs/nfsd/nfs4state.c
> +++ b/fs/nfsd/nfs4state.c
> @@ -650,6 +650,9 @@ alloc_init_deleg(struct nfs4_client *clp, struct svc_fh *current_fh)
> INIT_LIST_HEAD(&dp->dl_perclnt);
> INIT_LIST_HEAD(&dp->dl_recall_lru);
> dp->dl_type = NFS4_OPEN_DELEGATE_READ;
> + dp->dl_retries = 1;
> + nfsd4_init_cb(&dp->dl_recall, dp->dl_stid.sc_client,
> + NFSPROC4_CLNT_CB_RECALL);
> INIT_WORK(&dp->dl_recall.cb_work, nfsd4_run_cb_recall);
> return dp;
> out_dec:
> @@ -1866,6 +1869,7 @@ static struct nfs4_client *create_client(struct xdr_netobj name,
> free_client(clp);
> return NULL;
> }
> + nfsd4_init_cb(&clp->cl_cb_null, clp, NFSPROC4_CLNT_CB_NULL);
> INIT_WORK(&clp->cl_cb_null.cb_work, nfsd4_run_cb_null);
> clp->cl_time = get_seconds();
> clear_bit(0, &clp->cl_cb_slot_busy);
> @@ -3383,9 +3387,7 @@ static void nfsd_break_one_deleg(struct nfs4_delegation *dp)
> * it's safe to take a reference.
> */
> atomic_inc(&dp->dl_stid.sc_count);
> - dp->dl_retries = 1;
> - nfsd4_cb(&dp->dl_recall, dp->dl_stid.sc_client,
> - NFSPROC4_CLNT_CB_RECALL);
> + nfsd4_run_cb(&dp->dl_recall);
> }
>
> /* Called from break_lease() with i_lock held. */
> diff --git a/fs/nfsd/state.h b/fs/nfsd/state.h
> index 515408b..4d10e1d 100644
> --- a/fs/nfsd/state.h
> +++ b/fs/nfsd/state.h
> @@ -541,8 +541,9 @@ void nfsd4_run_cb_recall(struct work_struct *w);
> extern void nfsd4_probe_callback(struct nfs4_client *clp);
> 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_cb(struct nfsd4_callback *cb, struct nfs4_client *clp,
> +extern void nfsd4_init_cb(struct nfsd4_callback *cb, struct nfs4_client *clp,
> enum nfsd4_cb_op op);
> +extern void 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 *);
--
Jeff Layton <jlayton@primarydata.com>
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH 3/4] nfsd: split nfsd4_callback initialization and use
2014-09-24 15:29 ` Jeff Layton
@ 2014-09-24 16:43 ` Christoph Hellwig
0 siblings, 0 replies; 9+ messages in thread
From: Christoph Hellwig @ 2014-09-24 16:43 UTC (permalink / raw)
To: Jeff Layton; +Cc: Christoph Hellwig, J. Bruce Fields, linux-nfs
On Wed, Sep 24, 2014 at 11:29:54AM -0400, Jeff Layton wrote:
> > Split out initializing the nfs4_callback structure from using it. For
> > the NULL callback this gets rid of tons of pointless re-initializations.
> >
> > Note that I don't quite understand what protects us from running multiple
> > NULL callbacks at the same time, but at least this chance doesn't make
> > it worse..
> >
>
> Doesn't the workqueue ensure that? Or am I misunderstanding what you're
> saying there?
For the code after my patch that is indeed the case, but before my
patch we'd re-initialize various fields in cl_cb_null everytime
do_probe_callback gets called.
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 4/4] nfsd: introduce nfsd4_callback_ops
2014-09-24 10:19 refactor nfsd callback handling Christoph Hellwig
` (2 preceding siblings ...)
2014-09-24 10:19 ` [PATCH 3/4] nfsd: split nfsd4_callback initialization and use Christoph Hellwig
@ 2014-09-24 10:19 ` Christoph Hellwig
2014-09-24 15:30 ` refactor nfsd callback handling Jeff Layton
2014-09-26 18:03 ` J. Bruce Fields
5 siblings, 0 replies; 9+ messages in thread
From: Christoph Hellwig @ 2014-09-24 10:19 UTC (permalink / raw)
To: J. Bruce Fields; +Cc: linux-nfs
Add a higher level abstraction than the rpc_ops for callback operations.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
fs/nfsd/nfs4callback.c | 91 ++++++++++++++++----------------------------------
fs/nfsd/nfs4state.c | 51 +++++++++++++++++++++++++---
fs/nfsd/state.h | 12 ++++---
3 files changed, 83 insertions(+), 71 deletions(-)
diff --git a/fs/nfsd/nfs4callback.c b/fs/nfsd/nfs4callback.c
index 6d9d947..9cc51b9 100644
--- a/fs/nfsd/nfs4callback.c
+++ b/fs/nfsd/nfs4callback.c
@@ -824,18 +824,8 @@ static void nfsd4_cb_done(struct rpc_task *task, void *calldata)
dprintk("%s: freed slot, new seqid=%d\n", __func__,
clp->cl_cb_session->se_cb_seq_nr);
}
-}
-static void nfsd4_cb_recall_done(struct rpc_task *task, void *calldata)
-{
- struct nfsd4_callback *cb = calldata;
- struct nfs4_delegation *dp = to_delegation(cb);
- struct nfs4_client *clp = cb->cb_clp;
- struct rpc_clnt *current_rpc_client = clp->cl_cb_client;
-
- nfsd4_cb_done(task, calldata);
-
- if (current_rpc_client != task->tk_client) {
+ if (clp->cl_cb_client != task->tk_client) {
/* We're shutting down or changing cl_cb_client; leave
* it to nfsd4_process_cb_update to restart the call if
* necessary. */
@@ -844,45 +834,42 @@ static void nfsd4_cb_recall_done(struct rpc_task *task, void *calldata)
if (cb->cb_done)
return;
- switch (task->tk_status) {
+
+ switch (cb->cb_ops->done(cb, task)) {
case 0:
+ task->tk_status = 0;
+ rpc_restart_call_prepare(task);
+ return;
+ case 1:
break;
- case -EBADHANDLE:
- case -NFS4ERR_BAD_STATEID:
- /* Race: client probably got cb_recall
- * before open reply granting delegation */
- if (dp->dl_retries--) {
- rpc_delay(task, 2*HZ);
- task->tk_status = 0;
- rpc_restart_call_prepare(task);
- return;
- }
- default:
+ case -1:
/* Network partition? */
nfsd4_mark_cb_down(clp, task->tk_status);
+ break;
+ default:
+ BUG();
}
cb->cb_done = true;
}
-static void nfsd4_cb_recall_release(void *calldata)
+static void nfsd4_cb_release(void *calldata)
{
struct nfsd4_callback *cb = calldata;
struct nfs4_client *clp = cb->cb_clp;
if (cb->cb_done) {
- struct nfs4_delegation *dp = to_delegation(cb);
-
spin_lock(&clp->cl_lock);
list_del(&cb->cb_per_client);
spin_unlock(&clp->cl_lock);
- nfs4_put_stid(&dp->dl_stid);
+
+ cb->cb_ops->release(cb);
}
}
-static const struct rpc_call_ops nfsd4_cb_recall_ops = {
+static const struct rpc_call_ops nfsd4_cb_ops = {
.rpc_call_prepare = nfsd4_cb_prepare,
- .rpc_call_done = nfsd4_cb_recall_done,
- .rpc_release = nfsd4_cb_recall_release,
+ .rpc_call_done = nfsd4_cb_done,
+ .rpc_release = nfsd4_cb_release,
};
int nfsd4_create_callback_queue(void)
@@ -911,12 +898,6 @@ void nfsd4_shutdown_callback(struct nfs4_client *clp)
flush_workqueue(callback_wq);
}
-static void nfsd4_release_cb(struct nfsd4_callback *cb)
-{
- if (cb->cb_ops->rpc_release)
- cb->cb_ops->rpc_release(cb);
-}
-
/* requires cl_lock: */
static struct nfsd4_conn * __nfsd4_find_backchannel(struct nfs4_client *clp)
{
@@ -983,54 +964,40 @@ static void nfsd4_process_cb_update(struct nfsd4_callback *cb)
}
static void
-nfsd4_run_callback_rpc(struct nfsd4_callback *cb)
+nfsd4_run_cb_work(struct work_struct *work)
{
+ struct nfsd4_callback *cb =
+ container_of(work, struct nfsd4_callback, cb_work);
struct nfs4_client *clp = cb->cb_clp;
struct rpc_clnt *clnt;
+ if (cb->cb_ops && cb->cb_ops->prepare)
+ cb->cb_ops->prepare(cb);
+
if (clp->cl_flags & NFSD4_CLIENT_CB_FLAG_MASK)
nfsd4_process_cb_update(cb);
clnt = clp->cl_cb_client;
if (!clnt) {
/* Callback channel broken, or client killed; give up: */
- nfsd4_release_cb(cb);
+ if (cb->cb_ops && cb->cb_ops->release)
+ cb->cb_ops->release(cb);
return;
}
cb->cb_msg.rpc_cred = clp->cl_cb_cred;
rpc_call_async(clnt, &cb->cb_msg, RPC_TASK_SOFT | RPC_TASK_SOFTCONN,
- cb->cb_ops, cb);
-}
-
-void
-nfsd4_run_cb_null(struct work_struct *w)
-{
- struct nfsd4_callback *cb = container_of(w, struct nfsd4_callback,
- cb_work);
- nfsd4_run_callback_rpc(cb);
-}
-
-void
-nfsd4_run_cb_recall(struct work_struct *w)
-{
- struct nfsd4_callback *cb = container_of(w, struct nfsd4_callback,
- cb_work);
-
- nfsd4_prepare_cb_recall(to_delegation(cb));
- nfsd4_run_callback_rpc(cb);
+ cb->cb_ops ? &nfsd4_cb_ops : &nfsd4_cb_probe_ops, cb);
}
void nfsd4_init_cb(struct nfsd4_callback *cb, struct nfs4_client *clp,
- enum nfsd4_cb_op op)
+ struct nfsd4_callback_ops *ops, enum nfsd4_cb_op op)
{
cb->cb_clp = clp;
cb->cb_msg.rpc_proc = &nfs4_cb_procedures[op];
cb->cb_msg.rpc_argp = cb;
cb->cb_msg.rpc_resp = cb;
- if (op == NFSPROC4_CLNT_CB_NULL)
- cb->cb_ops = &nfsd4_cb_probe_ops;
- else
- cb->cb_ops = &nfsd4_cb_recall_ops;
+ cb->cb_ops = ops;
+ INIT_WORK(&cb->cb_work, nfsd4_run_cb_work);
INIT_LIST_HEAD(&cb->cb_per_client);
cb->cb_done = true;
}
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index 5e5012f..cf1ccc4 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -96,6 +96,8 @@ static struct kmem_cache *deleg_slab;
static void free_session(struct nfsd4_session *);
+static struct nfsd4_callback_ops nfsd4_cb_recall_ops;
+
static bool is_session_dead(struct nfsd4_session *ses)
{
return ses->se_flags & NFS4_SESSION_DEAD;
@@ -652,8 +654,7 @@ alloc_init_deleg(struct nfs4_client *clp, struct svc_fh *current_fh)
dp->dl_type = NFS4_OPEN_DELEGATE_READ;
dp->dl_retries = 1;
nfsd4_init_cb(&dp->dl_recall, dp->dl_stid.sc_client,
- NFSPROC4_CLNT_CB_RECALL);
- INIT_WORK(&dp->dl_recall.cb_work, nfsd4_run_cb_recall);
+ &nfsd4_cb_recall_ops, NFSPROC4_CLNT_CB_RECALL);
return dp;
out_dec:
atomic_long_dec(&num_delegations);
@@ -1869,8 +1870,7 @@ static struct nfs4_client *create_client(struct xdr_netobj name,
free_client(clp);
return NULL;
}
- nfsd4_init_cb(&clp->cl_cb_null, clp, NFSPROC4_CLNT_CB_NULL);
- INIT_WORK(&clp->cl_cb_null.cb_work, nfsd4_run_cb_null);
+ nfsd4_init_cb(&clp->cl_cb_null, clp, NULL, NFSPROC4_CLNT_CB_NULL);
clp->cl_time = get_seconds();
clear_bit(0, &clp->cl_cb_slot_busy);
copy_verf(clp, verf);
@@ -3355,8 +3355,12 @@ nfs4_share_conflict(struct svc_fh *current_fh, unsigned int deny_type)
return ret;
}
-void nfsd4_prepare_cb_recall(struct nfs4_delegation *dp)
+#define cb_to_delegation(cb) \
+ container_of(cb, struct nfs4_delegation, dl_recall)
+
+static void nfsd4_cb_recall_prepare(struct nfsd4_callback *cb)
{
+ struct nfs4_delegation *dp = cb_to_delegation(cb);
struct nfsd_net *nn = net_generic(dp->dl_stid.sc_client->net,
nfsd_net_id);
@@ -3377,6 +3381,43 @@ void nfsd4_prepare_cb_recall(struct nfs4_delegation *dp)
spin_unlock(&state_lock);
}
+static int nfsd4_cb_recall_done(struct nfsd4_callback *cb,
+ struct rpc_task *task)
+{
+ struct nfs4_delegation *dp = cb_to_delegation(cb);
+
+ switch (task->tk_status) {
+ case 0:
+ return 1;
+ case -EBADHANDLE:
+ case -NFS4ERR_BAD_STATEID:
+ /*
+ * Race: client probably got cb_recall before open reply
+ * granting delegation.
+ */
+ if (dp->dl_retries--) {
+ rpc_delay(task, 2 * HZ);
+ return 0;
+ }
+ /*FALLTHRU*/
+ default:
+ return -1;
+ }
+}
+
+static void nfsd4_cb_recall_release(struct nfsd4_callback *cb)
+{
+ struct nfs4_delegation *dp = cb_to_delegation(cb);
+
+ nfs4_put_stid(&dp->dl_stid);
+}
+
+static struct nfsd4_callback_ops nfsd4_cb_recall_ops = {
+ .prepare = nfsd4_cb_recall_prepare,
+ .done = nfsd4_cb_recall_done,
+ .release = nfsd4_cb_recall_release,
+};
+
static void nfsd_break_one_deleg(struct nfs4_delegation *dp)
{
/*
diff --git a/fs/nfsd/state.h b/fs/nfsd/state.h
index 4d10e1d..f583ce4 100644
--- a/fs/nfsd/state.h
+++ b/fs/nfsd/state.h
@@ -66,11 +66,17 @@ struct nfsd4_callback {
struct list_head cb_per_client;
u32 cb_minorversion;
struct rpc_message cb_msg;
- const struct rpc_call_ops *cb_ops;
+ struct nfsd4_callback_ops *cb_ops;
struct work_struct cb_work;
bool cb_done;
};
+struct nfsd4_callback_ops {
+ void (*prepare)(struct nfsd4_callback *);
+ int (*done)(struct nfsd4_callback *, struct rpc_task *);
+ void (*release)(struct nfsd4_callback *);
+};
+
/*
* A core object that represents a "common" stateid. These are generally
* embedded within the different (more specific) stateid objects and contain
@@ -536,13 +542,11 @@ extern struct nfs4_client_reclaim *nfsd4_find_reclaim_client(const char *recdir,
extern __be32 nfs4_check_open_reclaim(clientid_t *clid,
struct nfsd4_compound_state *cstate, struct nfsd_net *nn);
extern int set_callback_cred(void);
-void nfsd4_run_cb_null(struct work_struct *w);
-void nfsd4_run_cb_recall(struct work_struct *w);
extern void nfsd4_probe_callback(struct nfs4_client *clp);
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,
- enum nfsd4_cb_op op);
+ struct nfsd4_callback_ops *ops, enum nfsd4_cb_op op);
extern void nfsd4_run_cb(struct nfsd4_callback *cb);
extern int nfsd4_create_callback_queue(void);
extern void nfsd4_destroy_callback_queue(void);
--
1.9.1
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: refactor nfsd callback handling
2014-09-24 10:19 refactor nfsd callback handling Christoph Hellwig
` (3 preceding siblings ...)
2014-09-24 10:19 ` [PATCH 4/4] nfsd: introduce nfsd4_callback_ops Christoph Hellwig
@ 2014-09-24 15:30 ` Jeff Layton
2014-09-26 18:03 ` J. Bruce Fields
5 siblings, 0 replies; 9+ messages in thread
From: Jeff Layton @ 2014-09-24 15:30 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: J. Bruce Fields, linux-nfs
On Wed, 24 Sep 2014 12:19:15 +0200
Christoph Hellwig <hch@lst.de> wrote:
> This series reshuffles the callback code to allow for easier implementation
> of more callback operations.
>
Nice cleanup and results in a reduction of code (which is almost always
a good thing). Looks good to me.
Reviewed-by: Jeff Layton <jlayton@primarydata.com>
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: refactor nfsd callback handling
2014-09-24 10:19 refactor nfsd callback handling Christoph Hellwig
` (4 preceding siblings ...)
2014-09-24 15:30 ` refactor nfsd callback handling Jeff Layton
@ 2014-09-26 18:03 ` J. Bruce Fields
5 siblings, 0 replies; 9+ messages in thread
From: J. Bruce Fields @ 2014-09-26 18:03 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: linux-nfs
On Wed, Sep 24, 2014 at 12:19:15PM +0200, Christoph Hellwig wrote:
> This series reshuffles the callback code to allow for easier implementation
> of more callback operations.
They look sensible to me, but I got a few pynfs timeouts in overnight
testing, which I can't reproduce.
Almost positive the bug's elsewhere (pynfs, or something broken with my
test machines), but I'll give it a few more tries and then push it out.
Thanks!
--b.
^ permalink raw reply [flat|nested] 9+ messages in thread