Re: [PATCH v3 7/8] nfsd: set and test NFSD4_CLIENT_STABLE bit to reduce nfsdcltrack upcalls

["J. Bruce Fields" <bfields@fieldses.org>]

On Mon, Sep 08, 2014 at 10:46:48AM -0400, Jeff Layton wrote:
> The nfsdcltrack upcall doesn't utilize the NFSD4_CLIENT_STABLE flag,
> which basically results in an upcall every time we call into the client
> tracking ops.
> 
> Change it to set this bit on a successful "check" or "create" request,
> and clear it on a "remove" request.  Also, check to see if that bit is
> set before upcalling on a "check" or "remove" request, and skip
> upcalling appropriately, depending on its state.
> 
> Signed-off-by: Jeff Layton <jlayton@primarydata.com>
> ---
>  fs/nfsd/nfs4recover.c | 22 ++++++++++++++++++----
>  1 file changed, 18 insertions(+), 4 deletions(-)
> 
> diff --git a/fs/nfsd/nfs4recover.c b/fs/nfsd/nfs4recover.c
> index ae313861a6f5..5697e9d2e875 100644
> --- a/fs/nfsd/nfs4recover.c
> +++ b/fs/nfsd/nfs4recover.c
> @@ -1291,7 +1291,8 @@ nfsd4_umh_cltrack_create(struct nfs4_client *clp)
>  	grace_start = nfsd4_cltrack_grace_start(nn->boot_time);
>  
>  	nfsd4_cltrack_upcall_lock(clp);
> -	nfsd4_umh_cltrack_upcall("create", hexid, reclaim_complete, grace_start);
> +	if (!nfsd4_umh_cltrack_upcall("create", hexid, reclaim_complete, grace_start))
> +		set_bit(NFSD4_CLIENT_STABLE, &clp->cl_flags);

Don't you also want to skip the upcall when STABLE is set?

(Not a big deal in the 4.1 case as it's only called once per client (in
RECLAIM_COMPLETE) but annoying in the 4.0 case where it's called on
every OPEN_CONFIRM.)

--b.

>  	nfsd4_cltrack_upcall_unlock(clp);
>  
>  	kfree(reclaim_complete);
> @@ -1304,6 +1305,9 @@ nfsd4_umh_cltrack_remove(struct nfs4_client *clp)
>  {
>  	char *hexid;
>  
> +	if (!test_bit(NFSD4_CLIENT_STABLE, &clp->cl_flags))
> +		return;
> +
>  	hexid = bin_to_hex_dup(clp->cl_name.data, clp->cl_name.len);
>  	if (!hexid) {
>  		dprintk("%s: can't allocate memory for upcall!\n", __func__);
> @@ -1311,7 +1315,9 @@ nfsd4_umh_cltrack_remove(struct nfs4_client *clp)
>  	}
>  
>  	nfsd4_cltrack_upcall_lock(clp);
> -	nfsd4_umh_cltrack_upcall("remove", hexid, NULL, NULL);
> +	if (test_bit(NFSD4_CLIENT_STABLE, &clp->cl_flags) &&
> +	    nfsd4_umh_cltrack_upcall("remove", hexid, NULL, NULL) == 0)
> +		clear_bit(NFSD4_CLIENT_STABLE, &clp->cl_flags);
>  	nfsd4_cltrack_upcall_unlock(clp);
>  
>  	kfree(hexid);
> @@ -1323,6 +1329,9 @@ nfsd4_umh_cltrack_check(struct nfs4_client *clp)
>  	int ret;
>  	char *hexid, *legacy;
>  
> +	if (test_bit(NFSD4_CLIENT_STABLE, &clp->cl_flags))
> +		return 0;
> +
>  	hexid = bin_to_hex_dup(clp->cl_name.data, clp->cl_name.len);
>  	if (!hexid) {
>  		dprintk("%s: can't allocate memory for upcall!\n", __func__);
> @@ -1331,9 +1340,14 @@ nfsd4_umh_cltrack_check(struct nfs4_client *clp)
>  	legacy = nfsd4_cltrack_legacy_recdir(&clp->cl_name);
>  
>  	nfsd4_cltrack_upcall_lock(clp);
> -	ret = nfsd4_umh_cltrack_upcall("check", hexid, legacy, NULL);
> +	if (test_bit(NFSD4_CLIENT_STABLE, &clp->cl_flags)) {
> +		ret = 0;
> +	} else {
> +		ret = nfsd4_umh_cltrack_upcall("check", hexid, legacy, NULL);
> +		if (!ret)
> +			set_bit(NFSD4_CLIENT_STABLE, &clp->cl_flags);
> +	}
>  	nfsd4_cltrack_upcall_unlock(clp);
> -
>  	kfree(legacy);
>  	kfree(hexid);
>  	return ret;
> -- 
> 1.9.3
>