All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v8 0/6] nfsd: overhaul the client name tracking code
@ 2012-03-02 20:42 Jeff Layton
  2012-03-02 20:42 ` [PATCH v8 1/6] nfsd: add nfsd4_client_tracking_ops struct and a way to set it Jeff Layton
                   ` (5 more replies)
  0 siblings, 6 replies; 15+ messages in thread
From: Jeff Layton @ 2012-03-02 20:42 UTC (permalink / raw)
  To: bfields; +Cc: linux-nfs, skinsbursky

This is the eighth iteration of this patchset. The primary motivation
for the respin here is to deal with the changes introduced by Stanislav's
"namespacification" of rpc_pipefs. I think this one should be closer
to what Stanislav suggested for this code.

In particular, I've tried to make the "helpers" be containerized
by net namespace already and simply have the code call those helpers
for init_net, and ignore cases where they get called for other net
namespaces.

An earlier version of this patchset can be viewed here. That set also
contains a more comprehensive description of the rationale for doing
this:

    http://www.spinics.net/lists/linux-nfs/msg26324.html

Jeff Layton (6):
  nfsd: add nfsd4_client_tracking_ops struct and a way to set it
  sunrpc: create nfsd dir in rpc_pipefs
  nfsd: convert nfs4_client->cl_cb_flags to a generic flags field
  nfsd: add a header describing upcall to nfsdcld
  nfsd: add the infrastructure to handle the cld upcall
  nfsd: add notifier to handle mount/unmount of rpc_pipefs sb

 fs/nfsd/nfs4callback.c   |   14 +-
 fs/nfsd/nfs4proc.c       |    3 +-
 fs/nfsd/nfs4recover.c    |  594 ++++++++++++++++++++++++++++++++++++++++++++-
 fs/nfsd/nfs4state.c      |   50 ++---
 fs/nfsd/state.h          |   24 +-
 include/linux/nfsd/cld.h |   56 +++++
 net/sunrpc/rpc_pipe.c    |    5 +
 7 files changed, 683 insertions(+), 63 deletions(-)
 create mode 100644 include/linux/nfsd/cld.h

-- 
1.7.7.6


^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH v8 1/6] nfsd: add nfsd4_client_tracking_ops struct and a way to set it
  2012-03-02 20:42 [PATCH v8 0/6] nfsd: overhaul the client name tracking code Jeff Layton
@ 2012-03-02 20:42 ` Jeff Layton
  2012-03-05  8:34   ` Stanislav Kinsbursky
  2012-03-02 20:42 ` [PATCH v8 2/6] sunrpc: create nfsd dir in rpc_pipefs Jeff Layton
                   ` (4 subsequent siblings)
  5 siblings, 1 reply; 15+ messages in thread
From: Jeff Layton @ 2012-03-02 20:42 UTC (permalink / raw)
  To: bfields; +Cc: linux-nfs, skinsbursky

Abstract out the mechanism that we use to track clients into a set of
client name tracking functions.

This gives us a mechanism to plug in a new set of client tracking
functions without disturbing the callers. It also gives us a way to
decide on what tracking scheme to use at runtime.

For now, this just looks like pointless abstraction, but later we'll
add a new alternate scheme for tracking clients on stable storage.

Signed-off-by: Jeff Layton <jlayton@redhat.com>
---
 fs/nfsd/nfs4recover.c |  121 +++++++++++++++++++++++++++++++++++++++++++++----
 fs/nfsd/nfs4state.c   |   46 +++++++------------
 fs/nfsd/state.h       |   14 +++---
 3 files changed, 136 insertions(+), 45 deletions(-)

diff --git a/fs/nfsd/nfs4recover.c b/fs/nfsd/nfs4recover.c
index 0b3e875..1dda803 100644
--- a/fs/nfsd/nfs4recover.c
+++ b/fs/nfsd/nfs4recover.c
@@ -43,9 +43,20 @@
 
 #define NFSDDBG_FACILITY                NFSDDBG_PROC
 
+/* Declarations */
+struct nfsd4_client_tracking_ops {
+	int (*init)(void);
+	void (*exit)(void);
+	void (*create)(struct nfs4_client *);
+	void (*remove)(struct nfs4_client *);
+	int (*check)(struct nfs4_client *);
+	void (*grace_done)(time_t);
+};
+
 /* Globals */
 static struct file *rec_file;
 static char user_recovery_dirname[PATH_MAX] = "/var/lib/nfs/v4recovery";
+static struct nfsd4_client_tracking_ops *client_tracking_ops;
 
 static int
 nfs4_save_creds(const struct cred **original_creds)
@@ -117,7 +128,8 @@ out_no_tfm:
 	return status;
 }
 
-void nfsd4_create_clid_dir(struct nfs4_client *clp)
+static void
+nfsd4_create_clid_dir(struct nfs4_client *clp)
 {
 	const struct cred *original_cred;
 	char *dname = clp->cl_recdir;
@@ -265,7 +277,7 @@ out_unlock:
 	return status;
 }
 
-void
+static void
 nfsd4_remove_clid_dir(struct nfs4_client *clp)
 {
 	const struct cred *original_cred;
@@ -292,7 +304,6 @@ out:
 	if (status)
 		printk("NFSD: Failed to remove expired client state directory"
 				" %.*s\n", HEXDIR_LEN, clp->cl_recdir);
-	return;
 }
 
 static int
@@ -311,8 +322,8 @@ purge_old(struct dentry *parent, struct dentry *child)
 	return 0;
 }
 
-void
-nfsd4_recdir_purge_old(void) {
+static void
+nfsd4_recdir_purge_old(time_t boot_time __attribute__ ((unused))) {
 	int status;
 
 	if (!rec_file)
@@ -343,7 +354,7 @@ load_recdir(struct dentry *parent, struct dentry *child)
 	return 0;
 }
 
-int
+static int
 nfsd4_recdir_load(void) {
 	int status;
 
@@ -361,8 +372,8 @@ nfsd4_recdir_load(void) {
  * Hold reference to the recovery directory.
  */
 
-void
-nfsd4_init_recdir()
+static int
+nfsd4_init_recdir(void)
 {
 	const struct cred *original_cred;
 	int status;
@@ -377,20 +388,37 @@ nfsd4_init_recdir()
 		printk("NFSD: Unable to change credentials to find recovery"
 		       " directory: error %d\n",
 		       status);
-		return;
+		return status;
 	}
 
 	rec_file = filp_open(user_recovery_dirname, O_RDONLY | O_DIRECTORY, 0);
 	if (IS_ERR(rec_file)) {
 		printk("NFSD: unable to find recovery directory %s\n",
 				user_recovery_dirname);
+		status = PTR_ERR(rec_file);
 		rec_file = NULL;
 	}
 
 	nfs4_reset_creds(original_cred);
+	return status;
 }
 
-void
+static int
+nfsd4_load_reboot_recovery_data(void)
+{
+	int status;
+
+	nfs4_lock_state();
+	status = nfsd4_init_recdir();
+	if (!status)
+		status = nfsd4_recdir_load();
+	nfs4_unlock_state();
+	if (status)
+		printk(KERN_ERR "NFSD: Failure reading reboot recovery data\n");
+	return status;
+}
+
+static void
 nfsd4_shutdown_recdir(void)
 {
 	if (!rec_file)
@@ -425,3 +453,76 @@ nfs4_recoverydir(void)
 {
 	return user_recovery_dirname;
 }
+
+static int
+nfsd4_check_legacy_client(struct nfs4_client *clp)
+{
+	if (nfsd4_find_reclaim_client(clp) != NULL)
+		return 0;
+	else
+		return -ENOENT;
+}
+
+static struct nfsd4_client_tracking_ops nfsd4_legacy_tracking_ops = {
+	.init		= nfsd4_load_reboot_recovery_data,
+	.exit		= nfsd4_shutdown_recdir,
+	.create		= nfsd4_create_clid_dir,
+	.remove		= nfsd4_remove_clid_dir,
+	.check		= nfsd4_check_legacy_client,
+	.grace_done	= nfsd4_recdir_purge_old,
+};
+
+int
+nfsd4_client_tracking_init(void)
+{
+	int status;
+
+	client_tracking_ops = &nfsd4_legacy_tracking_ops;
+
+	status = client_tracking_ops->init();
+	if (status) {
+		printk(KERN_WARNING "NFSD: Unable to initialize client "
+				    "recovery tracking! (%d)\n", status);
+		client_tracking_ops = NULL;
+	}
+	return status;
+}
+
+void
+nfsd4_client_tracking_exit(void)
+{
+	if (client_tracking_ops) {
+		client_tracking_ops->exit();
+		client_tracking_ops = NULL;
+	}
+}
+
+void
+nfsd4_client_record_create(struct nfs4_client *clp)
+{
+	if (client_tracking_ops)
+		client_tracking_ops->create(clp);
+}
+
+void
+nfsd4_client_record_remove(struct nfs4_client *clp)
+{
+	if (client_tracking_ops)
+		client_tracking_ops->remove(clp);
+}
+
+int
+nfsd4_client_record_check(struct nfs4_client *clp)
+{
+	if (client_tracking_ops)
+		return client_tracking_ops->check(clp);
+
+	return -EOPNOTSUPP;
+}
+
+void
+nfsd4_record_grace_done(time_t boot_time)
+{
+	if (client_tracking_ops)
+		client_tracking_ops->grace_done(boot_time);
+}
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index c5cddd6..0c7ac26 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -2045,7 +2045,7 @@ nfsd4_reclaim_complete(struct svc_rqst *rqstp, struct nfsd4_compound_state *csta
 		goto out;
 
 	status = nfs_ok;
-	nfsd4_create_clid_dir(cstate->session->se_client);
+	nfsd4_client_record_create(cstate->session->se_client);
 out:
 	nfs4_unlock_state();
 	return status;
@@ -2240,7 +2240,7 @@ nfsd4_setclientid_confirm(struct svc_rqst *rqstp,
 			conf = find_confirmed_client_by_str(unconf->cl_recdir,
 							    hash);
 			if (conf) {
-				nfsd4_remove_clid_dir(conf);
+				nfsd4_client_record_remove(conf);
 				expire_client(conf);
 			}
 			move_to_confirmed(unconf);
@@ -3066,7 +3066,7 @@ static void
 nfsd4_end_grace(void)
 {
 	dprintk("NFSD: end of grace period\n");
-	nfsd4_recdir_purge_old();
+	nfsd4_record_grace_done(boot_time);
 	locks_end_grace(&nfsd4_manager);
 	/*
 	 * Now that every NFSv4 client has had the chance to recover and
@@ -3115,7 +3115,7 @@ nfs4_laundromat(void)
 		clp = list_entry(pos, struct nfs4_client, cl_lru);
 		dprintk("NFSD: purging unused client (clientid %08x)\n",
 			clp->cl_clientid.cl_id);
-		nfsd4_remove_clid_dir(clp);
+		nfsd4_client_record_remove(clp);
 		expire_client(clp);
 	}
 	spin_lock(&recall_lock);
@@ -3539,7 +3539,7 @@ nfsd4_open_confirm(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
 	dprintk("NFSD: %s: success, seqid=%d stateid=" STATEID_FMT "\n",
 		__func__, oc->oc_seqid, STATEID_VAL(&stp->st_stid.sc_stateid));
 
-	nfsd4_create_clid_dir(oo->oo_owner.so_client);
+	nfsd4_client_record_create(oo->oo_owner.so_client);
 	status = nfs_ok;
 out:
 	if (!cstate->replay_owner)
@@ -4397,19 +4397,13 @@ nfs4_release_reclaim(void)
 
 /*
  * called from OPEN, CLAIM_PREVIOUS with a new clientid. */
-static struct nfs4_client_reclaim *
-nfs4_find_reclaim_client(clientid_t *clid)
+struct nfs4_client_reclaim *
+nfsd4_find_reclaim_client(struct nfs4_client *clp)
 {
 	unsigned int strhashval;
-	struct nfs4_client *clp;
 	struct nfs4_client_reclaim *crp = NULL;
 
 
-	/* find clientid in conf_id_hashtbl */
-	clp = find_confirmed_client(clid);
-	if (clp == NULL)
-		return NULL;
-
 	dprintk("NFSD: nfs4_find_reclaim_client for %.*s with recdir %s\n",
 		            clp->cl_name.len, clp->cl_name.data,
 			    clp->cl_recdir);
@@ -4430,7 +4424,14 @@ nfs4_find_reclaim_client(clientid_t *clid)
 __be32
 nfs4_check_open_reclaim(clientid_t *clid)
 {
-	return nfs4_find_reclaim_client(clid) ? nfs_ok : nfserr_reclaim_bad;
+	struct nfs4_client *clp;
+
+	/* find clientid in conf_id_hashtbl */
+	clp = find_confirmed_client(clid);
+	if (clp == NULL)
+		return nfserr_reclaim_bad;
+
+	return nfsd4_client_record_check(clp) ? nfserr_reclaim_bad : nfs_ok;
 }
 
 #ifdef CONFIG_NFSD_FAULT_INJECTION
@@ -4577,19 +4578,6 @@ nfs4_state_init(void)
 	reclaim_str_hashtbl_size = 0;
 }
 
-static void
-nfsd4_load_reboot_recovery_data(void)
-{
-	int status;
-
-	nfs4_lock_state();
-	nfsd4_init_recdir();
-	status = nfsd4_recdir_load();
-	nfs4_unlock_state();
-	if (status)
-		printk("NFSD: Failure reading reboot recovery data\n");
-}
-
 /*
  * Since the lifetime of a delegation isn't limited to that of an open, a
  * client may quite reasonably hang on to a delegation as long as it has
@@ -4642,7 +4630,7 @@ out_free_laundry:
 int
 nfs4_state_start(void)
 {
-	nfsd4_load_reboot_recovery_data();
+	nfsd4_client_tracking_init();
 	return __nfs4_state_start();
 }
 
@@ -4676,7 +4664,7 @@ __nfs4_state_shutdown(void)
 		unhash_delegation(dp);
 	}
 
-	nfsd4_shutdown_recdir();
+	nfsd4_client_tracking_exit();
 }
 
 void
diff --git a/fs/nfsd/state.h b/fs/nfsd/state.h
index ffb5df1..e22f90f 100644
--- a/fs/nfsd/state.h
+++ b/fs/nfsd/state.h
@@ -463,6 +463,7 @@ extern __be32 nfs4_preprocess_stateid_op(struct nfsd4_compound_state *cstate,
 extern void nfs4_lock_state(void);
 extern void nfs4_unlock_state(void);
 extern int nfs4_in_grace(void);
+extern struct nfs4_client_reclaim *nfsd4_find_reclaim_client(struct nfs4_client *crp);
 extern __be32 nfs4_check_open_reclaim(clientid_t *clid);
 extern void nfs4_free_openowner(struct nfs4_openowner *);
 extern void nfs4_free_lockowner(struct nfs4_lockowner *);
@@ -477,16 +478,17 @@ extern void nfsd4_destroy_callback_queue(void);
 extern void nfsd4_shutdown_callback(struct nfs4_client *);
 extern void nfs4_put_delegation(struct nfs4_delegation *dp);
 extern __be32 nfs4_make_rec_clidname(char *clidname, struct xdr_netobj *clname);
-extern void nfsd4_init_recdir(void);
-extern int nfsd4_recdir_load(void);
-extern void nfsd4_shutdown_recdir(void);
 extern int nfs4_client_to_reclaim(const char *name);
 extern int nfs4_has_reclaimed_state(const char *name, bool use_exchange_id);
-extern void nfsd4_recdir_purge_old(void);
-extern void nfsd4_create_clid_dir(struct nfs4_client *clp);
-extern void nfsd4_remove_clid_dir(struct nfs4_client *clp);
 extern void release_session_client(struct nfsd4_session *);
 extern __be32 nfs4_validate_stateid(struct nfs4_client *, stateid_t *);
 extern void nfsd4_purge_closed_stateid(struct nfs4_stateowner *);
 
+/* nfs4recover operations */
+extern int nfsd4_client_tracking_init(void);
+extern void nfsd4_client_tracking_exit(void);
+extern void nfsd4_client_record_create(struct nfs4_client *clp);
+extern void nfsd4_client_record_remove(struct nfs4_client *clp);
+extern int nfsd4_client_record_check(struct nfs4_client *clp);
+extern void nfsd4_record_grace_done(time_t boot_time);
 #endif   /* NFSD4_STATE_H */
-- 
1.7.7.6


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH v8 2/6] sunrpc: create nfsd dir in rpc_pipefs
  2012-03-02 20:42 [PATCH v8 0/6] nfsd: overhaul the client name tracking code Jeff Layton
  2012-03-02 20:42 ` [PATCH v8 1/6] nfsd: add nfsd4_client_tracking_ops struct and a way to set it Jeff Layton
@ 2012-03-02 20:42 ` Jeff Layton
  2012-03-02 20:42 ` [PATCH v8 3/6] nfsd: convert nfs4_client->cl_cb_flags to a generic flags field Jeff Layton
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 15+ messages in thread
From: Jeff Layton @ 2012-03-02 20:42 UTC (permalink / raw)
  To: bfields; +Cc: linux-nfs, skinsbursky

Add a new top-level dir in rpc_pipefs to hold the pipe for the clientid
upcall.

Signed-off-by: Jeff Layton <jlayton@redhat.com>
---
 net/sunrpc/rpc_pipe.c |    5 +++++
 1 files changed, 5 insertions(+), 0 deletions(-)

diff --git a/net/sunrpc/rpc_pipe.c b/net/sunrpc/rpc_pipe.c
index 6873c9b..c81915d 100644
--- a/net/sunrpc/rpc_pipe.c
+++ b/net/sunrpc/rpc_pipe.c
@@ -992,6 +992,7 @@ enum {
 	RPCAUTH_statd,
 	RPCAUTH_nfsd4_cb,
 	RPCAUTH_cache,
+	RPCAUTH_nfsd,
 	RPCAUTH_RootEOF
 };
 
@@ -1024,6 +1025,10 @@ static const struct rpc_filelist files[] = {
 		.name = "cache",
 		.mode = S_IFDIR | S_IRUGO | S_IXUGO,
 	},
+	[RPCAUTH_nfsd] = {
+		.name = "nfsd",
+		.mode = S_IFDIR | S_IRUGO | S_IXUGO,
+	},
 };
 
 /*
-- 
1.7.7.6


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH v8 3/6] nfsd: convert nfs4_client->cl_cb_flags to a generic flags field
  2012-03-02 20:42 [PATCH v8 0/6] nfsd: overhaul the client name tracking code Jeff Layton
  2012-03-02 20:42 ` [PATCH v8 1/6] nfsd: add nfsd4_client_tracking_ops struct and a way to set it Jeff Layton
  2012-03-02 20:42 ` [PATCH v8 2/6] sunrpc: create nfsd dir in rpc_pipefs Jeff Layton
@ 2012-03-02 20:42 ` Jeff Layton
  2012-03-02 20:42 ` [PATCH v8 4/6] nfsd: add a header describing upcall to nfsdcld Jeff Layton
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 15+ messages in thread
From: Jeff Layton @ 2012-03-02 20:42 UTC (permalink / raw)
  To: bfields; +Cc: linux-nfs, skinsbursky

We'll need a way to flag the nfs4_client as already being recorded on
stable storage so that we don't continually upcall. Currently, that's
recorded in the cl_firststate field of the client struct. Using an
entire u32 to store a flag is rather wasteful though.

The cl_cb_flags field is only using 2 bits right now, so repurpose that
to a generic flags field. Rename NFSD4_CLIENT_KILL to
NFSD4_CLIENT_CB_KILL to make it evident that it's part of the callback
flags. Add a mask that we can use for existing checks that look to see
whether any flags are set, so that the new flags don't interfere.

Convert all references to cl_firstate to the NFSD4_CLIENT_STABLE flag,
and add a new NFSD4_CLIENT_RECLAIM_COMPLETE flag. I believe there's an
existing bug here too that this should fix:

nfs4_set_claim_prev sets cl_firststate on the first CLAIM_PREV open.
nfsd4_reclaim_complete looks for that flag though, and returns
NFS4ERR_COMPLETE_ALREADY if it's set. The upshot here is that once a
client does a CLAIM_PREV open, the RECLAIM_COMPLETE call will fail.
Let's fix this by adding a new RECLAIM_COMPLETE flag on the client to
indicate that that's already been done.

Signed-off-by: Jeff Layton <jlayton@redhat.com>
---
 fs/nfsd/nfs4callback.c |   14 +++++++-------
 fs/nfsd/nfs4proc.c     |    3 ++-
 fs/nfsd/nfs4recover.c  |   20 +++++++++++++-------
 fs/nfsd/nfs4state.c    |    4 ++--
 fs/nfsd/state.h        |   10 ++++++----
 5 files changed, 30 insertions(+), 21 deletions(-)

diff --git a/fs/nfsd/nfs4callback.c b/fs/nfsd/nfs4callback.c
index 0e262f3..a09dcc4 100644
--- a/fs/nfsd/nfs4callback.c
+++ b/fs/nfsd/nfs4callback.c
@@ -754,9 +754,9 @@ static void do_probe_callback(struct nfs4_client *clp)
  */
 void nfsd4_probe_callback(struct nfs4_client *clp)
 {
-	/* XXX: atomicity?  Also, should we be using cl_cb_flags? */
+	/* XXX: atomicity?  Also, should we be using cl_flags? */
 	clp->cl_cb_state = NFSD4_CB_UNKNOWN;
-	set_bit(NFSD4_CLIENT_CB_UPDATE, &clp->cl_cb_flags);
+	set_bit(NFSD4_CLIENT_CB_UPDATE, &clp->cl_flags);
 	do_probe_callback(clp);
 }
 
@@ -915,7 +915,7 @@ void nfsd4_destroy_callback_queue(void)
 /* must be called under the state lock */
 void nfsd4_shutdown_callback(struct nfs4_client *clp)
 {
-	set_bit(NFSD4_CLIENT_KILL, &clp->cl_cb_flags);
+	set_bit(NFSD4_CLIENT_CB_KILL, &clp->cl_flags);
 	/*
 	 * Note this won't actually result in a null callback;
 	 * instead, nfsd4_do_callback_rpc() will detect the killed
@@ -966,15 +966,15 @@ static void nfsd4_process_cb_update(struct nfsd4_callback *cb)
 		svc_xprt_put(clp->cl_cb_conn.cb_xprt);
 		clp->cl_cb_conn.cb_xprt = NULL;
 	}
-	if (test_bit(NFSD4_CLIENT_KILL, &clp->cl_cb_flags))
+	if (test_bit(NFSD4_CLIENT_CB_KILL, &clp->cl_flags))
 		return;
 	spin_lock(&clp->cl_lock);
 	/*
 	 * Only serialized callback code is allowed to clear these
 	 * flags; main nfsd code can only set them:
 	 */
-	BUG_ON(!clp->cl_cb_flags);
-	clear_bit(NFSD4_CLIENT_CB_UPDATE, &clp->cl_cb_flags);
+	BUG_ON(!(clp->cl_flags & NFSD4_CLIENT_CB_FLAG_MASK));
+	clear_bit(NFSD4_CLIENT_CB_UPDATE, &clp->cl_flags);
 	memcpy(&conn, &cb->cb_clp->cl_cb_conn, sizeof(struct nfs4_cb_conn));
 	c = __nfsd4_find_backchannel(clp);
 	if (c) {
@@ -1000,7 +1000,7 @@ void nfsd4_do_callback_rpc(struct work_struct *w)
 	struct nfs4_client *clp = cb->cb_clp;
 	struct rpc_clnt *clnt;
 
-	if (clp->cl_cb_flags)
+	if (clp->cl_flags & NFSD4_CLIENT_CB_FLAG_MASK)
 		nfsd4_process_cb_update(cb);
 
 	clnt = clp->cl_cb_client;
diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
index 896da74..af2c3e8 100644
--- a/fs/nfsd/nfs4proc.c
+++ b/fs/nfsd/nfs4proc.c
@@ -319,7 +319,8 @@ nfsd4_open(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
 	 * Before RECLAIM_COMPLETE done, server should deny new lock
 	 */
 	if (nfsd4_has_session(cstate) &&
-	    !cstate->session->se_client->cl_firststate &&
+	    !test_bit(NFSD4_CLIENT_RECLAIM_COMPLETE,
+		      &cstate->session->se_client->cl_flags) &&
 	    open->op_claim_type != NFS4_OPEN_CLAIM_PREVIOUS)
 		return nfserr_grace;
 
diff --git a/fs/nfsd/nfs4recover.c b/fs/nfsd/nfs4recover.c
index 1dda803..ccd9b97 100644
--- a/fs/nfsd/nfs4recover.c
+++ b/fs/nfsd/nfs4recover.c
@@ -138,9 +138,8 @@ nfsd4_create_clid_dir(struct nfs4_client *clp)
 
 	dprintk("NFSD: nfsd4_create_clid_dir for \"%s\"\n", dname);
 
-	if (clp->cl_firststate)
+	if (test_and_set_bit(NFSD4_CLIENT_STABLE, &clp->cl_flags))
 		return;
-	clp->cl_firststate = 1;
 	if (!rec_file)
 		return;
 	status = nfs4_save_creds(&original_cred);
@@ -283,13 +282,13 @@ nfsd4_remove_clid_dir(struct nfs4_client *clp)
 	const struct cred *original_cred;
 	int status;
 
-	if (!rec_file || !clp->cl_firststate)
+	if (!rec_file || !test_bit(NFSD4_CLIENT_STABLE, &clp->cl_flags))
 		return;
 
 	status = mnt_want_write_file(rec_file);
 	if (status)
 		goto out;
-	clp->cl_firststate = 0;
+	clear_bit(NFSD4_CLIENT_STABLE, &clp->cl_flags);
 
 	status = nfs4_save_creds(&original_cred);
 	if (status < 0)
@@ -457,10 +456,17 @@ nfs4_recoverydir(void)
 static int
 nfsd4_check_legacy_client(struct nfs4_client *clp)
 {
-	if (nfsd4_find_reclaim_client(clp) != NULL)
+	/* did we already find that this client is stable? */
+	if (test_bit(NFSD4_CLIENT_STABLE, &clp->cl_flags))
 		return 0;
-	else
-		return -ENOENT;
+
+	/* look for it in the reclaim hashtable otherwise */
+	if (nfsd4_find_reclaim_client(clp)) {
+		set_bit(NFSD4_CLIENT_STABLE, &clp->cl_flags);
+		return 0;
+	}
+
+	return -ENOENT;
 }
 
 static struct nfsd4_client_tracking_ops nfsd4_legacy_tracking_ops = {
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index 0c7ac26..e831ac6 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -2030,7 +2030,8 @@ nfsd4_reclaim_complete(struct svc_rqst *rqstp, struct nfsd4_compound_state *csta
 
 	nfs4_lock_state();
 	status = nfserr_complete_already;
-	if (cstate->session->se_client->cl_firststate)
+	if (test_and_set_bit(NFSD4_CLIENT_RECLAIM_COMPLETE,
+			     &cstate->session->se_client->cl_flags))
 		goto out;
 
 	status = nfserr_stale_clientid;
@@ -2779,7 +2780,6 @@ static void
 nfs4_set_claim_prev(struct nfsd4_open *open)
 {
 	open->op_openowner->oo_flags |= NFS4_OO_CONFIRMED;
-	open->op_openowner->oo_owner.so_client->cl_firststate = 1;
 }
 
 /* Should we give out recallable state?: */
diff --git a/fs/nfsd/state.h b/fs/nfsd/state.h
index e22f90f..fac413b 100644
--- a/fs/nfsd/state.h
+++ b/fs/nfsd/state.h
@@ -245,14 +245,16 @@ struct nfs4_client {
 	struct svc_cred		cl_cred; 	/* setclientid principal */
 	clientid_t		cl_clientid;	/* generated by server */
 	nfs4_verifier		cl_confirm;	/* generated by server */
-	u32			cl_firststate;	/* recovery dir creation */
 	u32			cl_minorversion;
 
 	/* for v4.0 and v4.1 callbacks: */
 	struct nfs4_cb_conn	cl_cb_conn;
-#define NFSD4_CLIENT_CB_UPDATE	1
-#define NFSD4_CLIENT_KILL	2
-	unsigned long		cl_cb_flags;
+#define NFSD4_CLIENT_CB_UPDATE		(0)
+#define NFSD4_CLIENT_CB_KILL		(1)
+#define NFSD4_CLIENT_STABLE		(2)	/* client on stable storage */
+#define NFSD4_CLIENT_RECLAIM_COMPLETE	(3)	/* reclaim_complete done */
+#define NFSD4_CLIENT_CB_FLAG_MASK	(0x3)
+	unsigned long		cl_flags;
 	struct rpc_clnt		*cl_cb_client;
 	u32			cl_cb_ident;
 #define NFSD4_CB_UP		0
-- 
1.7.7.6


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH v8 4/6] nfsd: add a header describing upcall to nfsdcld
  2012-03-02 20:42 [PATCH v8 0/6] nfsd: overhaul the client name tracking code Jeff Layton
                   ` (2 preceding siblings ...)
  2012-03-02 20:42 ` [PATCH v8 3/6] nfsd: convert nfs4_client->cl_cb_flags to a generic flags field Jeff Layton
@ 2012-03-02 20:42 ` Jeff Layton
  2012-03-02 20:42 ` [PATCH v8 5/6] nfsd: add the infrastructure to handle the cld upcall Jeff Layton
  2012-03-02 20:42 ` [PATCH v8 6/6] nfsd: add notifier to handle mount/unmount of rpc_pipefs sb Jeff Layton
  5 siblings, 0 replies; 15+ messages in thread
From: Jeff Layton @ 2012-03-02 20:42 UTC (permalink / raw)
  To: bfields; +Cc: linux-nfs, skinsbursky

The daemon takes a versioned binary struct. Hopefully this should allow
us to revise the struct later if it becomes necessary.

Signed-off-by: Jeff Layton <jlayton@redhat.com>
---
 include/linux/nfsd/cld.h |   56 ++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 56 insertions(+), 0 deletions(-)
 create mode 100644 include/linux/nfsd/cld.h

diff --git a/include/linux/nfsd/cld.h b/include/linux/nfsd/cld.h
new file mode 100644
index 0000000..f14a9ab
--- /dev/null
+++ b/include/linux/nfsd/cld.h
@@ -0,0 +1,56 @@
+/*
+ * Upcall description for nfsdcld communication
+ *
+ * Copyright (c) 2012 Red Hat, Inc.
+ * Author(s): Jeff Layton <jlayton@redhat.com>
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License as published by
+ *  the Free Software Foundation; either version 2 of the License, or
+ *  (at your option) any later version.
+ *
+ *  This program is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with this program; if not, write to the Free Software
+ *  Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
+ */
+
+#ifndef _NFSD_CLD_H
+#define _NFSD_CLD_H
+
+/* latest upcall version available */
+#define CLD_UPCALL_VERSION 1
+
+/* defined by RFC3530 */
+#define NFS4_OPAQUE_LIMIT 1024
+
+enum cld_command {
+	Cld_Create,		/* create a record for this cm_id */
+	Cld_Remove,		/* remove record of this cm_id */
+	Cld_Check,		/* is this cm_id allowed? */
+	Cld_GraceDone,		/* grace period is complete */
+};
+
+/* representation of long-form NFSv4 client ID */
+struct cld_name {
+	uint16_t	cn_len;				/* length of cm_id */
+	unsigned char	cn_id[NFS4_OPAQUE_LIMIT];	/* client-provided */
+} __attribute__((packed));
+
+/* message struct for communication with userspace */
+struct cld_msg {
+	uint8_t		cm_vers;		/* upcall version */
+	uint8_t		cm_cmd;			/* upcall command */
+	int16_t		cm_status;		/* return code */
+	uint32_t	cm_xid;			/* transaction id */
+	union {
+		int64_t		cm_gracetime;	/* grace period start time */
+		struct cld_name	cm_name;
+	} __attribute__((packed)) cm_u;
+} __attribute__((packed));
+
+#endif /* !_NFSD_CLD_H */
-- 
1.7.7.6


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH v8 5/6] nfsd: add the infrastructure to handle the cld upcall
  2012-03-02 20:42 [PATCH v8 0/6] nfsd: overhaul the client name tracking code Jeff Layton
                   ` (3 preceding siblings ...)
  2012-03-02 20:42 ` [PATCH v8 4/6] nfsd: add a header describing upcall to nfsdcld Jeff Layton
@ 2012-03-02 20:42 ` Jeff Layton
  2012-03-05  8:48   ` Stanislav Kinsbursky
  2012-03-02 20:42 ` [PATCH v8 6/6] nfsd: add notifier to handle mount/unmount of rpc_pipefs sb Jeff Layton
  5 siblings, 1 reply; 15+ messages in thread
From: Jeff Layton @ 2012-03-02 20:42 UTC (permalink / raw)
  To: bfields; +Cc: linux-nfs, skinsbursky

...and add a mechanism for switching between the "legacy" tracker and
the new one. The decision is made by looking to see whether the
v4recoverydir exists. If it does, then the legacy client tracker is
used.

If it's not, then the kernel will create a "cld" pipe in rpc_pipefs.
That pipe is used to talk to a daemon for handling the upcall.

Signed-off-by: Jeff Layton <jlayton@redhat.com>
---
 fs/nfsd/nfs4recover.c |  411 ++++++++++++++++++++++++++++++++++++++++++++++++-
 1 files changed, 410 insertions(+), 1 deletions(-)

diff --git a/fs/nfsd/nfs4recover.c b/fs/nfsd/nfs4recover.c
index ccd9b97..d928c58 100644
--- a/fs/nfsd/nfs4recover.c
+++ b/fs/nfsd/nfs4recover.c
@@ -1,5 +1,6 @@
 /*
 *  Copyright (c) 2004 The Regents of the University of Michigan.
+*  Copyright (c) 2012 Jeff Layton <jlayton@redhat.com>
 *  All rights reserved.
 *
 *  Andy Adamson <andros@citi.umich.edu>
@@ -36,6 +37,11 @@
 #include <linux/namei.h>
 #include <linux/crypto.h>
 #include <linux/sched.h>
+#include <linux/fs.h>
+#include <net/net_namespace.h>
+#include <linux/sunrpc/rpc_pipe_fs.h>
+#include <linux/sunrpc/clnt.h>
+#include <linux/nfsd/cld.h>
 
 #include "nfsd.h"
 #include "state.h"
@@ -478,12 +484,415 @@ static struct nfsd4_client_tracking_ops nfsd4_legacy_tracking_ops = {
 	.grace_done	= nfsd4_recdir_purge_old,
 };
 
+/* Globals */
+#define NFSD_PIPE_DIR		"nfsd"
+#define NFSD_CLD_PIPE		"cld"
+
+static struct rpc_pipe *cld_pipe;
+
+/* list of cld_msg's that are currently in use */
+static DEFINE_SPINLOCK(cld_lock);
+static LIST_HEAD(cld_list);
+static unsigned int cld_xid;
+
+struct cld_upcall {
+	struct list_head	cu_list;
+	struct task_struct	*cu_task;
+	struct cld_msg	cu_msg;
+};
+
+static int
+__cld_pipe_upcall(struct cld_msg *cmsg)
+{
+	int ret;
+	struct rpc_pipe_msg msg;
+
+	memset(&msg, 0, sizeof(msg));
+	msg.data = cmsg;
+	msg.len = sizeof(*cmsg);
+
+	/*
+	 * Set task state before we queue the upcall. That prevents
+	 * wake_up_process in the downcall from racing with schedule.
+	 */
+	set_current_state(TASK_UNINTERRUPTIBLE);
+	ret = rpc_queue_upcall(cld_pipe, &msg);
+	if (ret < 0) {
+		set_current_state(TASK_RUNNING);
+		goto out;
+	}
+
+	schedule();
+	set_current_state(TASK_RUNNING);
+
+	if (msg.errno < 0)
+		ret = msg.errno;
+out:
+	return ret;
+}
+
+static int
+cld_pipe_upcall(struct cld_msg *cmsg)
+{
+	int ret;
+
+	/*
+	 * -EAGAIN occurs when pipe is closed and reopened while there are
+	 *  upcalls queued.
+	 */
+	do {
+		ret = __cld_pipe_upcall(cmsg);
+	} while (ret == -EAGAIN);
+
+	return ret;
+}
+
+static ssize_t
+cld_pipe_downcall(struct file *filp, const char __user *src, size_t mlen)
+{
+	struct cld_upcall *tmp, *cup;
+	struct cld_msg *cmsg = (struct cld_msg *)src;
+	uint32_t xid;
+
+	if (mlen != sizeof(*cmsg)) {
+		dprintk("%s: got %lu bytes, expected %lu\n", __func__, mlen,
+			sizeof(*cmsg));
+		return -EINVAL;
+	}
+
+	/* copy just the xid so we can try to find that */
+	if (copy_from_user(&xid, &cmsg->cm_xid, sizeof(xid)) != 0) {
+		dprintk("%s: error when copying xid from userspace", __func__);
+		return -EFAULT;
+	}
+
+	/* walk the list and find corresponding xid */
+	cup = NULL;
+	spin_lock(&cld_lock);
+	list_for_each_entry(tmp, &cld_list, cu_list) {
+		if (get_unaligned(&tmp->cu_msg.cm_xid) == xid) {
+			cup = tmp;
+			list_del_init(&cup->cu_list);
+			break;
+		}
+	}
+	spin_unlock(&cld_lock);
+
+	/* couldn't find upcall? */
+	if (!cup) {
+		dprintk("%s: couldn't find upcall -- xid=%u\n", __func__,
+			cup->cu_msg.cm_xid);
+		return -EINVAL;
+	}
+
+	if (copy_from_user(&cup->cu_msg, src, mlen) != 0)
+		return -EFAULT;
+
+	wake_up_process(cup->cu_task);
+	return mlen;
+}
+
+static void
+cld_pipe_destroy_msg(struct rpc_pipe_msg *msg)
+{
+	struct cld_msg *cmsg = msg->data;
+	struct cld_upcall *cup = container_of(cmsg, struct cld_upcall,
+						 cu_msg);
+
+	/* errno >= 0 means we got a downcall */
+	if (msg->errno >= 0)
+		return;
+
+	wake_up_process(cup->cu_task);
+}
+
+static const struct rpc_pipe_ops cld_upcall_ops = {
+	.upcall		= rpc_pipe_generic_upcall,
+	.downcall	= cld_pipe_downcall,
+	.destroy_msg	= cld_pipe_destroy_msg,
+};
+
+static struct dentry *
+nfsd4_cld_register_sb(struct super_block *sb, struct rpc_pipe *pipe)
+{
+	struct dentry *dir, *dentry;
+
+	dir = rpc_d_lookup_sb(sb, NFSD_PIPE_DIR);
+	if (dir == NULL)
+		return ERR_PTR(-ENOENT);
+	dentry = rpc_mkpipe_dentry(dir, NFSD_CLD_PIPE, NULL, pipe);
+	dput(dir);
+	return dentry;
+}
+
+static void
+nfsd4_cld_unregister_sb(struct rpc_pipe *pipe)
+{
+	if (pipe->dentry)
+		rpc_unlink(pipe->dentry);
+}
+
+static struct dentry *
+nfsd4_cld_register_net(struct net *net, struct rpc_pipe *pipe)
+{
+	struct super_block *sb;
+	struct dentry *dentry;
+
+	sb = rpc_get_sb_net(net);
+	if (!sb)
+		return NULL;
+	dentry = nfsd4_cld_register_sb(sb, pipe);
+	rpc_put_sb_net(net);
+	return dentry;
+}
+
+static void
+nfsd4_cld_unregister_net(struct net *net, struct rpc_pipe *pipe)
+{
+	struct super_block *sb;
+
+	sb = rpc_get_sb_net(net);
+	if (sb) {
+		nfsd4_cld_unregister_sb(pipe);
+		rpc_put_sb_net(net);
+	}
+}
+
+/* Initialize rpc_pipefs pipe for communication with client tracking daemon */
+static int
+nfsd4_init_cld_pipe(void)
+{
+	int ret;
+	struct dentry *dentry;
+
+	/* FIXME: containerize this code */
+	if (cld_pipe)
+		return 0;
+
+	cld_pipe = rpc_mkpipe_data(&cld_upcall_ops, RPC_PIPE_WAIT_FOR_OPEN);
+	if (IS_ERR(cld_pipe)) {
+		ret = PTR_ERR(cld_pipe);
+		goto err;
+	}
+
+	dentry = nfsd4_cld_register_net(&init_net, cld_pipe);
+	if (IS_ERR(dentry)) {
+		ret = PTR_ERR(dentry);
+		goto err_destroy_data;
+	}
+
+	cld_pipe->dentry = dentry;
+	return 0;
+
+err_destroy_data:
+	rpc_destroy_pipe_data(cld_pipe);
+err:
+	cld_pipe = NULL;
+	printk(KERN_ERR "NFSD: unable to create nfsdcld upcall pipe (%d)\n",
+			ret);
+	return ret;
+}
+
+static void
+nfsd4_remove_cld_pipe(void)
+{
+	/* FIXME: containerize... */
+	nfsd4_cld_unregister_net(&init_net, cld_pipe);
+	rpc_destroy_pipe_data(cld_pipe);
+	cld_pipe = NULL;
+}
+
+static struct cld_upcall *
+alloc_cld_upcall(void)
+{
+	struct cld_upcall *new, *tmp;
+
+	new = kzalloc(sizeof(*new), GFP_KERNEL);
+	if (!new)
+		return new;
+
+	/* FIXME: hard cap on number in flight? */
+restart_search:
+	spin_lock(&cld_lock);
+	list_for_each_entry(tmp, &cld_list, cu_list) {
+		if (tmp->cu_msg.cm_xid == cld_xid) {
+			cld_xid++;
+			spin_unlock(&cld_lock);
+			goto restart_search;
+		}
+	}
+	new->cu_task = current;
+	new->cu_msg.cm_vers = CLD_UPCALL_VERSION;
+	put_unaligned(cld_xid++, &new->cu_msg.cm_xid);
+	list_add(&new->cu_list, &cld_list);
+	spin_unlock(&cld_lock);
+
+	dprintk("%s: allocated xid %u\n", __func__, new->cu_msg.cm_xid);
+
+	return new;
+}
+
+static void
+free_cld_upcall(struct cld_upcall *victim)
+{
+	spin_lock(&cld_lock);
+	list_del(&victim->cu_list);
+	spin_unlock(&cld_lock);
+	kfree(victim);
+}
+
+/* Ask daemon to create a new record */
+static void
+nfsd4_cld_create(struct nfs4_client *clp)
+{
+	int ret;
+	struct cld_upcall *cup;
+
+	/* Don't upcall if it's already stored */
+	if (test_bit(NFSD4_CLIENT_STABLE, &clp->cl_flags))
+		return;
+
+	cup = alloc_cld_upcall();
+	if (!cup) {
+		ret = -ENOMEM;
+		goto out_err;
+	}
+
+	cup->cu_msg.cm_cmd = Cld_Create;
+	cup->cu_msg.cm_u.cm_name.cn_len = clp->cl_name.len;
+	memcpy(cup->cu_msg.cm_u.cm_name.cn_id, clp->cl_name.data,
+			clp->cl_name.len);
+
+	ret = cld_pipe_upcall(&cup->cu_msg);
+	if (!ret) {
+		ret = cup->cu_msg.cm_status;
+		set_bit(NFSD4_CLIENT_STABLE, &clp->cl_flags);
+	}
+
+	free_cld_upcall(cup);
+out_err:
+	if (ret)
+		printk(KERN_ERR "NFSD: Unable to create client "
+				"record on stable storage: %d\n", ret);
+}
+
+/* Ask daemon to create a new record */
+static void
+nfsd4_cld_remove(struct nfs4_client *clp)
+{
+	int ret;
+	struct cld_upcall *cup;
+
+	/* Don't upcall if it's already removed */
+	if (!test_bit(NFSD4_CLIENT_STABLE, &clp->cl_flags))
+		return;
+
+	cup = alloc_cld_upcall();
+	if (!cup) {
+		ret = -ENOMEM;
+		goto out_err;
+	}
+
+	cup->cu_msg.cm_cmd = Cld_Remove;
+	cup->cu_msg.cm_u.cm_name.cn_len = clp->cl_name.len;
+	memcpy(cup->cu_msg.cm_u.cm_name.cn_id, clp->cl_name.data,
+			clp->cl_name.len);
+
+	ret = cld_pipe_upcall(&cup->cu_msg);
+	if (!ret) {
+		ret = cup->cu_msg.cm_status;
+		clear_bit(NFSD4_CLIENT_STABLE, &clp->cl_flags);
+	}
+
+	free_cld_upcall(cup);
+out_err:
+	if (ret)
+		printk(KERN_ERR "NFSD: Unable to remove client "
+				"record from stable storage: %d\n", ret);
+}
+
+/* Check for presence of a record, and update its timestamp */
+static int
+nfsd4_cld_check(struct nfs4_client *clp)
+{
+	int ret;
+	struct cld_upcall *cup;
+
+	/* Don't upcall if one was already stored during this grace pd */
+	if (test_bit(NFSD4_CLIENT_STABLE, &clp->cl_flags))
+		return 0;
+
+	cup = alloc_cld_upcall();
+	if (!cup) {
+		printk(KERN_ERR "NFSD: Unable to check client record on "
+				"stable storage: %d\n", -ENOMEM);
+		return -ENOMEM;
+	}
+
+	cup->cu_msg.cm_cmd = Cld_Check;
+	cup->cu_msg.cm_u.cm_name.cn_len = clp->cl_name.len;
+	memcpy(cup->cu_msg.cm_u.cm_name.cn_id, clp->cl_name.data,
+			clp->cl_name.len);
+
+	ret = cld_pipe_upcall(&cup->cu_msg);
+	if (!ret) {
+		ret = cup->cu_msg.cm_status;
+		set_bit(NFSD4_CLIENT_STABLE, &clp->cl_flags);
+	}
+
+	free_cld_upcall(cup);
+	return ret;
+}
+
+static void
+nfsd4_cld_grace_done(time_t boot_time)
+{
+	int ret;
+	struct cld_upcall *cup;
+
+	cup = alloc_cld_upcall();
+	if (!cup) {
+		ret = -ENOMEM;
+		goto out_err;
+	}
+
+	cup->cu_msg.cm_cmd = Cld_GraceDone;
+	cup->cu_msg.cm_u.cm_gracetime = (int64_t)boot_time;
+	ret = cld_pipe_upcall(&cup->cu_msg);
+	if (!ret)
+		ret = cup->cu_msg.cm_status;
+
+	free_cld_upcall(cup);
+out_err:
+	if (ret)
+		printk(KERN_ERR "NFSD: Unable to end grace period: %d\n", ret);
+}
+
+static struct nfsd4_client_tracking_ops nfsd4_cld_tracking_ops = {
+	.init		= nfsd4_init_cld_pipe,
+	.exit		= nfsd4_remove_cld_pipe,
+	.create		= nfsd4_cld_create,
+	.remove		= nfsd4_cld_remove,
+	.check		= nfsd4_cld_check,
+	.grace_done	= nfsd4_cld_grace_done,
+};
+
 int
 nfsd4_client_tracking_init(void)
 {
 	int status;
+	struct path path;
 
-	client_tracking_ops = &nfsd4_legacy_tracking_ops;
+	if (!client_tracking_ops) {
+		client_tracking_ops = &nfsd4_cld_tracking_ops;
+		status = kern_path(nfs4_recoverydir(), LOOKUP_FOLLOW, &path);
+		if (!status) {
+			if (S_ISDIR(path.dentry->d_inode->i_mode))
+				client_tracking_ops =
+						&nfsd4_legacy_tracking_ops;
+			path_put(&path);
+		}
+	}
 
 	status = client_tracking_ops->init();
 	if (status) {
-- 
1.7.7.6


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH v8 6/6] nfsd: add notifier to handle mount/unmount of rpc_pipefs sb
  2012-03-02 20:42 [PATCH v8 0/6] nfsd: overhaul the client name tracking code Jeff Layton
                   ` (4 preceding siblings ...)
  2012-03-02 20:42 ` [PATCH v8 5/6] nfsd: add the infrastructure to handle the cld upcall Jeff Layton
@ 2012-03-02 20:42 ` Jeff Layton
  5 siblings, 0 replies; 15+ messages in thread
From: Jeff Layton @ 2012-03-02 20:42 UTC (permalink / raw)
  To: bfields; +Cc: linux-nfs, skinsbursky

In the event that rpc_pipefs isn't mounted when nfsd starts, we
must register a notifier to handle creating the dentry once it
is mounted, and to remove the dentry on unmount.

Signed-off-by: Jeff Layton <jlayton@redhat.com>
---
 fs/nfsd/nfs4recover.c |   52 ++++++++++++++++++++++++++++++++++++++++++++++++-
 1 files changed, 51 insertions(+), 1 deletions(-)

diff --git a/fs/nfsd/nfs4recover.c b/fs/nfsd/nfs4recover.c
index d928c58..31af1d0 100644
--- a/fs/nfsd/nfs4recover.c
+++ b/fs/nfsd/nfs4recover.c
@@ -38,6 +38,7 @@
 #include <linux/crypto.h>
 #include <linux/sched.h>
 #include <linux/fs.h>
+#include <linux/module.h>
 #include <net/net_namespace.h>
 #include <linux/sunrpc/rpc_pipe_fs.h>
 #include <linux/sunrpc/clnt.h>
@@ -658,6 +659,48 @@ nfsd4_cld_unregister_net(struct net *net, struct rpc_pipe *pipe)
 	}
 }
 
+static int
+rpc_pipefs_event(struct notifier_block *nb, unsigned long event, void *ptr)
+{
+	struct super_block *sb = ptr;
+	struct net *net = sb->s_fs_info;
+	struct dentry *dentry;
+	int ret = 0;
+
+	if (!try_module_get(THIS_MODULE))
+		return 0;
+
+	/* FIXME: containerize this code */
+	if (!cld_pipe || net != &init_net) {
+		module_put(THIS_MODULE);
+		return 0;
+	}
+
+	switch (event) {
+	case RPC_PIPEFS_MOUNT:
+		dentry = nfsd4_cld_register_sb(sb, cld_pipe);
+		if (IS_ERR(dentry)) {
+			ret = PTR_ERR(dentry);
+			break;
+		}
+		cld_pipe->dentry = dentry;
+		break;
+	case RPC_PIPEFS_UMOUNT:
+		if (cld_pipe->dentry)
+			nfsd4_cld_unregister_sb(cld_pipe);
+		break;
+	default:
+		ret = -ENOTSUPP;
+		break;
+	}
+	module_put(THIS_MODULE);
+	return ret;
+}
+
+static struct notifier_block nfsd4_cld_block = {
+	.notifier_call = rpc_pipefs_event,
+};
+
 /* Initialize rpc_pipefs pipe for communication with client tracking daemon */
 static int
 nfsd4_init_cld_pipe(void)
@@ -669,10 +712,14 @@ nfsd4_init_cld_pipe(void)
 	if (cld_pipe)
 		return 0;
 
+	ret = rpc_pipefs_notifier_register(&nfsd4_cld_block);
+	if (ret)
+		goto err;
+
 	cld_pipe = rpc_mkpipe_data(&cld_upcall_ops, RPC_PIPE_WAIT_FOR_OPEN);
 	if (IS_ERR(cld_pipe)) {
 		ret = PTR_ERR(cld_pipe);
-		goto err;
+		goto err_notifier;
 	}
 
 	dentry = nfsd4_cld_register_net(&init_net, cld_pipe);
@@ -686,6 +733,8 @@ nfsd4_init_cld_pipe(void)
 
 err_destroy_data:
 	rpc_destroy_pipe_data(cld_pipe);
+err_notifier:
+	rpc_pipefs_notifier_unregister(&nfsd4_cld_block);
 err:
 	cld_pipe = NULL;
 	printk(KERN_ERR "NFSD: unable to create nfsdcld upcall pipe (%d)\n",
@@ -696,6 +745,7 @@ err:
 static void
 nfsd4_remove_cld_pipe(void)
 {
+	rpc_pipefs_notifier_unregister(&nfsd4_cld_block);
 	/* FIXME: containerize... */
 	nfsd4_cld_unregister_net(&init_net, cld_pipe);
 	rpc_destroy_pipe_data(cld_pipe);
-- 
1.7.7.6


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Re: [PATCH v8 1/6] nfsd: add nfsd4_client_tracking_ops struct and a way to set it
  2012-03-02 20:42 ` [PATCH v8 1/6] nfsd: add nfsd4_client_tracking_ops struct and a way to set it Jeff Layton
@ 2012-03-05  8:34   ` Stanislav Kinsbursky
  0 siblings, 0 replies; 15+ messages in thread
From: Stanislav Kinsbursky @ 2012-03-05  8:34 UTC (permalink / raw)
  To: Jeff Layton; +Cc: bfields, linux-nfs

Hi, Jeff.
Thank for the update.
Would you mind, if I'll ask for some more?
I no, then, please, have a look at my comments below.


03.03.2012 00:42, Jeff Layton пишет:
> Abstract out the mechanism that we use to track clients into a set of
> client name tracking functions.
>
> This gives us a mechanism to plug in a new set of client tracking
> functions without disturbing the callers. It also gives us a way to
> decide on what tracking scheme to use at runtime.
>
> For now, this just looks like pointless abstraction, but later we'll
> add a new alternate scheme for tracking clients on stable storage.
>
> Signed-off-by: Jeff Layton<jlayton@redhat.com>
> ---
>   fs/nfsd/nfs4recover.c |  121 +++++++++++++++++++++++++++++++++++++++++++++----
>   fs/nfsd/nfs4state.c   |   46 +++++++------------
>   fs/nfsd/state.h       |   14 +++---
>   3 files changed, 136 insertions(+), 45 deletions(-)
>
> diff --git a/fs/nfsd/nfs4recover.c b/fs/nfsd/nfs4recover.c
> index 0b3e875..1dda803 100644
> --- a/fs/nfsd/nfs4recover.c
> +++ b/fs/nfsd/nfs4recover.c
> @@ -43,9 +43,20 @@
>
>   #define NFSDDBG_FACILITY                NFSDDBG_PROC
>
> +/* Declarations */
> +struct nfsd4_client_tracking_ops {
> +	int (*init)(void);
> +	void (*exit)(void);

These two routines looks like going to be per network namespace in future - can 
you pass struct net pointers in there?

> +	void (*create)(struct nfs4_client *);
> +	void (*remove)(struct nfs4_client *);
> +	int (*check)(struct nfs4_client *);
> +	void (*grace_done)(time_t);

Maybe this one requires struct net too?

> +};
> +
>   /* Globals */
>   static struct file *rec_file;
>   static char user_recovery_dirname[PATH_MAX] = "/var/lib/nfs/v4recovery";
> +static struct nfsd4_client_tracking_ops *client_tracking_ops;
>
>   static int
>   nfs4_save_creds(const struct cred **original_creds)
> @@ -117,7 +128,8 @@ out_no_tfm:
>   	return status;
>   }
>
> -void nfsd4_create_clid_dir(struct nfs4_client *clp)
> +static void
> +nfsd4_create_clid_dir(struct nfs4_client *clp)
>   {
>   	const struct cred *original_cred;
>   	char *dname = clp->cl_recdir;
> @@ -265,7 +277,7 @@ out_unlock:
>   	return status;
>   }
>
> -void
> +static void
>   nfsd4_remove_clid_dir(struct nfs4_client *clp)
>   {
>   	const struct cred *original_cred;
> @@ -292,7 +304,6 @@ out:
>   	if (status)
>   		printk("NFSD: Failed to remove expired client state directory"
>   				" %.*s\n", HEXDIR_LEN, clp->cl_recdir);
> -	return;
>   }
>
>   static int
> @@ -311,8 +322,8 @@ purge_old(struct dentry *parent, struct dentry *child)
>   	return 0;
>   }
>
> -void
> -nfsd4_recdir_purge_old(void) {
> +static void
> +nfsd4_recdir_purge_old(time_t boot_time __attribute__ ((unused))) {
>   	int status;
>
>   	if (!rec_file)
> @@ -343,7 +354,7 @@ load_recdir(struct dentry *parent, struct dentry *child)
>   	return 0;
>   }
>
> -int
> +static int
>   nfsd4_recdir_load(void) {
>   	int status;
>
> @@ -361,8 +372,8 @@ nfsd4_recdir_load(void) {
>    * Hold reference to the recovery directory.
>    */
>
> -void
> -nfsd4_init_recdir()
> +static int
> +nfsd4_init_recdir(void)
>   {
>   	const struct cred *original_cred;
>   	int status;
> @@ -377,20 +388,37 @@ nfsd4_init_recdir()
>   		printk("NFSD: Unable to change credentials to find recovery"
>   		       " directory: error %d\n",
>   		       status);
> -		return;
> +		return status;
>   	}
>
>   	rec_file = filp_open(user_recovery_dirname, O_RDONLY | O_DIRECTORY, 0);
>   	if (IS_ERR(rec_file)) {
>   		printk("NFSD: unable to find recovery directory %s\n",
>   				user_recovery_dirname);
> +		status = PTR_ERR(rec_file);
>   		rec_file = NULL;
>   	}
>
>   	nfs4_reset_creds(original_cred);
> +	return status;
>   }
>
> -void
> +static int
> +nfsd4_load_reboot_recovery_data(void)
> +{
> +	int status;
> +
> +	nfs4_lock_state();
> +	status = nfsd4_init_recdir();
> +	if (!status)
> +		status = nfsd4_recdir_load();
> +	nfs4_unlock_state();
> +	if (status)
> +		printk(KERN_ERR "NFSD: Failure reading reboot recovery data\n");
> +	return status;
> +}
> +
> +static void
>   nfsd4_shutdown_recdir(void)
>   {
>   	if (!rec_file)
> @@ -425,3 +453,76 @@ nfs4_recoverydir(void)
>   {
>   	return user_recovery_dirname;
>   }
> +
> +static int
> +nfsd4_check_legacy_client(struct nfs4_client *clp)
> +{
> +	if (nfsd4_find_reclaim_client(clp) != NULL)
> +		return 0;
> +	else
> +		return -ENOENT;
> +}
> +
> +static struct nfsd4_client_tracking_ops nfsd4_legacy_tracking_ops = {
> +	.init		= nfsd4_load_reboot_recovery_data,
> +	.exit		= nfsd4_shutdown_recdir,
> +	.create		= nfsd4_create_clid_dir,
> +	.remove		= nfsd4_remove_clid_dir,
> +	.check		= nfsd4_check_legacy_client,
> +	.grace_done	= nfsd4_recdir_purge_old,
> +};
> +
> +int
> +nfsd4_client_tracking_init(void)

This function requires struct net being passed.

> +{
> +	int status;
> +
> +	client_tracking_ops =&nfsd4_legacy_tracking_ops;
> +
> +	status = client_tracking_ops->init();
> +	if (status) {
> +		printk(KERN_WARNING "NFSD: Unable to initialize client "
> +				    "recovery tracking! (%d)\n", status);
> +		client_tracking_ops = NULL;
> +	}
> +	return status;
> +}
> +
> +void
> +nfsd4_client_tracking_exit(void)
> +{

Ditto.

> +	if (client_tracking_ops) {
> +		client_tracking_ops->exit();
> +		client_tracking_ops = NULL;
> +	}
> +}
> +
> +void
> +nfsd4_client_record_create(struct nfs4_client *clp)
> +{
> +	if (client_tracking_ops)
> +		client_tracking_ops->create(clp);
> +}
> +
> +void
> +nfsd4_client_record_remove(struct nfs4_client *clp)
> +{
> +	if (client_tracking_ops)
> +		client_tracking_ops->remove(clp);
> +}
> +
> +int
> +nfsd4_client_record_check(struct nfs4_client *clp)
> +{
> +	if (client_tracking_ops)
> +		return client_tracking_ops->check(clp);
> +
> +	return -EOPNOTSUPP;
> +}
> +
> +void
> +nfsd4_record_grace_done(time_t boot_time)
> +{
> +	if (client_tracking_ops)
> +		client_tracking_ops->grace_done(boot_time);
> +}
> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
> index c5cddd6..0c7ac26 100644
> --- a/fs/nfsd/nfs4state.c
> +++ b/fs/nfsd/nfs4state.c
> @@ -2045,7 +2045,7 @@ nfsd4_reclaim_complete(struct svc_rqst *rqstp, struct nfsd4_compound_state *csta
>   		goto out;
>
>   	status = nfs_ok;
> -	nfsd4_create_clid_dir(cstate->session->se_client);
> +	nfsd4_client_record_create(cstate->session->se_client);
>   out:
>   	nfs4_unlock_state();
>   	return status;
> @@ -2240,7 +2240,7 @@ nfsd4_setclientid_confirm(struct svc_rqst *rqstp,
>   			conf = find_confirmed_client_by_str(unconf->cl_recdir,
>   							    hash);
>   			if (conf) {
> -				nfsd4_remove_clid_dir(conf);
> +				nfsd4_client_record_remove(conf);
>   				expire_client(conf);
>   			}
>   			move_to_confirmed(unconf);
> @@ -3066,7 +3066,7 @@ static void
>   nfsd4_end_grace(void)
>   {
>   	dprintk("NFSD: end of grace period\n");
> -	nfsd4_recdir_purge_old();
> +	nfsd4_record_grace_done(boot_time);
>   	locks_end_grace(&nfsd4_manager);
>   	/*
>   	 * Now that every NFSv4 client has had the chance to recover and
> @@ -3115,7 +3115,7 @@ nfs4_laundromat(void)
>   		clp = list_entry(pos, struct nfs4_client, cl_lru);
>   		dprintk("NFSD: purging unused client (clientid %08x)\n",
>   			clp->cl_clientid.cl_id);
> -		nfsd4_remove_clid_dir(clp);
> +		nfsd4_client_record_remove(clp);
>   		expire_client(clp);
>   	}
>   	spin_lock(&recall_lock);
> @@ -3539,7 +3539,7 @@ nfsd4_open_confirm(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
>   	dprintk("NFSD: %s: success, seqid=%d stateid=" STATEID_FMT "\n",
>   		__func__, oc->oc_seqid, STATEID_VAL(&stp->st_stid.sc_stateid));
>
> -	nfsd4_create_clid_dir(oo->oo_owner.so_client);
> +	nfsd4_client_record_create(oo->oo_owner.so_client);
>   	status = nfs_ok;
>   out:
>   	if (!cstate->replay_owner)
> @@ -4397,19 +4397,13 @@ nfs4_release_reclaim(void)
>
>   /*
>    * called from OPEN, CLAIM_PREVIOUS with a new clientid. */
> -static struct nfs4_client_reclaim *
> -nfs4_find_reclaim_client(clientid_t *clid)
> +struct nfs4_client_reclaim *
> +nfsd4_find_reclaim_client(struct nfs4_client *clp)
>   {
>   	unsigned int strhashval;
> -	struct nfs4_client *clp;
>   	struct nfs4_client_reclaim *crp = NULL;
>
>
> -	/* find clientid in conf_id_hashtbl */
> -	clp = find_confirmed_client(clid);
> -	if (clp == NULL)
> -		return NULL;
> -
>   	dprintk("NFSD: nfs4_find_reclaim_client for %.*s with recdir %s\n",
>   		            clp->cl_name.len, clp->cl_name.data,
>   			    clp->cl_recdir);
> @@ -4430,7 +4424,14 @@ nfs4_find_reclaim_client(clientid_t *clid)
>   __be32
>   nfs4_check_open_reclaim(clientid_t *clid)
>   {
> -	return nfs4_find_reclaim_client(clid) ? nfs_ok : nfserr_reclaim_bad;
> +	struct nfs4_client *clp;
> +
> +	/* find clientid in conf_id_hashtbl */
> +	clp = find_confirmed_client(clid);
> +	if (clp == NULL)
> +		return nfserr_reclaim_bad;
> +
> +	return nfsd4_client_record_check(clp) ? nfserr_reclaim_bad : nfs_ok;
>   }
>
>   #ifdef CONFIG_NFSD_FAULT_INJECTION
> @@ -4577,19 +4578,6 @@ nfs4_state_init(void)
>   	reclaim_str_hashtbl_size = 0;
>   }
>
> -static void
> -nfsd4_load_reboot_recovery_data(void)
> -{
> -	int status;
> -
> -	nfs4_lock_state();
> -	nfsd4_init_recdir();
> -	status = nfsd4_recdir_load();
> -	nfs4_unlock_state();
> -	if (status)
> -		printk("NFSD: Failure reading reboot recovery data\n");
> -}
> -
>   /*
>    * Since the lifetime of a delegation isn't limited to that of an open, a
>    * client may quite reasonably hang on to a delegation as long as it has
> @@ -4642,7 +4630,7 @@ out_free_laundry:
>   int
>   nfs4_state_start(void)
>   {
> -	nfsd4_load_reboot_recovery_data();
> +	nfsd4_client_tracking_init();

Pass &init_net in nfsd4_client_tracking_init().

>   	return __nfs4_state_start();
>   }
>
> @@ -4676,7 +4664,7 @@ __nfs4_state_shutdown(void)
>   		unhash_delegation(dp);
>   	}
>
> -	nfsd4_shutdown_recdir();
> +	nfsd4_client_tracking_exit();

Ditto.

>   }
>
>   void
> diff --git a/fs/nfsd/state.h b/fs/nfsd/state.h
> index ffb5df1..e22f90f 100644
> --- a/fs/nfsd/state.h
> +++ b/fs/nfsd/state.h
> @@ -463,6 +463,7 @@ extern __be32 nfs4_preprocess_stateid_op(struct nfsd4_compound_state *cstate,
>   extern void nfs4_lock_state(void);
>   extern void nfs4_unlock_state(void);
>   extern int nfs4_in_grace(void);
> +extern struct nfs4_client_reclaim *nfsd4_find_reclaim_client(struct nfs4_client *crp);
>   extern __be32 nfs4_check_open_reclaim(clientid_t *clid);
>   extern void nfs4_free_openowner(struct nfs4_openowner *);
>   extern void nfs4_free_lockowner(struct nfs4_lockowner *);
> @@ -477,16 +478,17 @@ extern void nfsd4_destroy_callback_queue(void);
>   extern void nfsd4_shutdown_callback(struct nfs4_client *);
>   extern void nfs4_put_delegation(struct nfs4_delegation *dp);
>   extern __be32 nfs4_make_rec_clidname(char *clidname, struct xdr_netobj *clname);
> -extern void nfsd4_init_recdir(void);
> -extern int nfsd4_recdir_load(void);
> -extern void nfsd4_shutdown_recdir(void);
>   extern int nfs4_client_to_reclaim(const char *name);
>   extern int nfs4_has_reclaimed_state(const char *name, bool use_exchange_id);
> -extern void nfsd4_recdir_purge_old(void);
> -extern void nfsd4_create_clid_dir(struct nfs4_client *clp);
> -extern void nfsd4_remove_clid_dir(struct nfs4_client *clp);
>   extern void release_session_client(struct nfsd4_session *);
>   extern __be32 nfs4_validate_stateid(struct nfs4_client *, stateid_t *);
>   extern void nfsd4_purge_closed_stateid(struct nfs4_stateowner *);
>
> +/* nfs4recover operations */
> +extern int nfsd4_client_tracking_init(void);
> +extern void nfsd4_client_tracking_exit(void);
> +extern void nfsd4_client_record_create(struct nfs4_client *clp);
> +extern void nfsd4_client_record_remove(struct nfs4_client *clp);
> +extern int nfsd4_client_record_check(struct nfs4_client *clp);
> +extern void nfsd4_record_grace_done(time_t boot_time);
>   #endif   /* NFSD4_STATE_H */


-- 
Best regards,
Stanislav Kinsbursky

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v8 5/6] nfsd: add the infrastructure to handle the cld upcall
  2012-03-02 20:42 ` [PATCH v8 5/6] nfsd: add the infrastructure to handle the cld upcall Jeff Layton
@ 2012-03-05  8:48   ` Stanislav Kinsbursky
  2012-03-05 12:42     ` Jeff Layton
  0 siblings, 1 reply; 15+ messages in thread
From: Stanislav Kinsbursky @ 2012-03-05  8:48 UTC (permalink / raw)
  To: Jeff Layton; +Cc: bfields, linux-nfs

This variant is almost what was desired.
If you would be able to prepare one more version - please look at my comments below.

03.03.2012 00:42, Jeff Layton пишет:
> ...and add a mechanism for switching between the "legacy" tracker and
> the new one. The decision is made by looking to see whether the
> v4recoverydir exists. If it does, then the legacy client tracker is
> used.
>
> If it's not, then the kernel will create a "cld" pipe in rpc_pipefs.
> That pipe is used to talk to a daemon for handling the upcall.
>
> Signed-off-by: Jeff Layton<jlayton@redhat.com>
> ---
>   fs/nfsd/nfs4recover.c |  411 ++++++++++++++++++++++++++++++++++++++++++++++++-
>   1 files changed, 410 insertions(+), 1 deletions(-)
>
> diff --git a/fs/nfsd/nfs4recover.c b/fs/nfsd/nfs4recover.c
> index ccd9b97..d928c58 100644
> --- a/fs/nfsd/nfs4recover.c
> +++ b/fs/nfsd/nfs4recover.c
> @@ -1,5 +1,6 @@
>   /*
>   *  Copyright (c) 2004 The Regents of the University of Michigan.
> +*  Copyright (c) 2012 Jeff Layton<jlayton@redhat.com>
>   *  All rights reserved.
>   *
>   *  Andy Adamson<andros@citi.umich.edu>
> @@ -36,6 +37,11 @@
>   #include<linux/namei.h>
>   #include<linux/crypto.h>
>   #include<linux/sched.h>
> +#include<linux/fs.h>
> +#include<net/net_namespace.h>
> +#include<linux/sunrpc/rpc_pipe_fs.h>
> +#include<linux/sunrpc/clnt.h>
> +#include<linux/nfsd/cld.h>
>
>   #include "nfsd.h"
>   #include "state.h"
> @@ -478,12 +484,415 @@ static struct nfsd4_client_tracking_ops nfsd4_legacy_tracking_ops = {
>   	.grace_done	= nfsd4_recdir_purge_old,
>   };
>
> +/* Globals */
> +#define NFSD_PIPE_DIR		"nfsd"
> +#define NFSD_CLD_PIPE		"cld"
> +
> +static struct rpc_pipe *cld_pipe;
> +
> +/* list of cld_msg's that are currently in use */
> +static DEFINE_SPINLOCK(cld_lock);
> +static LIST_HEAD(cld_list);
> +static unsigned int cld_xid;
> +
> +struct cld_upcall {
> +	struct list_head	cu_list;
> +	struct task_struct	*cu_task;
> +	struct cld_msg	cu_msg;
> +};
> +
> +static int
> +__cld_pipe_upcall(struct cld_msg *cmsg)
> +{
> +	int ret;
> +	struct rpc_pipe_msg msg;
> +
> +	memset(&msg, 0, sizeof(msg));
> +	msg.data = cmsg;
> +	msg.len = sizeof(*cmsg);
> +
> +	/*
> +	 * Set task state before we queue the upcall. That prevents
> +	 * wake_up_process in the downcall from racing with schedule.
> +	 */
> +	set_current_state(TASK_UNINTERRUPTIBLE);
> +	ret = rpc_queue_upcall(cld_pipe,&msg);


Would be great to replace hard-coded cld_pipe here with a passed one.
For example, put this cld_pipe on cld_msg structure.
And initialize this pointer (hard-coded value for now) below in 
nfsd4_cld_create() and all other places where alloc_cld_upcall() is called.

BTW, maybe allocate this struct cld_upcall per NFS client somehow? And thus get 
rid of alloc-free calls?


> +	if (ret<  0) {
> +		set_current_state(TASK_RUNNING);
> +		goto out;
> +	}
> +
> +	schedule();
> +	set_current_state(TASK_RUNNING);
> +
> +	if (msg.errno<  0)
> +		ret = msg.errno;
> +out:
> +	return ret;
> +}
> +
> +static int
> +cld_pipe_upcall(struct cld_msg *cmsg)
> +{
> +	int ret;
> +
> +	/*
> +	 * -EAGAIN occurs when pipe is closed and reopened while there are
> +	 *  upcalls queued.
> +	 */
> +	do {
> +		ret = __cld_pipe_upcall(cmsg);
> +	} while (ret == -EAGAIN);
> +
> +	return ret;
> +}
> +
> +static ssize_t
> +cld_pipe_downcall(struct file *filp, const char __user *src, size_t mlen)
> +{
> +	struct cld_upcall *tmp, *cup;
> +	struct cld_msg *cmsg = (struct cld_msg *)src;
> +	uint32_t xid;
> +
> +	if (mlen != sizeof(*cmsg)) {
> +		dprintk("%s: got %lu bytes, expected %lu\n", __func__, mlen,
> +			sizeof(*cmsg));
> +		return -EINVAL;
> +	}
> +
> +	/* copy just the xid so we can try to find that */
> +	if (copy_from_user(&xid,&cmsg->cm_xid, sizeof(xid)) != 0) {
> +		dprintk("%s: error when copying xid from userspace", __func__);
> +		return -EFAULT;
> +	}
> +
> +	/* walk the list and find corresponding xid */
> +	cup = NULL;
> +	spin_lock(&cld_lock);
> +	list_for_each_entry(tmp,&cld_list, cu_list) {
> +		if (get_unaligned(&tmp->cu_msg.cm_xid) == xid) {
> +			cup = tmp;
> +			list_del_init(&cup->cu_list);
> +			break;
> +		}
> +	}
> +	spin_unlock(&cld_lock);
> +
> +	/* couldn't find upcall? */
> +	if (!cup) {
> +		dprintk("%s: couldn't find upcall -- xid=%u\n", __func__,
> +			cup->cu_msg.cm_xid);
> +		return -EINVAL;
> +	}
> +
> +	if (copy_from_user(&cup->cu_msg, src, mlen) != 0)
> +		return -EFAULT;
> +
> +	wake_up_process(cup->cu_task);
> +	return mlen;
> +}
> +
> +static void
> +cld_pipe_destroy_msg(struct rpc_pipe_msg *msg)
> +{
> +	struct cld_msg *cmsg = msg->data;
> +	struct cld_upcall *cup = container_of(cmsg, struct cld_upcall,
> +						 cu_msg);
> +
> +	/* errno>= 0 means we got a downcall */
> +	if (msg->errno>= 0)
> +		return;
> +
> +	wake_up_process(cup->cu_task);
> +}
> +
> +static const struct rpc_pipe_ops cld_upcall_ops = {
> +	.upcall		= rpc_pipe_generic_upcall,
> +	.downcall	= cld_pipe_downcall,
> +	.destroy_msg	= cld_pipe_destroy_msg,
> +};
> +
> +static struct dentry *
> +nfsd4_cld_register_sb(struct super_block *sb, struct rpc_pipe *pipe)
> +{
> +	struct dentry *dir, *dentry;
> +
> +	dir = rpc_d_lookup_sb(sb, NFSD_PIPE_DIR);
> +	if (dir == NULL)
> +		return ERR_PTR(-ENOENT);
> +	dentry = rpc_mkpipe_dentry(dir, NFSD_CLD_PIPE, NULL, pipe);
> +	dput(dir);
> +	return dentry;
> +}
> +
> +static void
> +nfsd4_cld_unregister_sb(struct rpc_pipe *pipe)
> +{
> +	if (pipe->dentry)
> +		rpc_unlink(pipe->dentry);
> +}
> +
> +static struct dentry *
> +nfsd4_cld_register_net(struct net *net, struct rpc_pipe *pipe)
> +{
> +	struct super_block *sb;
> +	struct dentry *dentry;
> +
> +	sb = rpc_get_sb_net(net);
> +	if (!sb)
> +		return NULL;
> +	dentry = nfsd4_cld_register_sb(sb, pipe);
> +	rpc_put_sb_net(net);
> +	return dentry;
> +}
> +
> +static void
> +nfsd4_cld_unregister_net(struct net *net, struct rpc_pipe *pipe)
> +{
> +	struct super_block *sb;
> +
> +	sb = rpc_get_sb_net(net);
> +	if (sb) {
> +		nfsd4_cld_unregister_sb(pipe);
> +		rpc_put_sb_net(net);
> +	}
> +}
> +
> +/* Initialize rpc_pipefs pipe for communication with client tracking daemon */
> +static int
> +nfsd4_init_cld_pipe(void)

This function will be called per network namespace context.
Could you, please, parametrize it by net instead of using hard-coded init_net 
few lines below?

> +{
> +	int ret;
> +	struct dentry *dentry;
> +
> +	/* FIXME: containerize this code */
> +	if (cld_pipe)
> +		return 0;
> +
> +	cld_pipe = rpc_mkpipe_data(&cld_upcall_ops, RPC_PIPE_WAIT_FOR_OPEN);
> +	if (IS_ERR(cld_pipe)) {
> +		ret = PTR_ERR(cld_pipe);
> +		goto err;
> +	}
> +
> +	dentry = nfsd4_cld_register_net(&init_net, cld_pipe);
> +	if (IS_ERR(dentry)) {
> +		ret = PTR_ERR(dentry);
> +		goto err_destroy_data;
> +	}
> +
> +	cld_pipe->dentry = dentry;
> +	return 0;
> +
> +err_destroy_data:
> +	rpc_destroy_pipe_data(cld_pipe);
> +err:
> +	cld_pipe = NULL;
> +	printk(KERN_ERR "NFSD: unable to create nfsdcld upcall pipe (%d)\n",
> +			ret);
> +	return ret;
> +}
> +
> +static void
> +nfsd4_remove_cld_pipe(void)
> +{

Ditto.

> +	/* FIXME: containerize... */
> +	nfsd4_cld_unregister_net(&init_net, cld_pipe);
> +	rpc_destroy_pipe_data(cld_pipe);
> +	cld_pipe = NULL;
> +}
> +
> +static struct cld_upcall *
> +alloc_cld_upcall(void)
> +{
> +	struct cld_upcall *new, *tmp;
> +
> +	new = kzalloc(sizeof(*new), GFP_KERNEL);
> +	if (!new)
> +		return new;
> +
> +	/* FIXME: hard cap on number in flight? */
> +restart_search:
> +	spin_lock(&cld_lock);
> +	list_for_each_entry(tmp,&cld_list, cu_list) {
> +		if (tmp->cu_msg.cm_xid == cld_xid) {
> +			cld_xid++;
> +			spin_unlock(&cld_lock);
> +			goto restart_search;
> +		}
> +	}
> +	new->cu_task = current;
> +	new->cu_msg.cm_vers = CLD_UPCALL_VERSION;
> +	put_unaligned(cld_xid++,&new->cu_msg.cm_xid);
> +	list_add(&new->cu_list,&cld_list);
> +	spin_unlock(&cld_lock);
> +
> +	dprintk("%s: allocated xid %u\n", __func__, new->cu_msg.cm_xid);
> +
> +	return new;
> +}
> +
> +static void
> +free_cld_upcall(struct cld_upcall *victim)
> +{
> +	spin_lock(&cld_lock);
> +	list_del(&victim->cu_list);
> +	spin_unlock(&cld_lock);
> +	kfree(victim);
> +}
> +
> +/* Ask daemon to create a new record */
> +static void
> +nfsd4_cld_create(struct nfs4_client *clp)
> +{
> +	int ret;
> +	struct cld_upcall *cup;
> +
> +	/* Don't upcall if it's already stored */
> +	if (test_bit(NFSD4_CLIENT_STABLE,&clp->cl_flags))
> +		return;
> +
> +	cup = alloc_cld_upcall();
> +	if (!cup) {
> +		ret = -ENOMEM;
> +		goto out_err;
> +	}
> +
> +	cup->cu_msg.cm_cmd = Cld_Create;
> +	cup->cu_msg.cm_u.cm_name.cn_len = clp->cl_name.len;
> +	memcpy(cup->cu_msg.cm_u.cm_name.cn_id, clp->cl_name.data,
> +			clp->cl_name.len);
> +
> +	ret = cld_pipe_upcall(&cup->cu_msg);
> +	if (!ret) {
> +		ret = cup->cu_msg.cm_status;
> +		set_bit(NFSD4_CLIENT_STABLE,&clp->cl_flags);
> +	}
> +
> +	free_cld_upcall(cup);
> +out_err:
> +	if (ret)
> +		printk(KERN_ERR "NFSD: Unable to create client "
> +				"record on stable storage: %d\n", ret);
> +}
> +
> +/* Ask daemon to create a new record */
> +static void
> +nfsd4_cld_remove(struct nfs4_client *clp)
> +{
> +	int ret;
> +	struct cld_upcall *cup;
> +
> +	/* Don't upcall if it's already removed */
> +	if (!test_bit(NFSD4_CLIENT_STABLE,&clp->cl_flags))
> +		return;
> +
> +	cup = alloc_cld_upcall();
> +	if (!cup) {
> +		ret = -ENOMEM;
> +		goto out_err;
> +	}
> +
> +	cup->cu_msg.cm_cmd = Cld_Remove;
> +	cup->cu_msg.cm_u.cm_name.cn_len = clp->cl_name.len;
> +	memcpy(cup->cu_msg.cm_u.cm_name.cn_id, clp->cl_name.data,
> +			clp->cl_name.len);
> +
> +	ret = cld_pipe_upcall(&cup->cu_msg);
> +	if (!ret) {
> +		ret = cup->cu_msg.cm_status;
> +		clear_bit(NFSD4_CLIENT_STABLE,&clp->cl_flags);
> +	}
> +
> +	free_cld_upcall(cup);
> +out_err:
> +	if (ret)
> +		printk(KERN_ERR "NFSD: Unable to remove client "
> +				"record from stable storage: %d\n", ret);
> +}
> +
> +/* Check for presence of a record, and update its timestamp */
> +static int
> +nfsd4_cld_check(struct nfs4_client *clp)
> +{
> +	int ret;
> +	struct cld_upcall *cup;
> +
> +	/* Don't upcall if one was already stored during this grace pd */
> +	if (test_bit(NFSD4_CLIENT_STABLE,&clp->cl_flags))
> +		return 0;
> +
> +	cup = alloc_cld_upcall();
> +	if (!cup) {
> +		printk(KERN_ERR "NFSD: Unable to check client record on "
> +				"stable storage: %d\n", -ENOMEM);
> +		return -ENOMEM;
> +	}
> +
> +	cup->cu_msg.cm_cmd = Cld_Check;
> +	cup->cu_msg.cm_u.cm_name.cn_len = clp->cl_name.len;
> +	memcpy(cup->cu_msg.cm_u.cm_name.cn_id, clp->cl_name.data,
> +			clp->cl_name.len);
> +
> +	ret = cld_pipe_upcall(&cup->cu_msg);
> +	if (!ret) {
> +		ret = cup->cu_msg.cm_status;
> +		set_bit(NFSD4_CLIENT_STABLE,&clp->cl_flags);
> +	}
> +
> +	free_cld_upcall(cup);
> +	return ret;
> +}
> +
> +static void
> +nfsd4_cld_grace_done(time_t boot_time)
> +{
> +	int ret;
> +	struct cld_upcall *cup;
> +
> +	cup = alloc_cld_upcall();
> +	if (!cup) {
> +		ret = -ENOMEM;
> +		goto out_err;
> +	}
> +
> +	cup->cu_msg.cm_cmd = Cld_GraceDone;
> +	cup->cu_msg.cm_u.cm_gracetime = (int64_t)boot_time;
> +	ret = cld_pipe_upcall(&cup->cu_msg);
> +	if (!ret)
> +		ret = cup->cu_msg.cm_status;
> +
> +	free_cld_upcall(cup);
> +out_err:
> +	if (ret)
> +		printk(KERN_ERR "NFSD: Unable to end grace period: %d\n", ret);
> +}
> +
> +static struct nfsd4_client_tracking_ops nfsd4_cld_tracking_ops = {
> +	.init		= nfsd4_init_cld_pipe,
> +	.exit		= nfsd4_remove_cld_pipe,
> +	.create		= nfsd4_cld_create,
> +	.remove		= nfsd4_cld_remove,
> +	.check		= nfsd4_cld_check,
> +	.grace_done	= nfsd4_cld_grace_done,
> +};
> +
>   int
>   nfsd4_client_tracking_init(void)
>   {
>   	int status;
> +	struct path path;
>
> -	client_tracking_ops =&nfsd4_legacy_tracking_ops;
> +	if (!client_tracking_ops) {
> +		client_tracking_ops =&nfsd4_cld_tracking_ops;
> +		status = kern_path(nfs4_recoverydir(), LOOKUP_FOLLOW,&path);
> +		if (!status) {
> +			if (S_ISDIR(path.dentry->d_inode->i_mode))
> +				client_tracking_ops =
> +						&nfsd4_legacy_tracking_ops;
> +			path_put(&path);
> +		}
> +	}
>
>   	status = client_tracking_ops->init();
>   	if (status) {


-- 
Best regards,
Stanislav Kinsbursky

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v8 5/6] nfsd: add the infrastructure to handle the cld upcall
  2012-03-05  8:48   ` Stanislav Kinsbursky
@ 2012-03-05 12:42     ` Jeff Layton
  2012-03-05 12:51       ` Stanislav Kinsbursky
  0 siblings, 1 reply; 15+ messages in thread
From: Jeff Layton @ 2012-03-05 12:42 UTC (permalink / raw)
  To: Stanislav Kinsbursky; +Cc: bfields, linux-nfs

On Mon, 05 Mar 2012 12:48:49 +0400
Stanislav Kinsbursky <skinsbursky@parallels.com> wrote:

> This variant is almost what was desired.
> If you would be able to prepare one more version - please look at my comments below.
> 
> 03.03.2012 00:42, Jeff Layton пишет:
> > ...and add a mechanism for switching between the "legacy" tracker and
> > the new one. The decision is made by looking to see whether the
> > v4recoverydir exists. If it does, then the legacy client tracker is
> > used.
> >
> > If it's not, then the kernel will create a "cld" pipe in rpc_pipefs.
> > That pipe is used to talk to a daemon for handling the upcall.
> >
> > Signed-off-by: Jeff Layton<jlayton@redhat.com>
> > ---
> >   fs/nfsd/nfs4recover.c |  411 ++++++++++++++++++++++++++++++++++++++++++++++++-
> >   1 files changed, 410 insertions(+), 1 deletions(-)
> >
> > diff --git a/fs/nfsd/nfs4recover.c b/fs/nfsd/nfs4recover.c
> > index ccd9b97..d928c58 100644
> > --- a/fs/nfsd/nfs4recover.c
> > +++ b/fs/nfsd/nfs4recover.c
> > @@ -1,5 +1,6 @@
> >   /*
> >   *  Copyright (c) 2004 The Regents of the University of Michigan.
> > +*  Copyright (c) 2012 Jeff Layton<jlayton@redhat.com>
> >   *  All rights reserved.
> >   *
> >   *  Andy Adamson<andros@citi.umich.edu>
> > @@ -36,6 +37,11 @@
> >   #include<linux/namei.h>
> >   #include<linux/crypto.h>
> >   #include<linux/sched.h>
> > +#include<linux/fs.h>
> > +#include<net/net_namespace.h>
> > +#include<linux/sunrpc/rpc_pipe_fs.h>
> > +#include<linux/sunrpc/clnt.h>
> > +#include<linux/nfsd/cld.h>
> >
> >   #include "nfsd.h"
> >   #include "state.h"
> > @@ -478,12 +484,415 @@ static struct nfsd4_client_tracking_ops nfsd4_legacy_tracking_ops = {
> >   	.grace_done	= nfsd4_recdir_purge_old,
> >   };
> >
> > +/* Globals */
> > +#define NFSD_PIPE_DIR		"nfsd"
> > +#define NFSD_CLD_PIPE		"cld"
> > +
> > +static struct rpc_pipe *cld_pipe;
> > +
> > +/* list of cld_msg's that are currently in use */
> > +static DEFINE_SPINLOCK(cld_lock);
> > +static LIST_HEAD(cld_list);
> > +static unsigned int cld_xid;
> > +
> > +struct cld_upcall {
> > +	struct list_head	cu_list;
> > +	struct task_struct	*cu_task;
> > +	struct cld_msg	cu_msg;
> > +};
> > +
> > +static int
> > +__cld_pipe_upcall(struct cld_msg *cmsg)
> > +{
> > +	int ret;
> > +	struct rpc_pipe_msg msg;
> > +
> > +	memset(&msg, 0, sizeof(msg));
> > +	msg.data = cmsg;
> > +	msg.len = sizeof(*cmsg);
> > +
> > +	/*
> > +	 * Set task state before we queue the upcall. That prevents
> > +	 * wake_up_process in the downcall from racing with schedule.
> > +	 */
> > +	set_current_state(TASK_UNINTERRUPTIBLE);
> > +	ret = rpc_queue_upcall(cld_pipe,&msg);
> 
> 
> Would be great to replace hard-coded cld_pipe here with a passed one.
> For example, put this cld_pipe on cld_msg structure.
> And initialize this pointer (hard-coded value for now) below in 
> nfsd4_cld_create() and all other places where alloc_cld_upcall() is called.
> 
> BTW, maybe allocate this struct cld_upcall per NFS client somehow? And thus get 
> rid of alloc-free calls?
> 
> 

Ok, I think I must have misunderstood what you meant when you said no
per-net ops required. I'll respin and see if I can get closer to what
you intended.

Since most of the upcalls are infrequent and come from nfsd threads,
the number of calls in flight will always be less than the order of the
number of nfsd threads. That value may be much less than the number of
nfs4_clients, so we're probably best off allocating these dynamically.

Now that I've had some time to look, I think what you're probably going
to want to do eventually is to add a new per-namespace object for nfsd,
similar to struct nfs_net for the nfs client. This container will hold
the pointer to the upcall pipe as well as other objects.

The nfs4_client objects for nfsd will then need to be per-namespace,
and will have a pointer back to "struct nfsd_net" or whatever you end up
calling it. From that they'll be able to get the rpc_pipe pointer.

For now, I'll just hardcode init_net in the appropriate places under the
assumption that eventually you'll be able to get a rpc_pipe pointer
given a pointer to an nfs4_client struct.

Thanks,
-- 
Jeff Layton <jlayton@redhat.com>

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v8 5/6] nfsd: add the infrastructure to handle the cld upcall
  2012-03-05 12:42     ` Jeff Layton
@ 2012-03-05 12:51       ` Stanislav Kinsbursky
  2012-03-05 14:39         ` Jeff Layton
  0 siblings, 1 reply; 15+ messages in thread
From: Stanislav Kinsbursky @ 2012-03-05 12:51 UTC (permalink / raw)
  To: Jeff Layton; +Cc: bfields, linux-nfs

05.03.2012 16:42, Jeff Layton пишет:
>>
>>
>> Would be great to replace hard-coded cld_pipe here with a passed one.
>> For example, put this cld_pipe on cld_msg structure.
>> And initialize this pointer (hard-coded value for now) below in
>> nfsd4_cld_create() and all other places where alloc_cld_upcall() is called.
>>
>> BTW, maybe allocate this struct cld_upcall per NFS client somehow? And thus get
>> rid of alloc-free calls?
>>
>>
>
> Ok, I think I must have misunderstood what you meant when you said no
> per-net ops required. I'll respin and see if I can get closer to what
> you intended.
>
> Since most of the upcalls are infrequent and come from nfsd threads,
> the number of calls in flight will always be less than the order of the
> number of nfsd threads. That value may be much less than the number of
> nfs4_clients, so we're probably best off allocating these dynamically.
>


Sure, it's up to you. Only wanna note, that this dynamic allocation in several 
places looks a little bit confusing to me.


> Now that I've had some time to look, I think what you're probably going
> to want to do eventually is to add a new per-namespace object for nfsd,
> similar to struct nfs_net for the nfs client. This container will hold
> the pointer to the upcall pipe as well as other objects.
>

Exactly.

> The nfs4_client objects for nfsd will then need to be per-namespace,
> and will have a pointer back to "struct nfsd_net" or whatever you end up
> calling it. From that they'll be able to get the rpc_pipe pointer.
>

You most probably right (I'm not working on NFSd yet - so hard to be sure).

> For now, I'll just hardcode init_net in the appropriate places under the
> assumption that eventually you'll be able to get a rpc_pipe pointer
> given a pointer to an nfs4_client struct.
>

Yes, thank you, Jeff.

-- 
Best regards,
Stanislav Kinsbursky

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v8 5/6] nfsd: add the infrastructure to handle the cld upcall
  2012-03-05 12:51       ` Stanislav Kinsbursky
@ 2012-03-05 14:39         ` Jeff Layton
  2012-03-05 14:53           ` Stanislav Kinsbursky
  0 siblings, 1 reply; 15+ messages in thread
From: Jeff Layton @ 2012-03-05 14:39 UTC (permalink / raw)
  To: Stanislav Kinsbursky; +Cc: bfields, linux-nfs

On Mon, 05 Mar 2012 16:51:03 +0400
Stanislav Kinsbursky <skinsbursky@parallels.com> wrote:

> 05.03.2012 16:42, Jeff Layton пишет:
> >>
> >>
> >> Would be great to replace hard-coded cld_pipe here with a passed one.
> >> For example, put this cld_pipe on cld_msg structure.
> >> And initialize this pointer (hard-coded value for now) below in
> >> nfsd4_cld_create() and all other places where alloc_cld_upcall() is called.
> >>
> >> BTW, maybe allocate this struct cld_upcall per NFS client somehow? And thus get
> >> rid of alloc-free calls?
> >>
> >>
> >
> > Ok, I think I must have misunderstood what you meant when you said no
> > per-net ops required. I'll respin and see if I can get closer to what
> > you intended.
> >
> > Since most of the upcalls are infrequent and come from nfsd threads,
> > the number of calls in flight will always be less than the order of the
> > number of nfsd threads. That value may be much less than the number of
> > nfs4_clients, so we're probably best off allocating these dynamically.
> >
> 
> 
> Sure, it's up to you. Only wanna note, that this dynamic allocation in several 
> places looks a little bit confusing to me.
> 
> 
> > Now that I've had some time to look, I think what you're probably going
> > to want to do eventually is to add a new per-namespace object for nfsd,
> > similar to struct nfs_net for the nfs client. This container will hold
> > the pointer to the upcall pipe as well as other objects.
> >
> 
> Exactly.
> 
> > The nfs4_client objects for nfsd will then need to be per-namespace,
> > and will have a pointer back to "struct nfsd_net" or whatever you end up
> > calling it. From that they'll be able to get the rpc_pipe pointer.
> >
> 
> You most probably right (I'm not working on NFSd yet - so hard to be sure).
> 
> > For now, I'll just hardcode init_net in the appropriate places under the
> > assumption that eventually you'll be able to get a rpc_pipe pointer
> > given a pointer to an nfs4_client struct.
> >
> 
> Yes, thank you, Jeff.
> 

Ok, I've started looking at the existing examples to try and ease the
eventual containerization of this. Am I correct that the existing
blocklayout pipe containerization is not yet complete?

AIUI, you have a rpc_pipe per nfs_net struct, and those structs are
allocated on a per-net basis. bl_pipe_downcall does this however:

        if (copy_from_user(&bl_mount_reply, src, mlen) != 0)
                return -EFAULT;

...but bl_mount_reply is global:

    static struct bl_dev_msg bl_mount_reply;

So, if you're getting downcalls in two different network namespaces at
the same time this could end up corrupt. It seems like bl_mount_reply
needs to be part of nfs_net as well.

When you get a downcall, you have a filp pointer. Is there any way to
determine the "correct" nfs_net object for a particular filp?

-- 
Jeff Layton <jlayton@redhat.com>

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v8 5/6] nfsd: add the infrastructure to handle the cld upcall
  2012-03-05 14:39         ` Jeff Layton
@ 2012-03-05 14:53           ` Stanislav Kinsbursky
  2012-03-05 15:16             ` Jeff Layton
  0 siblings, 1 reply; 15+ messages in thread
From: Stanislav Kinsbursky @ 2012-03-05 14:53 UTC (permalink / raw)
  To: Jeff Layton; +Cc: bfields, linux-nfs

05.03.2012 18:39, Jeff Layton пишет:
>
> Ok, I've started looking at the existing examples to try and ease the
> eventual containerization of this. Am I correct that the existing
> blocklayout pipe containerization is not yet complete?
>
> AIUI, you have a rpc_pipe per nfs_net struct, and those structs are
> allocated on a per-net basis. bl_pipe_downcall does this however:
>
>          if (copy_from_user(&bl_mount_reply, src, mlen) != 0)
>                  return -EFAULT;
>
> ...but bl_mount_reply is global:
>
>      static struct bl_dev_msg bl_mount_reply;
>
> So, if you're getting downcalls in two different network namespaces at
> the same time this could end up corrupt. It seems like bl_mount_reply
> needs to be part of nfs_net as well.

Thanks for this catch, Jeff!
I need to fix this.

>
> When you get a downcall, you have a filp pointer. Is there any way to
> determine the "correct" nfs_net object for a particular filp?
>

Yes, sure.
This nfs_net object will be, obviously, in the same network namespace, as PipeFS 
superblock. Network namespace is stored on PipeFS sb->s_fs_info.
IOW, code below should works:

struct nfs_net *nn = net_generic(file->f_dentry->d_sb->s_fs_info, nfs_net_id);

-- 
Best regards,
Stanislav Kinsbursky

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v8 5/6] nfsd: add the infrastructure to handle the cld upcall
  2012-03-05 14:53           ` Stanislav Kinsbursky
@ 2012-03-05 15:16             ` Jeff Layton
  2012-03-05 15:48               ` Stanislav Kinsbursky
  0 siblings, 1 reply; 15+ messages in thread
From: Jeff Layton @ 2012-03-05 15:16 UTC (permalink / raw)
  To: Stanislav Kinsbursky; +Cc: bfields, linux-nfs

On Mon, 05 Mar 2012 18:53:27 +0400
Stanislav Kinsbursky <skinsbursky@parallels.com> wrote:

> 05.03.2012 18:39, Jeff Layton пишет:
> >
> > Ok, I've started looking at the existing examples to try and ease the
> > eventual containerization of this. Am I correct that the existing
> > blocklayout pipe containerization is not yet complete?
> >
> > AIUI, you have a rpc_pipe per nfs_net struct, and those structs are
> > allocated on a per-net basis. bl_pipe_downcall does this however:
> >
> >          if (copy_from_user(&bl_mount_reply, src, mlen) != 0)
> >                  return -EFAULT;
> >
> > ...but bl_mount_reply is global:
> >
> >      static struct bl_dev_msg bl_mount_reply;
> >
> > So, if you're getting downcalls in two different network namespaces at
> > the same time this could end up corrupt. It seems like bl_mount_reply
> > needs to be part of nfs_net as well.
> 
> Thanks for this catch, Jeff!
> I need to fix this.
> 
> >
> > When you get a downcall, you have a filp pointer. Is there any way to
> > determine the "correct" nfs_net object for a particular filp?
> >
> 
> Yes, sure.
> This nfs_net object will be, obviously, in the same network namespace, as PipeFS 
> superblock. Network namespace is stored on PipeFS sb->s_fs_info.
> IOW, code below should works:
> 
> struct nfs_net *nn = net_generic(file->f_dentry->d_sb->s_fs_info, nfs_net_id);
> 

Ok, I'm going to have to do something similar for the cld upcall
eventually. We'll need a per-ns list of upcalls in flight, xid counter
and spinlock.

What may be best is to go ahead and introduce a nfsd_net struct as a
per-net object start putting stuff inside it.

-- 
Jeff Layton <jlayton@redhat.com>

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v8 5/6] nfsd: add the infrastructure to handle the cld upcall
  2012-03-05 15:16             ` Jeff Layton
@ 2012-03-05 15:48               ` Stanislav Kinsbursky
  0 siblings, 0 replies; 15+ messages in thread
From: Stanislav Kinsbursky @ 2012-03-05 15:48 UTC (permalink / raw)
  To: Jeff Layton; +Cc: bfields, linux-nfs

05.03.2012 19:16, Jeff Layton пишет:
> On Mon, 05 Mar 2012 18:53:27 +0400
> Stanislav Kinsbursky<skinsbursky@parallels.com>  wrote:
>
>> 05.03.2012 18:39, Jeff Layton пишет:
>>>
>>> Ok, I've started looking at the existing examples to try and ease the
>>> eventual containerization of this. Am I correct that the existing
>>> blocklayout pipe containerization is not yet complete?
>>>
>>> AIUI, you have a rpc_pipe per nfs_net struct, and those structs are
>>> allocated on a per-net basis. bl_pipe_downcall does this however:
>>>
>>>           if (copy_from_user(&bl_mount_reply, src, mlen) != 0)
>>>                   return -EFAULT;
>>>
>>> ...but bl_mount_reply is global:
>>>
>>>       static struct bl_dev_msg bl_mount_reply;
>>>
>>> So, if you're getting downcalls in two different network namespaces at
>>> the same time this could end up corrupt. It seems like bl_mount_reply
>>> needs to be part of nfs_net as well.
>>
>> Thanks for this catch, Jeff!
>> I need to fix this.
>>
>>>
>>> When you get a downcall, you have a filp pointer. Is there any way to
>>> determine the "correct" nfs_net object for a particular filp?
>>>
>>
>> Yes, sure.
>> This nfs_net object will be, obviously, in the same network namespace, as PipeFS
>> superblock. Network namespace is stored on PipeFS sb->s_fs_info.
>> IOW, code below should works:
>>
>> struct nfs_net *nn = net_generic(file->f_dentry->d_sb->s_fs_info, nfs_net_id);
>>
>
> Ok, I'm going to have to do something similar for the cld upcall
> eventually. We'll need a per-ns list of upcalls in flight, xid counter
> and spinlock.
>
> What may be best is to go ahead and introduce a nfsd_net struct as a
> per-net object start putting stuff inside it.
>

Agreed.
Would be cool, if you'll introduce this with your patch set.

-- 
Best regards,
Stanislav Kinsbursky

^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2012-03-05 15:49 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-03-02 20:42 [PATCH v8 0/6] nfsd: overhaul the client name tracking code Jeff Layton
2012-03-02 20:42 ` [PATCH v8 1/6] nfsd: add nfsd4_client_tracking_ops struct and a way to set it Jeff Layton
2012-03-05  8:34   ` Stanislav Kinsbursky
2012-03-02 20:42 ` [PATCH v8 2/6] sunrpc: create nfsd dir in rpc_pipefs Jeff Layton
2012-03-02 20:42 ` [PATCH v8 3/6] nfsd: convert nfs4_client->cl_cb_flags to a generic flags field Jeff Layton
2012-03-02 20:42 ` [PATCH v8 4/6] nfsd: add a header describing upcall to nfsdcld Jeff Layton
2012-03-02 20:42 ` [PATCH v8 5/6] nfsd: add the infrastructure to handle the cld upcall Jeff Layton
2012-03-05  8:48   ` Stanislav Kinsbursky
2012-03-05 12:42     ` Jeff Layton
2012-03-05 12:51       ` Stanislav Kinsbursky
2012-03-05 14:39         ` Jeff Layton
2012-03-05 14:53           ` Stanislav Kinsbursky
2012-03-05 15:16             ` Jeff Layton
2012-03-05 15:48               ` Stanislav Kinsbursky
2012-03-02 20:42 ` [PATCH v8 6/6] nfsd: add notifier to handle mount/unmount of rpc_pipefs sb Jeff Layton

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.