[PATCH] lockd: eliminate duplicate calls to nlmsvc_lookup_host in nlmsvc_lock and nlmsvc_testlock

[PATCH] lockd: eliminate duplicate calls to nlmsvc_lookup_host in nlmsvc_lock and nlmsvc_testlock

[Jeff Layton]

Both nlmsvc_lock and nlmsvc_testlock call nlmsvc_lookup_host to find a
nlm_host struct. All the callers of these functions, however, call
nlmsvc_retrieve_args or nlm4svc_retrieve_args which also return a
nlm_host struct.

Change nlmsvc_lock and nlmsvc_testlock to take a host arg instead of
calling nlmsvc_lookup_host themselves and change the callers to pass
a pointer to the nlm_host they've already found.

Also, get rid of some unneeded nlm_host NULL checks.

I think I've got the reference counting right with this patch, but
I wouldn't mind someone sanity checking it.

Signed-off-by: Jeff Layton <jlayton@redhat.com>
---
 fs/lockd/svc4proc.c         |    7 +++----
 fs/lockd/svclock.c          |   26 +++++++-------------------
 fs/lockd/svcproc.c          |    7 +++----
 include/linux/lockd/lockd.h |    6 ++++--
 4 files changed, 17 insertions(+), 29 deletions(-)

diff --git a/fs/lockd/svc4proc.c b/fs/lockd/svc4proc.c
index 2e27176..3994446 100644
--- a/fs/lockd/svc4proc.c
+++ b/fs/lockd/svc4proc.c
@@ -58,8 +58,7 @@ nlm4svc_retrieve_args(struct svc_rqst *rqstp, struct nlm_args *argp,
 	return 0;
 
 no_locks:
-	if (host)
-		nlm_release_host(host);
+	nlm_release_host(host);
  	if (error)
 		return error;	
 	return nlm_lck_denied_nolocks;
@@ -100,7 +99,7 @@ nlm4svc_proc_test(struct svc_rqst *rqstp, struct nlm_args *argp,
 		return resp->status == nlm_drop_reply ? rpc_drop_reply :rpc_success;
 
 	/* Now check for conflicting locks */
-	resp->status = nlmsvc_testlock(rqstp, file, &argp->lock, &resp->lock, &resp->cookie);
+	resp->status = nlmsvc_testlock(rqstp, file, host, &argp->lock, &resp->lock, &resp->cookie);
 	if (resp->status == nlm_drop_reply)
 		rc = rpc_drop_reply;
 	else
@@ -146,7 +145,7 @@ nlm4svc_proc_lock(struct svc_rqst *rqstp, struct nlm_args *argp,
 #endif
 
 	/* Now try to lock the file */
-	resp->status = nlmsvc_lock(rqstp, file, &argp->lock,
+	resp->status = nlmsvc_lock(rqstp, file, host, &argp->lock,
 					argp->block, &argp->cookie);
 	if (resp->status == nlm_drop_reply)
 		rc = rpc_drop_reply;
diff --git a/fs/lockd/svclock.c b/fs/lockd/svclock.c
index 56a08ab..6680715 100644
--- a/fs/lockd/svclock.c
+++ b/fs/lockd/svclock.c
@@ -358,10 +358,10 @@ nlmsvc_defer_lock_rqst(struct svc_rqst *rqstp, struct nlm_block *block)
  */
 __be32
 nlmsvc_lock(struct svc_rqst *rqstp, struct nlm_file *file,
-			struct nlm_lock *lock, int wait, struct nlm_cookie *cookie)
+	    struct nlm_host *host, struct nlm_lock *lock, int wait,
+	    struct nlm_cookie *cookie)
 {
 	struct nlm_block	*block = NULL;
-	struct nlm_host		*host;
 	int			error;
 	__be32			ret;
 
@@ -373,11 +373,6 @@ nlmsvc_lock(struct svc_rqst *rqstp, struct nlm_file *file,
 				(long long)lock->fl.fl_end,
 				wait);
 
-	/* Create host handle for callback */
-	host = nlmsvc_lookup_host(rqstp, lock->caller, lock->len);
-	if (host == NULL)
-		return nlm_lck_denied_nolocks;
-
 	/* Lock file against concurrent access */
 	mutex_lock(&file->f_mutex);
 	/* Get existing block (in case client is busy-waiting)
@@ -386,7 +381,7 @@ nlmsvc_lock(struct svc_rqst *rqstp, struct nlm_file *file,
 	block = nlmsvc_lookup_block(file, lock);
 	if (block == NULL) {
 		block = nlmsvc_create_block(rqstp, nlm_get_host(host), file,
-				lock, cookie);
+					    lock, cookie);
 		ret = nlm_lck_denied_nolocks;
 		if (block == NULL)
 			goto out;
@@ -450,7 +445,6 @@ nlmsvc_lock(struct svc_rqst *rqstp, struct nlm_file *file,
 out:
 	mutex_unlock(&file->f_mutex);
 	nlmsvc_release_block(block);
-	nlm_release_host(host);
 	dprintk("lockd: nlmsvc_lock returned %u\n", ret);
 	return ret;
 }
@@ -460,8 +454,8 @@ out:
  */
 __be32
 nlmsvc_testlock(struct svc_rqst *rqstp, struct nlm_file *file,
-		struct nlm_lock *lock, struct nlm_lock *conflock,
-		struct nlm_cookie *cookie)
+		struct nlm_host *host, struct nlm_lock *lock,
+		struct nlm_lock *conflock, struct nlm_cookie *cookie)
 {
 	struct nlm_block 	*block = NULL;
 	int			error;
@@ -479,17 +473,11 @@ nlmsvc_testlock(struct svc_rqst *rqstp, struct nlm_file *file,
 
 	if (block == NULL) {
 		struct file_lock *conf = kzalloc(sizeof(*conf), GFP_KERNEL);
-		struct nlm_host	*host;
 
 		if (conf == NULL)
 			return nlm_granted;
-		/* Create host handle for callback */
-		host = nlmsvc_lookup_host(rqstp, lock->caller, lock->len);
-		if (host == NULL) {
-			kfree(conf);
-			return nlm_lck_denied_nolocks;
-		}
-		block = nlmsvc_create_block(rqstp, host, file, lock, cookie);
+		block = nlmsvc_create_block(rqstp, nlm_get_host(host), file,
+					    lock, cookie);
 		if (block == NULL) {
 			kfree(conf);
 			return nlm_granted;
diff --git a/fs/lockd/svcproc.c b/fs/lockd/svcproc.c
index ce6952b..76019d2 100644
--- a/fs/lockd/svcproc.c
+++ b/fs/lockd/svcproc.c
@@ -87,8 +87,7 @@ nlmsvc_retrieve_args(struct svc_rqst *rqstp, struct nlm_args *argp,
 	return 0;
 
 no_locks:
-	if (host)
-		nlm_release_host(host);
+	nlm_release_host(host);
 	if (error)
 		return error;
 	return nlm_lck_denied_nolocks;
@@ -129,7 +128,7 @@ nlmsvc_proc_test(struct svc_rqst *rqstp, struct nlm_args *argp,
 		return resp->status == nlm_drop_reply ? rpc_drop_reply :rpc_success;
 
 	/* Now check for conflicting locks */
-	resp->status = cast_status(nlmsvc_testlock(rqstp, file, &argp->lock, &resp->lock, &resp->cookie));
+	resp->status = cast_status(nlmsvc_testlock(rqstp, file, host, &argp->lock, &resp->lock, &resp->cookie));
 	if (resp->status == nlm_drop_reply)
 		rc = rpc_drop_reply;
 	else
@@ -176,7 +175,7 @@ nlmsvc_proc_lock(struct svc_rqst *rqstp, struct nlm_args *argp,
 #endif
 
 	/* Now try to lock the file */
-	resp->status = cast_status(nlmsvc_lock(rqstp, file, &argp->lock,
+	resp->status = cast_status(nlmsvc_lock(rqstp, file, host, &argp->lock,
 					       argp->block, &argp->cookie));
 	if (resp->status == nlm_drop_reply)
 		rc = rpc_drop_reply;
diff --git a/include/linux/lockd/lockd.h b/include/linux/lockd/lockd.h
index 102d928..f81f9dd 100644
--- a/include/linux/lockd/lockd.h
+++ b/include/linux/lockd/lockd.h
@@ -200,10 +200,12 @@ typedef int	  (*nlm_host_match_fn_t)(void *cur, struct nlm_host *ref);
  * Server-side lock handling
  */
 __be32		  nlmsvc_lock(struct svc_rqst *, struct nlm_file *,
-					struct nlm_lock *, int, struct nlm_cookie *);
+			      struct nlm_host *, struct nlm_lock *, int,
+			      struct nlm_cookie *);
 __be32		  nlmsvc_unlock(struct nlm_file *, struct nlm_lock *);
 __be32		  nlmsvc_testlock(struct svc_rqst *, struct nlm_file *,
-			struct nlm_lock *, struct nlm_lock *, struct nlm_cookie *);
+			struct nlm_host *, struct nlm_lock *,
+			struct nlm_lock *, struct nlm_cookie *);
 __be32		  nlmsvc_cancel_blocked(struct nlm_file *, struct nlm_lock *);
 unsigned long	  nlmsvc_retry_blocked(void);
 void		  nlmsvc_traverse_blocks(struct nlm_host *, struct nlm_file *,
-- 
1.5.5.1

Re: [PATCH] lockd: eliminate duplicate calls to nlmsvc_lookup_host in nlmsvc_lock and nlmsvc_testlock

[J. Bruce Fields]

On Sat, Jul 12, 2008 at 09:17:10AM -0400, Jeff Layton wrote:
> Both nlmsvc_lock and nlmsvc_testlock call nlmsvc_lookup_host to find a
> nlm_host struct. All the callers of these functions, however, call
> nlmsvc_retrieve_args or nlm4svc_retrieve_args which also return a
> nlm_host struct.
> 
> Change nlmsvc_lock and nlmsvc_testlock to take a host arg instead of
> calling nlmsvc_lookup_host themselves and change the callers to pass
> a pointer to the nlm_host they've already found.

That makes sense to me, thanks.

> Also, get rid of some unneeded nlm_host NULL checks.
> 
> I think I've got the reference counting right with this patch, but
> I wouldn't mind someone sanity checking it.

So, the way I do that sanity-checking is by breaking up the individual
changes as finely as I can, as follows....

--b.

[PATCH 1/4] lockd: nlm_release_host() checks for NULL, caller needn't

[J. Bruce Fields]

From: Jeff Layton <jlayton@redhat.com>

No need to check for a NULL argument twice.

Signed-off-by: Jeff Layton <jlayton@redhat.com>
Signed-off-by: J. Bruce Fields <bfields@citi.umich.edu>
---
 fs/lockd/svc4proc.c |    3 +--
 fs/lockd/svcproc.c  |    3 +--
 2 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/fs/lockd/svc4proc.c b/fs/lockd/svc4proc.c
index 385437e..006a832 100644
--- a/fs/lockd/svc4proc.c
+++ b/fs/lockd/svc4proc.c
@@ -58,8 +58,7 @@ nlm4svc_retrieve_args(struct svc_rqst *rqstp, struct nlm_args *argp,
 	return 0;
 
 no_locks:
-	if (host)
-		nlm_release_host(host);
+	nlm_release_host(host);
  	if (error)
 		return error;	
 	return nlm_lck_denied_nolocks;
diff --git a/fs/lockd/svcproc.c b/fs/lockd/svcproc.c
index 88379cc..fce3d70 100644
--- a/fs/lockd/svcproc.c
+++ b/fs/lockd/svcproc.c
@@ -87,8 +87,7 @@ nlmsvc_retrieve_args(struct svc_rqst *rqstp, struct nlm_args *argp,
 	return 0;
 
 no_locks:
-	if (host)
-		nlm_release_host(host);
+	nlm_release_host(host);
 	if (error)
 		return error;
 	return nlm_lck_denied_nolocks;
-- 
1.5.5.rc1

[PATCH 2/4] lockd: eliminate duplicate nlmsvc_lookup_host call from nlmsvc_testlock

[J. Bruce Fields]

From: Jeff Layton <jlayton@redhat.com>

nlmsvc_testlock calls nlmsvc_lookup_host to find a nlm_host struct. The
callers of this functions, however, call nlmsvc_retrieve_args or
nlm4svc_retrieve_args, which also return a nlm_host struct.

Change nlmsvc_testlock to take a host arg instead of calling
nlmsvc_lookup_host itself and change the callers to pass a pointer to
the nlm_host they've already found.

We take a reference to host in the place where nlmsvc_testlock()
previous did a new lookup, so the reference counting is unchanged from
before.

Signed-off-by: Jeff Layton <jlayton@redhat.com>
Signed-off-by: J. Bruce Fields <bfields@citi.umich.edu>
---
 fs/lockd/svc4proc.c         |    2 +-
 fs/lockd/svclock.c          |   12 +++---------
 fs/lockd/svcproc.c          |    2 +-
 include/linux/lockd/lockd.h |    3 ++-
 4 files changed, 7 insertions(+), 12 deletions(-)

diff --git a/fs/lockd/svc4proc.c b/fs/lockd/svc4proc.c
index 006a832..8cfb9da 100644
--- a/fs/lockd/svc4proc.c
+++ b/fs/lockd/svc4proc.c
@@ -99,7 +99,7 @@ nlm4svc_proc_test(struct svc_rqst *rqstp, struct nlm_args *argp,
 		return resp->status == nlm_drop_reply ? rpc_drop_reply :rpc_success;
 
 	/* Now check for conflicting locks */
-	resp->status = nlmsvc_testlock(rqstp, file, &argp->lock, &resp->lock, &resp->cookie);
+	resp->status = nlmsvc_testlock(rqstp, file, host, &argp->lock, &resp->lock, &resp->cookie);
 	if (resp->status == nlm_drop_reply)
 		rc = rpc_drop_reply;
 	else
diff --git a/fs/lockd/svclock.c b/fs/lockd/svclock.c
index 81aca85..f40afb3 100644
--- a/fs/lockd/svclock.c
+++ b/fs/lockd/svclock.c
@@ -460,8 +460,8 @@ out:
  */
 __be32
 nlmsvc_testlock(struct svc_rqst *rqstp, struct nlm_file *file,
-		struct nlm_lock *lock, struct nlm_lock *conflock,
-		struct nlm_cookie *cookie)
+		struct nlm_host *host, struct nlm_lock *lock,
+		struct nlm_lock *conflock, struct nlm_cookie *cookie)
 {
 	struct nlm_block 	*block = NULL;
 	int			error;
@@ -479,16 +479,10 @@ nlmsvc_testlock(struct svc_rqst *rqstp, struct nlm_file *file,
 
 	if (block == NULL) {
 		struct file_lock *conf = kzalloc(sizeof(*conf), GFP_KERNEL);
-		struct nlm_host	*host;
 
 		if (conf == NULL)
 			return nlm_granted;
-		/* Create host handle for callback */
-		host = nlmsvc_lookup_host(rqstp, lock->caller, lock->len);
-		if (host == NULL) {
-			kfree(conf);
-			return nlm_lck_denied_nolocks;
-		}
+		nlm_get_host(host);
 		block = nlmsvc_create_block(rqstp, host, file, lock, cookie);
 		if (block == NULL) {
 			kfree(conf);
diff --git a/fs/lockd/svcproc.c b/fs/lockd/svcproc.c
index fce3d70..e099f58 100644
--- a/fs/lockd/svcproc.c
+++ b/fs/lockd/svcproc.c
@@ -128,7 +128,7 @@ nlmsvc_proc_test(struct svc_rqst *rqstp, struct nlm_args *argp,
 		return resp->status == nlm_drop_reply ? rpc_drop_reply :rpc_success;
 
 	/* Now check for conflicting locks */
-	resp->status = cast_status(nlmsvc_testlock(rqstp, file, &argp->lock, &resp->lock, &resp->cookie));
+	resp->status = cast_status(nlmsvc_testlock(rqstp, file, host, &argp->lock, &resp->lock, &resp->cookie));
 	if (resp->status == nlm_drop_reply)
 		rc = rpc_drop_reply;
 	else
diff --git a/include/linux/lockd/lockd.h b/include/linux/lockd/lockd.h
index 102d928..b279670 100644
--- a/include/linux/lockd/lockd.h
+++ b/include/linux/lockd/lockd.h
@@ -203,7 +203,8 @@ __be32		  nlmsvc_lock(struct svc_rqst *, struct nlm_file *,
 					struct nlm_lock *, int, struct nlm_cookie *);
 __be32		  nlmsvc_unlock(struct nlm_file *, struct nlm_lock *);
 __be32		  nlmsvc_testlock(struct svc_rqst *, struct nlm_file *,
-			struct nlm_lock *, struct nlm_lock *, struct nlm_cookie *);
+			struct nlm_host *, struct nlm_lock *,
+			struct nlm_lock *, struct nlm_cookie *);
 __be32		  nlmsvc_cancel_blocked(struct nlm_file *, struct nlm_lock *);
 unsigned long	  nlmsvc_retry_blocked(void);
 void		  nlmsvc_traverse_blocks(struct nlm_host *, struct nlm_file *,
-- 
1.5.5.rc1

[PATCH 3/4] lockd: eliminate duplicate nlmsvc_lookup_host call from nlmsvc_lock

[J. Bruce Fields]

From: Jeff Layton <jlayton@redhat.com>

nlmsvc_lock calls nlmsvc_lookup_host to find a nlm_host struct. The
callers of this function, however, call nlmsvc_retrieve_args or
nlm4svc_retrieve_args, which also return a nlm_host struct.

Change nlmsvc_lock to take a host arg instead of calling
nlmsvc_lookup_host itself and change the callers to pass a pointer to
the nlm_host they've already found.

Since nlmsvc_testlock() now just uses the caller's reference, we no
longer need to get or release it.

Signed-off-by: Jeff Layton <jlayton@redhat.com>
Signed-off-by: J. Bruce Fields <bfields@citi.umich.edu>
---
 fs/lockd/svc4proc.c         |    2 +-
 fs/lockd/svclock.c          |   12 +++---------
 fs/lockd/svcproc.c          |    2 +-
 include/linux/lockd/lockd.h |    3 ++-
 4 files changed, 7 insertions(+), 12 deletions(-)

diff --git a/fs/lockd/svc4proc.c b/fs/lockd/svc4proc.c
index 8cfb9da..189b2ce 100644
--- a/fs/lockd/svc4proc.c
+++ b/fs/lockd/svc4proc.c
@@ -145,7 +145,7 @@ nlm4svc_proc_lock(struct svc_rqst *rqstp, struct nlm_args *argp,
 #endif
 
 	/* Now try to lock the file */
-	resp->status = nlmsvc_lock(rqstp, file, &argp->lock,
+	resp->status = nlmsvc_lock(rqstp, file, host, &argp->lock,
 					argp->block, &argp->cookie);
 	if (resp->status == nlm_drop_reply)
 		rc = rpc_drop_reply;
diff --git a/fs/lockd/svclock.c b/fs/lockd/svclock.c
index f40afb3..a6d3ed0 100644
--- a/fs/lockd/svclock.c
+++ b/fs/lockd/svclock.c
@@ -358,10 +358,10 @@ nlmsvc_defer_lock_rqst(struct svc_rqst *rqstp, struct nlm_block *block)
  */
 __be32
 nlmsvc_lock(struct svc_rqst *rqstp, struct nlm_file *file,
-			struct nlm_lock *lock, int wait, struct nlm_cookie *cookie)
+	    struct nlm_host *host, struct nlm_lock *lock, int wait,
+	    struct nlm_cookie *cookie)
 {
 	struct nlm_block	*block = NULL;
-	struct nlm_host		*host;
 	int			error;
 	__be32			ret;
 
@@ -373,11 +373,6 @@ nlmsvc_lock(struct svc_rqst *rqstp, struct nlm_file *file,
 				(long long)lock->fl.fl_end,
 				wait);
 
-	/* Create host handle for callback */
-	host = nlmsvc_lookup_host(rqstp, lock->caller, lock->len);
-	if (host == NULL)
-		return nlm_lck_denied_nolocks;
-
 	/* Lock file against concurrent access */
 	mutex_lock(&file->f_mutex);
 	/* Get existing block (in case client is busy-waiting)
@@ -450,7 +445,6 @@ nlmsvc_lock(struct svc_rqst *rqstp, struct nlm_file *file,
 out:
 	mutex_unlock(&file->f_mutex);
 	nlmsvc_release_block(block);
-	nlm_release_host(host);
 	dprintk("lockd: nlmsvc_lock returned %u\n", ret);
 	return ret;
 }
@@ -483,7 +477,7 @@ nlmsvc_testlock(struct svc_rqst *rqstp, struct nlm_file *file,
 		if (conf == NULL)
 			return nlm_granted;
 		nlm_get_host(host);
-		block = nlmsvc_create_block(rqstp, host, file, lock, cookie);
+		block = nlmsvc_create_block(rqstp, file, lock, cookie);
 		if (block == NULL) {
 			kfree(conf);
 			return nlm_granted;
diff --git a/fs/lockd/svcproc.c b/fs/lockd/svcproc.c
index e099f58..82dc908 100644
--- a/fs/lockd/svcproc.c
+++ b/fs/lockd/svcproc.c
@@ -175,7 +175,7 @@ nlmsvc_proc_lock(struct svc_rqst *rqstp, struct nlm_args *argp,
 #endif
 
 	/* Now try to lock the file */
-	resp->status = cast_status(nlmsvc_lock(rqstp, file, &argp->lock,
+	resp->status = cast_status(nlmsvc_lock(rqstp, file, host, &argp->lock,
 					       argp->block, &argp->cookie));
 	if (resp->status == nlm_drop_reply)
 		rc = rpc_drop_reply;
diff --git a/include/linux/lockd/lockd.h b/include/linux/lockd/lockd.h
index b279670..f81f9dd 100644
--- a/include/linux/lockd/lockd.h
+++ b/include/linux/lockd/lockd.h
@@ -200,7 +200,8 @@ typedef int	  (*nlm_host_match_fn_t)(void *cur, struct nlm_host *ref);
  * Server-side lock handling
  */
 __be32		  nlmsvc_lock(struct svc_rqst *, struct nlm_file *,
-					struct nlm_lock *, int, struct nlm_cookie *);
+			      struct nlm_host *, struct nlm_lock *, int,
+			      struct nlm_cookie *);
 __be32		  nlmsvc_unlock(struct nlm_file *, struct nlm_lock *);
 __be32		  nlmsvc_testlock(struct svc_rqst *, struct nlm_file *,
 			struct nlm_host *, struct nlm_lock *,
-- 
1.5.5.rc1

[PATCH 4/4] lockd: minor svclock.c style fixes

[J. Bruce Fields]

Signed-off-by: J. Bruce Fields <bfields@citi.umich.edu>
---
 fs/lockd/svclock.c |   10 +++++-----
 1 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/fs/lockd/svclock.c b/fs/lockd/svclock.c
index a6d3ed0..926055c 100644
--- a/fs/lockd/svclock.c
+++ b/fs/lockd/svclock.c
@@ -129,9 +129,9 @@ nlmsvc_lookup_block(struct nlm_file *file, struct nlm_lock *lock)
 
 static inline int nlm_cookie_match(struct nlm_cookie *a, struct nlm_cookie *b)
 {
-	if(a->len != b->len)
+	if (a->len != b->len)
 		return 0;
-	if(memcmp(a->data,b->data,a->len))
+	if (memcmp(a->data, b->data,a->len))
 		return 0;
 	return 1;
 }
@@ -381,7 +381,7 @@ nlmsvc_lock(struct svc_rqst *rqstp, struct nlm_file *file,
 	block = nlmsvc_lookup_block(file, lock);
 	if (block == NULL) {
 		block = nlmsvc_create_block(rqstp, nlm_get_host(host), file,
-				lock, cookie);
+					    lock, cookie);
 		ret = nlm_lck_denied_nolocks;
 		if (block == NULL)
 			goto out;
@@ -412,7 +412,7 @@ nlmsvc_lock(struct svc_rqst *rqstp, struct nlm_file *file,
 	lock->fl.fl_flags &= ~FL_SLEEP;
 
 	dprintk("lockd: vfs_lock_file returned %d\n", error);
-	switch(error) {
+	switch (error) {
 		case 0:
 			ret = nlm_granted;
 			goto out;
@@ -880,7 +880,7 @@ nlmsvc_retry_blocked(void)
 
 		if (block->b_when == NLM_NEVER)
 			break;
-	        if (time_after(block->b_when,jiffies)) {
+		if (time_after(block->b_when, jiffies)) {
 			timeout = block->b_when - jiffies;
 			break;
 		}
-- 
1.5.5.rc1

Re: [PATCH 3/4] lockd: eliminate duplicate nlmsvc_lookup_host call from nlmsvc_lock

[J. Bruce Fields]

On Tue, Jul 15, 2008 at 02:48:12PM -0400, J. Bruce Fields wrote:
> From: Jeff Layton <jlayton@redhat.com>
> 
> nlmsvc_lock calls nlmsvc_lookup_host to find a nlm_host struct. The
> callers of this function, however, call nlmsvc_retrieve_args or
> nlm4svc_retrieve_args, which also return a nlm_host struct.
> 
> Change nlmsvc_lock to take a host arg instead of calling
> nlmsvc_lookup_host itself and change the callers to pass a pointer to
> the nlm_host they've already found.
> 
> Since nlmsvc_testlock() now just uses the caller's reference, we no
> longer need to get or release it.
> 
> Signed-off-by: Jeff Layton <jlayton@redhat.com>
> Signed-off-by: J. Bruce Fields <bfields@citi.umich.edu>
> ---
>  fs/lockd/svc4proc.c         |    2 +-
>  fs/lockd/svclock.c          |   12 +++---------
>  fs/lockd/svcproc.c          |    2 +-
>  include/linux/lockd/lockd.h |    3 ++-
>  4 files changed, 7 insertions(+), 12 deletions(-)
> 
> diff --git a/fs/lockd/svc4proc.c b/fs/lockd/svc4proc.c
> index 8cfb9da..189b2ce 100644
> --- a/fs/lockd/svc4proc.c
> +++ b/fs/lockd/svc4proc.c
> @@ -145,7 +145,7 @@ nlm4svc_proc_lock(struct svc_rqst *rqstp, struct nlm_args *argp,
>  #endif
>  
>  	/* Now try to lock the file */
> -	resp->status = nlmsvc_lock(rqstp, file, &argp->lock,
> +	resp->status = nlmsvc_lock(rqstp, file, host, &argp->lock,
>  					argp->block, &argp->cookie);
>  	if (resp->status == nlm_drop_reply)
>  		rc = rpc_drop_reply;
> diff --git a/fs/lockd/svclock.c b/fs/lockd/svclock.c
> index f40afb3..a6d3ed0 100644
> --- a/fs/lockd/svclock.c
> +++ b/fs/lockd/svclock.c
> @@ -358,10 +358,10 @@ nlmsvc_defer_lock_rqst(struct svc_rqst *rqstp, struct nlm_block *block)
>   */
>  __be32
>  nlmsvc_lock(struct svc_rqst *rqstp, struct nlm_file *file,
> -			struct nlm_lock *lock, int wait, struct nlm_cookie *cookie)
> +	    struct nlm_host *host, struct nlm_lock *lock, int wait,
> +	    struct nlm_cookie *cookie)
>  {
>  	struct nlm_block	*block = NULL;
> -	struct nlm_host		*host;
>  	int			error;
>  	__be32			ret;
>  
> @@ -373,11 +373,6 @@ nlmsvc_lock(struct svc_rqst *rqstp, struct nlm_file *file,
>  				(long long)lock->fl.fl_end,
>  				wait);
>  
> -	/* Create host handle for callback */
> -	host = nlmsvc_lookup_host(rqstp, lock->caller, lock->len);
> -	if (host == NULL)
> -		return nlm_lck_denied_nolocks;
> -
>  	/* Lock file against concurrent access */
>  	mutex_lock(&file->f_mutex);
>  	/* Get existing block (in case client is busy-waiting)
> @@ -450,7 +445,6 @@ nlmsvc_lock(struct svc_rqst *rqstp, struct nlm_file *file,
>  out:
>  	mutex_unlock(&file->f_mutex);
>  	nlmsvc_release_block(block);
> -	nlm_release_host(host);
>  	dprintk("lockd: nlmsvc_lock returned %u\n", ret);
>  	return ret;
>  }
> @@ -483,7 +477,7 @@ nlmsvc_testlock(struct svc_rqst *rqstp, struct nlm_file *file,
>  		if (conf == NULL)
>  			return nlm_granted;
>  		nlm_get_host(host);
> -		block = nlmsvc_create_block(rqstp, host, file, lock, cookie);
> +		block = nlmsvc_create_block(rqstp, file, lock, cookie);
>  		if (block == NULL) {
>  			kfree(conf);
>  			return nlm_granted;

Oh, jeez, except ignore that chunk!  Version that actually compiles
pushed to

	git://linux-nfs.org/~bfields/linux.git

--b.

> diff --git a/fs/lockd/svcproc.c b/fs/lockd/svcproc.c
> index e099f58..82dc908 100644
> --- a/fs/lockd/svcproc.c
> +++ b/fs/lockd/svcproc.c
> @@ -175,7 +175,7 @@ nlmsvc_proc_lock(struct svc_rqst *rqstp, struct nlm_args *argp,
>  #endif
>  
>  	/* Now try to lock the file */
> -	resp->status = cast_status(nlmsvc_lock(rqstp, file, &argp->lock,
> +	resp->status = cast_status(nlmsvc_lock(rqstp, file, host, &argp->lock,
>  					       argp->block, &argp->cookie));
>  	if (resp->status == nlm_drop_reply)
>  		rc = rpc_drop_reply;
> diff --git a/include/linux/lockd/lockd.h b/include/linux/lockd/lockd.h
> index b279670..f81f9dd 100644
> --- a/include/linux/lockd/lockd.h
> +++ b/include/linux/lockd/lockd.h
> @@ -200,7 +200,8 @@ typedef int	  (*nlm_host_match_fn_t)(void *cur, struct nlm_host *ref);
>   * Server-side lock handling
>   */
>  __be32		  nlmsvc_lock(struct svc_rqst *, struct nlm_file *,
> -					struct nlm_lock *, int, struct nlm_cookie *);
> +			      struct nlm_host *, struct nlm_lock *, int,
> +			      struct nlm_cookie *);
>  __be32		  nlmsvc_unlock(struct nlm_file *, struct nlm_lock *);
>  __be32		  nlmsvc_testlock(struct svc_rqst *, struct nlm_file *,
>  			struct nlm_host *, struct nlm_lock *,
> -- 
> 1.5.5.rc1
> 

Re: [PATCH 2/4] lockd: eliminate duplicate nlmsvc_lookup_host call from nlmsvc_testlock

[J. Bruce Fields]

On Tue, Jul 15, 2008 at 02:48:11PM -0400, J. Bruce Fields wrote:
> From: Jeff Layton <jlayton@redhat.com>
> 
> nlmsvc_testlock calls nlmsvc_lookup_host to find a nlm_host struct. The
> callers of this functions, however, call nlmsvc_retrieve_args or
> nlm4svc_retrieve_args, which also return a nlm_host struct.
> 
> Change nlmsvc_testlock to take a host arg instead of calling
> nlmsvc_lookup_host itself and change the callers to pass a pointer to
> the nlm_host they've already found.
> 
> We take a reference to host in the place where nlmsvc_testlock()
> previous did a new lookup, so the reference counting is unchanged from
> before.
> 
> Signed-off-by: Jeff Layton <jlayton@redhat.com>
> Signed-off-by: J. Bruce Fields <bfields@citi.umich.edu>
> ---
>  fs/lockd/svc4proc.c         |    2 +-
>  fs/lockd/svclock.c          |   12 +++---------
>  fs/lockd/svcproc.c          |    2 +-
>  include/linux/lockd/lockd.h |    3 ++-
>  4 files changed, 7 insertions(+), 12 deletions(-)
> 
> diff --git a/fs/lockd/svc4proc.c b/fs/lockd/svc4proc.c
> index 006a832..8cfb9da 100644
> --- a/fs/lockd/svc4proc.c
> +++ b/fs/lockd/svc4proc.c
> @@ -99,7 +99,7 @@ nlm4svc_proc_test(struct svc_rqst *rqstp, struct nlm_args *argp,
>  		return resp->status == nlm_drop_reply ? rpc_drop_reply :rpc_success;
>  
>  	/* Now check for conflicting locks */
> -	resp->status = nlmsvc_testlock(rqstp, file, &argp->lock, &resp->lock, &resp->cookie);
> +	resp->status = nlmsvc_testlock(rqstp, file, host, &argp->lock, &resp->lock, &resp->cookie);
>  	if (resp->status == nlm_drop_reply)
>  		rc = rpc_drop_reply;
>  	else
> diff --git a/fs/lockd/svclock.c b/fs/lockd/svclock.c
> index 81aca85..f40afb3 100644
> --- a/fs/lockd/svclock.c
> +++ b/fs/lockd/svclock.c
> @@ -460,8 +460,8 @@ out:
>   */
>  __be32
>  nlmsvc_testlock(struct svc_rqst *rqstp, struct nlm_file *file,
> -		struct nlm_lock *lock, struct nlm_lock *conflock,
> -		struct nlm_cookie *cookie)
> +		struct nlm_host *host, struct nlm_lock *lock,
> +		struct nlm_lock *conflock, struct nlm_cookie *cookie)
>  {
>  	struct nlm_block 	*block = NULL;
>  	int			error;
> @@ -479,16 +479,10 @@ nlmsvc_testlock(struct svc_rqst *rqstp, struct nlm_file *file,
>  
>  	if (block == NULL) {
>  		struct file_lock *conf = kzalloc(sizeof(*conf), GFP_KERNEL);
> -		struct nlm_host	*host;
>  
>  		if (conf == NULL)
>  			return nlm_granted;
> -		/* Create host handle for callback */
> -		host = nlmsvc_lookup_host(rqstp, lock->caller, lock->len);
> -		if (host == NULL) {
> -			kfree(conf);
> -			return nlm_lck_denied_nolocks;
> -		}
> +		nlm_get_host(host);
>  		block = nlmsvc_create_block(rqstp, host, file, lock, cookie);
>  		if (block == NULL) {
>  			kfree(conf);

By the way, it'd seem clearer if nlmsvc_create_block() did the job of
taking whatever reference it needed on "host" itself.

Seems like you could do the same for nlm_alloc_host() as well.

--b.

commit cc8c1b0a6514c670b1ab568241210bbdbece7923
Author: J. Bruce Fields <bfields@citi.umich.edu>
Date:   Tue Jul 15 15:05:45 2008 -0400

    lockd: get host reference in nlmsvc_create_block() instead of callers
    
    As it is it looks like there's an obvious reference count leak until you
    track down the definition of nlm_alloc_host() and see that it's
    guaranteed to consume a reference, success or failure.
    
    Signed-off-by: J. Bruce Fields <bfields@citi.umich.edu>

diff --git a/fs/lockd/svclock.c b/fs/lockd/svclock.c
index 92c49d7..80d4f2e 100644
--- a/fs/lockd/svclock.c
+++ b/fs/lockd/svclock.c
@@ -180,6 +180,7 @@ nlmsvc_create_block(struct svc_rqst *rqstp, struct nlm_host *host,
 	struct nlm_block	*block;
 	struct nlm_rqst		*call = NULL;
 
+	nlm_get_host(host);
 	call = nlm_alloc_call(host);
 	if (call == NULL)
 		return NULL;
@@ -380,8 +381,7 @@ nlmsvc_lock(struct svc_rqst *rqstp, struct nlm_file *file,
 	 */
 	block = nlmsvc_lookup_block(file, lock);
 	if (block == NULL) {
-		block = nlmsvc_create_block(rqstp, nlm_get_host(host), file,
-					    lock, cookie);
+		block = nlmsvc_create_block(rqstp, host, file, lock, cookie);
 		ret = nlm_lck_denied_nolocks;
 		if (block == NULL)
 			goto out;
@@ -476,7 +476,6 @@ nlmsvc_testlock(struct svc_rqst *rqstp, struct nlm_file *file,
 
 		if (conf == NULL)
 			return nlm_granted;
-		nlm_get_host(host);
 		block = nlmsvc_create_block(rqstp, host, file, lock, cookie);
 		if (block == NULL) {
 			kfree(conf);

Re: [PATCH] lockd: eliminate duplicate calls to nlmsvc_lookup_host in nlmsvc_lock and nlmsvc_testlock

[Jeff Layton]

On Tue, 15 Jul 2008 14:45:54 -0400
"J. Bruce Fields" <bfields@fieldses.org> wrote:

> On Sat, Jul 12, 2008 at 09:17:10AM -0400, Jeff Layton wrote:
> > Both nlmsvc_lock and nlmsvc_testlock call nlmsvc_lookup_host to find a
> > nlm_host struct. All the callers of these functions, however, call
> > nlmsvc_retrieve_args or nlm4svc_retrieve_args which also return a
> > nlm_host struct.
> > 
> > Change nlmsvc_lock and nlmsvc_testlock to take a host arg instead of
> > calling nlmsvc_lookup_host themselves and change the callers to pass
> > a pointer to the nlm_host they've already found.
> 
> That makes sense to me, thanks.
> 
> > Also, get rid of some unneeded nlm_host NULL checks.
> > 
> > I think I've got the reference counting right with this patch, but
> > I wouldn't mind someone sanity checking it.
> 
> So, the way I do that sanity-checking is by breaking up the individual
> changes as finely as I can, as follows....
> 
> --b.

Thanks Bruce,
  The patchset in your for-2.6.27 branch looks fine to me.

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

Re: [PATCH 2/4] lockd: eliminate duplicate nlmsvc_lookup_host call from nlmsvc_testlock

[Jeff Layton]

On Tue, 15 Jul 2008 15:09:02 -0400
"J. Bruce Fields" <bfields@fieldses.org> wrote:

> On Tue, Jul 15, 2008 at 02:48:11PM -0400, J. Bruce Fields wrote:
> > From: Jeff Layton <jlayton@redhat.com>
> > 
> > nlmsvc_testlock calls nlmsvc_lookup_host to find a nlm_host struct. The
> > callers of this functions, however, call nlmsvc_retrieve_args or
> > nlm4svc_retrieve_args, which also return a nlm_host struct.
> > 
> > Change nlmsvc_testlock to take a host arg instead of calling
> > nlmsvc_lookup_host itself and change the callers to pass a pointer to
> > the nlm_host they've already found.
> > 
> > We take a reference to host in the place where nlmsvc_testlock()
> > previous did a new lookup, so the reference counting is unchanged from
> > before.
> > 
> > Signed-off-by: Jeff Layton <jlayton@redhat.com>
> > Signed-off-by: J. Bruce Fields <bfields@citi.umich.edu>
> > ---
> >  fs/lockd/svc4proc.c         |    2 +-
> >  fs/lockd/svclock.c          |   12 +++---------
> >  fs/lockd/svcproc.c          |    2 +-
> >  include/linux/lockd/lockd.h |    3 ++-
> >  4 files changed, 7 insertions(+), 12 deletions(-)
> > 
> > diff --git a/fs/lockd/svc4proc.c b/fs/lockd/svc4proc.c
> > index 006a832..8cfb9da 100644
> > --- a/fs/lockd/svc4proc.c
> > +++ b/fs/lockd/svc4proc.c
> > @@ -99,7 +99,7 @@ nlm4svc_proc_test(struct svc_rqst *rqstp, struct nlm_args *argp,
> >  		return resp->status == nlm_drop_reply ? rpc_drop_reply :rpc_success;
> >  
> >  	/* Now check for conflicting locks */
> > -	resp->status = nlmsvc_testlock(rqstp, file, &argp->lock, &resp->lock, &resp->cookie);
> > +	resp->status = nlmsvc_testlock(rqstp, file, host, &argp->lock, &resp->lock, &resp->cookie);
> >  	if (resp->status == nlm_drop_reply)
> >  		rc = rpc_drop_reply;
> >  	else
> > diff --git a/fs/lockd/svclock.c b/fs/lockd/svclock.c
> > index 81aca85..f40afb3 100644
> > --- a/fs/lockd/svclock.c
> > +++ b/fs/lockd/svclock.c
> > @@ -460,8 +460,8 @@ out:
> >   */
> >  __be32
> >  nlmsvc_testlock(struct svc_rqst *rqstp, struct nlm_file *file,
> > -		struct nlm_lock *lock, struct nlm_lock *conflock,
> > -		struct nlm_cookie *cookie)
> > +		struct nlm_host *host, struct nlm_lock *lock,
> > +		struct nlm_lock *conflock, struct nlm_cookie *cookie)
> >  {
> >  	struct nlm_block 	*block = NULL;
> >  	int			error;
> > @@ -479,16 +479,10 @@ nlmsvc_testlock(struct svc_rqst *rqstp, struct nlm_file *file,
> >  
> >  	if (block == NULL) {
> >  		struct file_lock *conf = kzalloc(sizeof(*conf), GFP_KERNEL);
> > -		struct nlm_host	*host;
> >  
> >  		if (conf == NULL)
> >  			return nlm_granted;
> > -		/* Create host handle for callback */
> > -		host = nlmsvc_lookup_host(rqstp, lock->caller, lock->len);
> > -		if (host == NULL) {
> > -			kfree(conf);
> > -			return nlm_lck_denied_nolocks;
> > -		}
> > +		nlm_get_host(host);
> >  		block = nlmsvc_create_block(rqstp, host, file, lock, cookie);
> >  		if (block == NULL) {
> >  			kfree(conf);
> 
> By the way, it'd seem clearer if nlmsvc_create_block() did the job of
> taking whatever reference it needed on "host" itself.
> 

That does seem like the best thing to do here...

> Seems like you could do the same for nlm_alloc_host() as well.
> 
> --b.
> 
> commit cc8c1b0a6514c670b1ab568241210bbdbece7923
> Author: J. Bruce Fields <bfields@citi.umich.edu>
> Date:   Tue Jul 15 15:05:45 2008 -0400
> 
>     lockd: get host reference in nlmsvc_create_block() instead of callers
>     
>     As it is it looks like there's an obvious reference count leak until you
>     track down the definition of nlm_alloc_host() and see that it's
>     guaranteed to consume a reference, success or failure.
>     

I'm not sure I follow this. I don't see an nlm_alloc_host(). Do you mean
nlm_alloc_call()? If so, then it looks like it should only consume a
reference on failure (when signaled).

>     Signed-off-by: J. Bruce Fields <bfields@citi.umich.edu>
> 
> diff --git a/fs/lockd/svclock.c b/fs/lockd/svclock.c
> index 92c49d7..80d4f2e 100644
> --- a/fs/lockd/svclock.c
> +++ b/fs/lockd/svclock.c
> @@ -180,6 +180,7 @@ nlmsvc_create_block(struct svc_rqst *rqstp, struct nlm_host *host,
>  	struct nlm_block	*block;
>  	struct nlm_rqst		*call = NULL;
>  
> +	nlm_get_host(host);
>  	call = nlm_alloc_call(host);
>  	if (call == NULL)
>  		return NULL;
> @@ -380,8 +381,7 @@ nlmsvc_lock(struct svc_rqst *rqstp, struct nlm_file *file,
>  	 */
>  	block = nlmsvc_lookup_block(file, lock);
>  	if (block == NULL) {
> -		block = nlmsvc_create_block(rqstp, nlm_get_host(host), file,
> -					    lock, cookie);
> +		block = nlmsvc_create_block(rqstp, host, file, lock, cookie);
>  		ret = nlm_lck_denied_nolocks;
>  		if (block == NULL)
>  			goto out;
> @@ -476,7 +476,6 @@ nlmsvc_testlock(struct svc_rqst *rqstp, struct nlm_file *file,
>  
>  		if (conf == NULL)
>  			return nlm_granted;
> -		nlm_get_host(host);
>  		block = nlmsvc_create_block(rqstp, host, file, lock, cookie);
>  		if (block == NULL) {
>  			kfree(conf);


-- 
Jeff Layton <jlayton@redhat.com>

Re: [PATCH 2/4] lockd: eliminate duplicate nlmsvc_lookup_host call from nlmsvc_testlock

[J. Bruce Fields]

On Tue, Jul 15, 2008 at 03:32:22PM -0400, Jeff Layton wrote:
> On Tue, 15 Jul 2008 15:09:02 -0400
> "J. Bruce Fields" <bfields@fieldses.org> wrote:
> 
> > On Tue, Jul 15, 2008 at 02:48:11PM -0400, J. Bruce Fields wrote:
> > > From: Jeff Layton <jlayton@redhat.com>
> > > 
> > > nlmsvc_testlock calls nlmsvc_lookup_host to find a nlm_host struct. The
> > > callers of this functions, however, call nlmsvc_retrieve_args or
> > > nlm4svc_retrieve_args, which also return a nlm_host struct.
> > > 
> > > Change nlmsvc_testlock to take a host arg instead of calling
> > > nlmsvc_lookup_host itself and change the callers to pass a pointer to
> > > the nlm_host they've already found.
> > > 
> > > We take a reference to host in the place where nlmsvc_testlock()
> > > previous did a new lookup, so the reference counting is unchanged from
> > > before.
> > > 
> > > Signed-off-by: Jeff Layton <jlayton@redhat.com>
> > > Signed-off-by: J. Bruce Fields <bfields@citi.umich.edu>
> > > ---
> > >  fs/lockd/svc4proc.c         |    2 +-
> > >  fs/lockd/svclock.c          |   12 +++---------
> > >  fs/lockd/svcproc.c          |    2 +-
> > >  include/linux/lockd/lockd.h |    3 ++-
> > >  4 files changed, 7 insertions(+), 12 deletions(-)
> > > 
> > > diff --git a/fs/lockd/svc4proc.c b/fs/lockd/svc4proc.c
> > > index 006a832..8cfb9da 100644
> > > --- a/fs/lockd/svc4proc.c
> > > +++ b/fs/lockd/svc4proc.c
> > > @@ -99,7 +99,7 @@ nlm4svc_proc_test(struct svc_rqst *rqstp, struct nlm_args *argp,
> > >  		return resp->status == nlm_drop_reply ? rpc_drop_reply :rpc_success;
> > >  
> > >  	/* Now check for conflicting locks */
> > > -	resp->status = nlmsvc_testlock(rqstp, file, &argp->lock, &resp->lock, &resp->cookie);
> > > +	resp->status = nlmsvc_testlock(rqstp, file, host, &argp->lock, &resp->lock, &resp->cookie);
> > >  	if (resp->status == nlm_drop_reply)
> > >  		rc = rpc_drop_reply;
> > >  	else
> > > diff --git a/fs/lockd/svclock.c b/fs/lockd/svclock.c
> > > index 81aca85..f40afb3 100644
> > > --- a/fs/lockd/svclock.c
> > > +++ b/fs/lockd/svclock.c
> > > @@ -460,8 +460,8 @@ out:
> > >   */
> > >  __be32
> > >  nlmsvc_testlock(struct svc_rqst *rqstp, struct nlm_file *file,
> > > -		struct nlm_lock *lock, struct nlm_lock *conflock,
> > > -		struct nlm_cookie *cookie)
> > > +		struct nlm_host *host, struct nlm_lock *lock,
> > > +		struct nlm_lock *conflock, struct nlm_cookie *cookie)
> > >  {
> > >  	struct nlm_block 	*block = NULL;
> > >  	int			error;
> > > @@ -479,16 +479,10 @@ nlmsvc_testlock(struct svc_rqst *rqstp, struct nlm_file *file,
> > >  
> > >  	if (block == NULL) {
> > >  		struct file_lock *conf = kzalloc(sizeof(*conf), GFP_KERNEL);
> > > -		struct nlm_host	*host;
> > >  
> > >  		if (conf == NULL)
> > >  			return nlm_granted;
> > > -		/* Create host handle for callback */
> > > -		host = nlmsvc_lookup_host(rqstp, lock->caller, lock->len);
> > > -		if (host == NULL) {
> > > -			kfree(conf);
> > > -			return nlm_lck_denied_nolocks;
> > > -		}
> > > +		nlm_get_host(host);
> > >  		block = nlmsvc_create_block(rqstp, host, file, lock, cookie);
> > >  		if (block == NULL) {
> > >  			kfree(conf);
> > 
> > By the way, it'd seem clearer if nlmsvc_create_block() did the job of
> > taking whatever reference it needed on "host" itself.
> > 
> 
> That does seem like the best thing to do here...
> 
> > Seems like you could do the same for nlm_alloc_host() as well.
> > 
> > --b.
> > 
> > commit cc8c1b0a6514c670b1ab568241210bbdbece7923
> > Author: J. Bruce Fields <bfields@citi.umich.edu>
> > Date:   Tue Jul 15 15:05:45 2008 -0400
> > 
> >     lockd: get host reference in nlmsvc_create_block() instead of callers
> >     
> >     As it is it looks like there's an obvious reference count leak until you
> >     track down the definition of nlm_alloc_host() and see that it's
> >     guaranteed to consume a reference, success or failure.
> >     
> 
> I'm not sure I follow this. I don't see an nlm_alloc_host().  Do you mean
> nlm_alloc_call()?

Whoops, yes.

> If so, then it looks like it should only consume a
> reference on failure (when signaled).

It only has an explicit nlm_release_host() in that case, but in the
other case it exits having stored a pointer in a_host which will
eventually have nlm_release_host() called on it.

So I'd say it has "consumed" (either stored, or thrown away) one
reference in either case.

--b.

> 
> >     Signed-off-by: J. Bruce Fields <bfields@citi.umich.edu>
> > 
> > diff --git a/fs/lockd/svclock.c b/fs/lockd/svclock.c
> > index 92c49d7..80d4f2e 100644
> > --- a/fs/lockd/svclock.c
> > +++ b/fs/lockd/svclock.c
> > @@ -180,6 +180,7 @@ nlmsvc_create_block(struct svc_rqst *rqstp, struct nlm_host *host,
> >  	struct nlm_block	*block;
> >  	struct nlm_rqst		*call = NULL;
> >  
> > +	nlm_get_host(host);
> >  	call = nlm_alloc_call(host);
> >  	if (call == NULL)
> >  		return NULL;
> > @@ -380,8 +381,7 @@ nlmsvc_lock(struct svc_rqst *rqstp, struct nlm_file *file,
> >  	 */
> >  	block = nlmsvc_lookup_block(file, lock);
> >  	if (block == NULL) {
> > -		block = nlmsvc_create_block(rqstp, nlm_get_host(host), file,
> > -					    lock, cookie);
> > +		block = nlmsvc_create_block(rqstp, host, file, lock, cookie);
> >  		ret = nlm_lck_denied_nolocks;
> >  		if (block == NULL)
> >  			goto out;
> > @@ -476,7 +476,6 @@ nlmsvc_testlock(struct svc_rqst *rqstp, struct nlm_file *file,
> >  
> >  		if (conf == NULL)
> >  			return nlm_granted;
> > -		nlm_get_host(host);
> >  		block = nlmsvc_create_block(rqstp, host, file, lock, cookie);
> >  		if (block == NULL) {
> >  			kfree(conf);
> 
> 
> -- 
> Jeff Layton <jlayton@redhat.com>