[PATCH 0/2] nfsd: bugfixes for

[PATCH 0/2] nfsd: bugfixes for

[Jeff Layton]

These patches are a couple of bugfixes for some problems I noticed
while working on the nfsdcld patches. Bruce asked that I break them
out separately and rebase the rest of the set on top.

I've tested earlier patchsets with similar changes, but have not
heavily tested these individually. Unfortunately, I might not get to
it for a little while.

Jeff Layton (2):
  nfsd: convert nfs4_client->cl_cb_flags to a generic flags field
  nfsd: only initialize client tracking if nfs4_state_start is
    successful

 fs/nfsd/nfs4callback.c |   14 +++++-----
 fs/nfsd/nfs4proc.c     |    3 +-
 fs/nfsd/nfs4recover.c  |    7 ++---
 fs/nfsd/nfs4state.c    |   64 ++++++++++++++++++++++++++++++------------------
 fs/nfsd/state.h        |   11 +++++---
 5 files changed, 59 insertions(+), 40 deletions(-)

-- 
1.7.7.6

[PATCH 1/2] nfsd: convert nfs4_client->cl_cb_flags to a generic flags field

[Jeff Layton]

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  |    7 +++----
 fs/nfsd/nfs4state.c    |   38 +++++++++++++++++++++++++++-----------
 fs/nfsd/state.h        |   11 +++++++----
 5 files changed, 46 insertions(+), 27 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 0b3e875..6523809 100644
--- a/fs/nfsd/nfs4recover.c
+++ b/fs/nfsd/nfs4recover.c
@@ -126,9 +126,8 @@ void 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);
@@ -271,13 +270,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)
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index c5cddd6..e547b27 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?: */
@@ -4398,18 +4398,11 @@ 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)
+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);
@@ -4424,13 +4417,36 @@ nfs4_find_reclaim_client(clientid_t *clid)
 	return NULL;
 }
 
+static int
+nfsd4_client_record_check(struct nfs4_client *clp)
+{
+	/* did we already find that this client is stable? */
+	if (test_bit(NFSD4_CLIENT_STABLE, &clp->cl_flags))
+		return 0;
+
+	/* 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;
+}
+
 /*
 * Called from OPEN. Look for clientid in reclaim list.
 */
 __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
diff --git a/fs/nfsd/state.h b/fs/nfsd/state.h
index ffb5df1..b9c036d 100644
--- a/fs/nfsd/state.h
+++ b/fs/nfsd/state.h
@@ -245,14 +245,17 @@ 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	(1 << NFSD4_CLIENT_CB_UPDATE | \
+					 1 << NFSD4_CLIENT_CB_KILL)
+	unsigned long		cl_flags;
 	struct rpc_clnt		*cl_cb_client;
 	u32			cl_cb_ident;
 #define NFSD4_CB_UP		0
-- 
1.7.7.6

[PATCH 2/2] nfsd: only initialize client tracking if nfs4_state_start is successful

[Jeff Layton]

The current code never calls nfsd4_client_tracking_exit if
nfs4_state_start returns an error. Also, it's better to go ahead and
consolidate these functions since one is just a trivial wrapper around
the other.

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

diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index e547b27..1198fe8 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -4629,21 +4629,26 @@ set_max_delegations(void)
 
 /* initialization to perform when the nfsd service is started: */
 
-static int
-__nfs4_state_start(void)
+int
+nfs4_state_start(void)
 {
 	int ret;
 
+	nfsd4_load_reboot_recovery_data();
 	boot_time = get_seconds();
 	locks_start_grace(&nfsd4_manager);
 	printk(KERN_INFO "NFSD: starting %ld-second grace period\n",
 	       nfsd4_grace);
 	ret = set_callback_cred();
-	if (ret)
-		return -ENOMEM;
+	if (ret) {
+		ret = -ENOMEM;
+		goto out_recovery;
+	}
 	laundry_wq = create_singlethread_workqueue("nfsd4");
-	if (laundry_wq == NULL)
-		return -ENOMEM;
+	if (laundry_wq == NULL) {
+		ret = -ENOMEM;
+		goto out_recovery;
+	}
 	ret = nfsd4_create_callback_queue();
 	if (ret)
 		goto out_free_laundry;
@@ -4652,16 +4657,11 @@ __nfs4_state_start(void)
 	return 0;
 out_free_laundry:
 	destroy_workqueue(laundry_wq);
+out_recovery:
+	nfsd4_shutdown_recdir();
 	return ret;
 }
 
-int
-nfs4_state_start(void)
-{
-	nfsd4_load_reboot_recovery_data();
-	return __nfs4_state_start();
-}
-
 static void
 __nfs4_state_shutdown(void)
 {
-- 
1.7.7.6

Re: [PATCH 1/2] nfsd: convert nfs4_client->cl_cb_flags to a generic flags field

[J. Bruce Fields]

On Mon, Mar 05, 2012 at 11:42:35AM -0500, Jeff Layton wrote:
> 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.

I think this is right, and agree that flags make more sense.  But as a
quick bugfix only, how about this?

--b.

commit a61eba426304acb3f6ef32e0505e2001fc695107
Author: J. Bruce Fields <bfields@redhat.com>
Date:   Tue Mar 6 14:35:16 2012 -0500

    nfsd4: don't set cl_firststate on first reclaim in 4.1 case
    
    We set cl_firststate when we first decide that a client will be
    permitted to reclaim state on next boot.  This happens:
    
    	- for new 4.0 clients, when they confirm their first open
    	- for returning 4.0 clients, when they reclaim their first open
    	- for 4.1+ clients, when they perform reclaim_complete
    
    We also use cl_firststate to decide whether a reclaim_complete has
    already been performed, in the 4.1+ case.
    
    We were setting it on 4.1 open reclaims, which caused spurious
    COMPLETE_ALREADY errors on RECLAIM_COMPLETE from an nfs4.1 client with
    anything to reclaim.
    
    Reported-by: Jeff Layton <jlayton@redhat.com>
    Signed-off-by: J. Bruce Fields <bfields@redhat.com>

diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index 967c677..207c3bd 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -2778,10 +2778,15 @@ nfs4_upgrade_open(struct svc_rqst *rqstp, struct nfs4_file *fp, struct svc_fh *c
 
 
 static void
-nfs4_set_claim_prev(struct nfsd4_open *open)
+nfs4_set_claim_prev(struct nfsd4_open *open, bool has_session)
 {
 	open->op_openowner->oo_flags |= NFS4_OO_CONFIRMED;
-	open->op_openowner->oo_owner.so_client->cl_firststate = 1;
+	/*
+	 * On a 4.1+ client, we don't create a state record for a client
+	 * until it performs RECLAIM_COMPLETE:
+	 */
+	if (!has_session)
+		open->op_openowner->oo_owner.so_client->cl_firststate = 1;
 }
 
 /* Should we give out recallable state?: */
@@ -3044,7 +3049,7 @@ out:
 	if (fp)
 		put_nfs4_file(fp);
 	if (status == 0 && open->op_claim_type == NFS4_OPEN_CLAIM_PREVIOUS)
-		nfs4_set_claim_prev(open);
+		nfs4_set_claim_prev(open, nfsd4_has_session(&resp->cstate));
 	/*
 	* To finish the open response, we just need to set the rflags.
 	*/

Re: [PATCH 1/2] nfsd: convert nfs4_client->cl_cb_flags to a generic flags field

[J. Bruce Fields]

On Tue, Mar 06, 2012 at 03:11:19PM -0500, J. Bruce Fields wrote:
> On Mon, Mar 05, 2012 at 11:42:35AM -0500, Jeff Layton wrote:
> > 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.
> 
> I think this is right, and agree that flags make more sense.  But as a
> quick bugfix only, how about this?

Also I think there's another preexisting bug here.  Yuch.

commit e02c696c47958adb0a2f5499aa6032288c5b475b
Author: J. Bruce Fields <bfields@redhat.com>
Date:   Tue Mar 6 14:43:36 2012 -0500

    nfsd4: purge stable client records with insufficient state
    
    To escape having your stable storage record purged at the end of the
    grace period, it's not sufficient to simply have performed a
    setclientid_confirm; you also need to meet the same requirements as
    someone creating a new record: either you should have done an open or
    open reclaim (in the 4.0 case) or a reclaim_complete (in the 4.1 case).
    
    Signed-off-by: J. Bruce Fields <bfields@redhat.com>

diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index 207c3bd..c9c446d 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -4415,7 +4415,9 @@ nfs4_has_reclaimed_state(const char *name, bool use_exchange_id)
 	struct nfs4_client *clp;
 
 	clp = find_confirmed_client_by_str(name, strhashval);
-	return clp ? 1 : 0;
+	if (!clp)
+		return 0;
+	return clp->cl_firststate;
 }
 
 /*

Re: [PATCH 1/2] nfsd: convert nfs4_client->cl_cb_flags to a generic flags field

[Jeff Layton]

On Tue, 6 Mar 2012 15:14:38 -0500
"J. Bruce Fields" <bfields@fieldses.org> wrote:

> On Tue, Mar 06, 2012 at 03:11:19PM -0500, J. Bruce Fields wrote:
> > On Mon, Mar 05, 2012 at 11:42:35AM -0500, Jeff Layton wrote:
> > > 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.
> > 
> > I think this is right, and agree that flags make more sense.  But as a
> > quick bugfix only, how about this?
> 
> Also I think there's another preexisting bug here.  Yuch.
> 
> commit e02c696c47958adb0a2f5499aa6032288c5b475b
> Author: J. Bruce Fields <bfields@redhat.com>
> Date:   Tue Mar 6 14:43:36 2012 -0500
> 
>     nfsd4: purge stable client records with insufficient state
>     
>     To escape having your stable storage record purged at the end of the
>     grace period, it's not sufficient to simply have performed a
>     setclientid_confirm; you also need to meet the same requirements as
>     someone creating a new record: either you should have done an open or
>     open reclaim (in the 4.0 case) or a reclaim_complete (in the 4.1 case).
>     
>     Signed-off-by: J. Bruce Fields <bfields@redhat.com>
> 
> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
> index 207c3bd..c9c446d 100644
> --- a/fs/nfsd/nfs4state.c
> +++ b/fs/nfsd/nfs4state.c
> @@ -4415,7 +4415,9 @@ nfs4_has_reclaimed_state(const char *name, bool use_exchange_id)
>  	struct nfs4_client *clp;
>  
>  	clp = find_confirmed_client_by_str(name, strhashval);
> -	return clp ? 1 : 0;
> +	if (!clp)
> +		return 0;
> +	return clp->cl_firststate;
>  }
>  
>  /*

This patch and the previous one look fine to me. Let me know when you
get them committed and I'll plan to rebase my set on top of them. It
might be a little while before I can get to it though...

-- 
Jeff Layton <jlayton@redhat.com>

Re: [PATCH 1/2] nfsd: convert nfs4_client->cl_cb_flags to a generic flags field

[J. Bruce Fields]

On Tue, Mar 06, 2012 at 03:14:38PM -0500, J. Bruce Fields wrote:
> On Tue, Mar 06, 2012 at 03:11:19PM -0500, J. Bruce Fields wrote:
> > On Mon, Mar 05, 2012 at 11:42:35AM -0500, Jeff Layton wrote:
> > > 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.
> > 
> > I think this is right, and agree that flags make more sense.  But as a
> > quick bugfix only, how about this?
> 
> Also I think there's another preexisting bug here.  Yuch.

And one more minor leak on error.

commit a8ae08ebf1f336808e20c1c275f68d36d88e0682
Author: J. Bruce Fields <bfields@redhat.com>
Date:   Tue Mar 6 15:52:04 2012 -0500

    nfsd4: fix recovery-entry leak nfsd startup failure
    
    Another leak on error
    
    Signed-off-by: J. Bruce Fields <bfields@redhat.com>

diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index ffb46d6..90c7e06 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -4706,6 +4706,7 @@ nfs4_state_start(void)
 out_free_laundry:
 	destroy_workqueue(laundry_wq);
 out_recovery:
+	nfs4_release_reclaim();
 	nfsd4_shutdown_recdir();
 	return ret;
 }