Linux-NFS Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH] Revert "lockd: Show pid of lockd for remote locks"
@ 2019-05-20 14:33 Benjamin Coddington
  2019-05-21 15:58 ` J . Bruce Fields
  0 siblings, 1 reply; 4+ messages in thread
From: Benjamin Coddington @ 2019-05-20 14:33 UTC (permalink / raw)
  To: J . Bruce Fields; +Cc: Xuewei Zhang, linux-nfs

This reverts most of commit b8eee0e90f97 ("lockd: Show pid of lockd for
remote locks"), which caused remote locks to not be differentiated between
remote processes for NLM.

We retain the fixup for setting the client's fl_pid to a negative value.

Fixes: b8eee0e90f97 ("lockd: Show pid of lockd for remote locks")
Cc: stable@vger.kernel.org

Signed-off-by: Benjamin Coddington <bcodding@redhat.com>
---
 fs/lockd/xdr.c  | 4 ++--
 fs/lockd/xdr4.c | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/fs/lockd/xdr.c b/fs/lockd/xdr.c
index 9846f7e95282..7147e4aebecc 100644
--- a/fs/lockd/xdr.c
+++ b/fs/lockd/xdr.c
@@ -127,7 +127,7 @@ nlm_decode_lock(__be32 *p, struct nlm_lock *lock)
 
 	locks_init_lock(fl);
 	fl->fl_owner = current->files;
-	fl->fl_pid   = current->tgid;
+	fl->fl_pid   = (pid_t)lock->svid;
 	fl->fl_flags = FL_POSIX;
 	fl->fl_type  = F_RDLCK;		/* as good as anything else */
 	start = ntohl(*p++);
@@ -269,7 +269,7 @@ nlmsvc_decode_shareargs(struct svc_rqst *rqstp, __be32 *p)
 	memset(lock, 0, sizeof(*lock));
 	locks_init_lock(&lock->fl);
 	lock->svid = ~(u32) 0;
-	lock->fl.fl_pid = current->tgid;
+	lock->fl.fl_pid = (pid_t)lock->svid;
 
 	if (!(p = nlm_decode_cookie(p, &argp->cookie))
 	 || !(p = xdr_decode_string_inplace(p, &lock->caller,
diff --git a/fs/lockd/xdr4.c b/fs/lockd/xdr4.c
index 70154f376695..7ed9edf9aed4 100644
--- a/fs/lockd/xdr4.c
+++ b/fs/lockd/xdr4.c
@@ -119,7 +119,7 @@ nlm4_decode_lock(__be32 *p, struct nlm_lock *lock)
 
 	locks_init_lock(fl);
 	fl->fl_owner = current->files;
-	fl->fl_pid   = current->tgid;
+	fl->fl_pid   = (pid_t)lock->svid;
 	fl->fl_flags = FL_POSIX;
 	fl->fl_type  = F_RDLCK;		/* as good as anything else */
 	p = xdr_decode_hyper(p, &start);
@@ -266,7 +266,7 @@ nlm4svc_decode_shareargs(struct svc_rqst *rqstp, __be32 *p)
 	memset(lock, 0, sizeof(*lock));
 	locks_init_lock(&lock->fl);
 	lock->svid = ~(u32) 0;
-	lock->fl.fl_pid = current->tgid;
+	lock->fl.fl_pid = (pid_t)lock->svid;
 
 	if (!(p = nlm4_decode_cookie(p, &argp->cookie))
 	 || !(p = xdr_decode_string_inplace(p, &lock->caller,
-- 
2.20.1


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

* Re: [PATCH] Revert "lockd: Show pid of lockd for remote locks"
  2019-05-20 14:33 [PATCH] Revert "lockd: Show pid of lockd for remote locks" Benjamin Coddington
@ 2019-05-21 15:58 ` J . Bruce Fields
  2019-05-21 20:45   ` Benjamin Coddington
  0 siblings, 1 reply; 4+ messages in thread
From: J . Bruce Fields @ 2019-05-21 15:58 UTC (permalink / raw)
  To: Benjamin Coddington; +Cc: Xuewei Zhang, linux-nfs

On Mon, May 20, 2019 at 10:33:07AM -0400, Benjamin Coddington wrote:
> This reverts most of commit b8eee0e90f97 ("lockd: Show pid of lockd for
> remote locks"), which caused remote locks to not be differentiated between
> remote processes for NLM.

To make sure I understand: I assume a client resolves conflicts among
its own processes before involving the server, so the server only needs
to resolve conflicts among nlm_hosts.  Is that right?  So the only
practical affect is the missing grant callbacks that Xuewei noticed?

Or is my assumption not true in general?  Or is it true only for some
client implementations?

--b.

> 
> We retain the fixup for setting the client's fl_pid to a negative value.
> 
> Fixes: b8eee0e90f97 ("lockd: Show pid of lockd for remote locks")
> Cc: stable@vger.kernel.org
> 
> Signed-off-by: Benjamin Coddington <bcodding@redhat.com>
> ---
>  fs/lockd/xdr.c  | 4 ++--
>  fs/lockd/xdr4.c | 4 ++--
>  2 files changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/fs/lockd/xdr.c b/fs/lockd/xdr.c
> index 9846f7e95282..7147e4aebecc 100644
> --- a/fs/lockd/xdr.c
> +++ b/fs/lockd/xdr.c
> @@ -127,7 +127,7 @@ nlm_decode_lock(__be32 *p, struct nlm_lock *lock)
>  
>  	locks_init_lock(fl);
>  	fl->fl_owner = current->files;
> -	fl->fl_pid   = current->tgid;
> +	fl->fl_pid   = (pid_t)lock->svid;
>  	fl->fl_flags = FL_POSIX;
>  	fl->fl_type  = F_RDLCK;		/* as good as anything else */
>  	start = ntohl(*p++);
> @@ -269,7 +269,7 @@ nlmsvc_decode_shareargs(struct svc_rqst *rqstp, __be32 *p)
>  	memset(lock, 0, sizeof(*lock));
>  	locks_init_lock(&lock->fl);
>  	lock->svid = ~(u32) 0;
> -	lock->fl.fl_pid = current->tgid;
> +	lock->fl.fl_pid = (pid_t)lock->svid;
>  
>  	if (!(p = nlm_decode_cookie(p, &argp->cookie))
>  	 || !(p = xdr_decode_string_inplace(p, &lock->caller,
> diff --git a/fs/lockd/xdr4.c b/fs/lockd/xdr4.c
> index 70154f376695..7ed9edf9aed4 100644
> --- a/fs/lockd/xdr4.c
> +++ b/fs/lockd/xdr4.c
> @@ -119,7 +119,7 @@ nlm4_decode_lock(__be32 *p, struct nlm_lock *lock)
>  
>  	locks_init_lock(fl);
>  	fl->fl_owner = current->files;
> -	fl->fl_pid   = current->tgid;
> +	fl->fl_pid   = (pid_t)lock->svid;
>  	fl->fl_flags = FL_POSIX;
>  	fl->fl_type  = F_RDLCK;		/* as good as anything else */
>  	p = xdr_decode_hyper(p, &start);
> @@ -266,7 +266,7 @@ nlm4svc_decode_shareargs(struct svc_rqst *rqstp, __be32 *p)
>  	memset(lock, 0, sizeof(*lock));
>  	locks_init_lock(&lock->fl);
>  	lock->svid = ~(u32) 0;
> -	lock->fl.fl_pid = current->tgid;
> +	lock->fl.fl_pid = (pid_t)lock->svid;
>  
>  	if (!(p = nlm4_decode_cookie(p, &argp->cookie))
>  	 || !(p = xdr_decode_string_inplace(p, &lock->caller,
> -- 
> 2.20.1

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

* Re: [PATCH] Revert "lockd: Show pid of lockd for remote locks"
  2019-05-21 15:58 ` J . Bruce Fields
@ 2019-05-21 20:45   ` Benjamin Coddington
  2019-05-21 22:33     ` Xuewei Zhang
  0 siblings, 1 reply; 4+ messages in thread
From: Benjamin Coddington @ 2019-05-21 20:45 UTC (permalink / raw)
  To: J . Bruce Fields; +Cc: Xuewei Zhang, linux-nfs

On 21 May 2019, at 11:58, J . Bruce Fields wrote:

> On Mon, May 20, 2019 at 10:33:07AM -0400, Benjamin Coddington wrote:
>> This reverts most of commit b8eee0e90f97 ("lockd: Show pid of lockd for
>> remote locks"), which caused remote locks to not be differentiated between
>> remote processes for NLM.
>
> To make sure I understand: I assume a client resolves conflicts among
> its own processes before involving the server, so the server only needs
> to resolve conflicts among nlm_hosts.  Is that right?  So the only
> practical affect is the missing grant callbacks that Xuewei noticed?

For the linux client, I think that is correct.

> Or is my assumption not true in general?  Or is it true only for some
> client implementations?

I don't know the answer here.  I imagine a client could leave off checking
for local lock conflicts.

I have a set of patches that stuff nlm_lockowner into fl_owner on the
server.  That allows us to set fl_pid of lockd and not lose the svid.  I am
just testing them thoroughly at this point, and I'll probably get them
posted tomorrow.

Ben

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

* Re: [PATCH] Revert "lockd: Show pid of lockd for remote locks"
  2019-05-21 20:45   ` Benjamin Coddington
@ 2019-05-21 22:33     ` Xuewei Zhang
  0 siblings, 0 replies; 4+ messages in thread
From: Xuewei Zhang @ 2019-05-21 22:33 UTC (permalink / raw)
  To: Benjamin Coddington; +Cc: J . Bruce Fields, linux-nfs

On Tue, May 21, 2019 at 1:45 PM Benjamin Coddington <bcodding@redhat.com> wrote:
>
> On 21 May 2019, at 11:58, J . Bruce Fields wrote:
>
> > On Mon, May 20, 2019 at 10:33:07AM -0400, Benjamin Coddington wrote:
> >> This reverts most of commit b8eee0e90f97 ("lockd: Show pid of lockd for
> >> remote locks"), which caused remote locks to not be differentiated between
> >> remote processes for NLM.

I just tested this today to be sure. And yes, this revert fixes the
problem we observed.

Reviewed-by: Xuewei Zhang <xueweiz@google.com>

Best regards,
Xuewei

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

end of thread, back to index

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-05-20 14:33 [PATCH] Revert "lockd: Show pid of lockd for remote locks" Benjamin Coddington
2019-05-21 15:58 ` J . Bruce Fields
2019-05-21 20:45   ` Benjamin Coddington
2019-05-21 22:33     ` Xuewei Zhang

Linux-NFS Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-nfs/0 linux-nfs/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-nfs linux-nfs/ https://lore.kernel.org/linux-nfs \
		linux-nfs@vger.kernel.org linux-nfs@archiver.kernel.org
	public-inbox-index linux-nfs


Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-nfs


AGPL code for this site: git clone https://public-inbox.org/ public-inbox