From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jeff Layton Subject: Re: [PATCH] fs/locks.c: Copy fl_lmops to conflock for nfsd using Date: Tue, 5 Aug 2014 15:20:54 -0400 Message-ID: <20140805152054.60e00162@tlielax.poochiereds.net> References: <53BAAAC5.9000106@gmail.com> <53DCF97D.3000605@gmail.com> <20140802190505.442f07b8@tlielax.poochiereds.net> <20140805191458.GV23341@fieldses.org> Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Cc: Kinglong Mee , Linux NFS Mailing List , NeilBrown , Trond Myklebust , linux-fsdevel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org To: "J. Bruce Fields" Return-path: In-Reply-To: <20140805191458.GV23341-uC3wQj2KruNg9hUCZPvPmw@public.gmane.org> Sender: linux-nfs-owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org List-Id: linux-fsdevel.vger.kernel.org On Tue, 5 Aug 2014 15:14:58 -0400 "J. Bruce Fields" wrote: > On Sat, Aug 02, 2014 at 07:05:05PM -0400, Jeff Layton wrote: > > On Sat, 02 Aug 2014 22:45:17 +0800 > > Kinglong Mee wrote: > > > > > Commit d5b9026a67 ([PATCH] knfsd: locks: flag NFSv4-owned locks) > > > using fl_lmops field in file_lock for checking nfsd4 lockowner. > > > > > > But, commit 1a747ee0cc (locks: don't call ->copy_lock methods > > > on return of conflicting locks) causes the fl_lmops of conflock > > > for nfsd4_lock always be NULL. > > > > > > Also, commit 0996905f93 (lockd: posix_test_lock() should not call > > > locks_copy_lock()) caused the fl_lmops of conflock for nfsd4_lockt > > > always be NULL too. > > > > > > So that, nfsd4 lockowner for it always be NULL too. > > > > > > This patch re-coping the fl_lmops to conflock. > > > > > > Signed-off-by: Kinglong Mee > > > --- > > > fs/locks.c | 3 +-- > > > 1 file changed, 1 insertion(+), 2 deletions(-) > > > > > > diff --git a/fs/locks.c b/fs/locks.c > > > index 717fbc4..cc1219a 100644 > > > --- a/fs/locks.c > > > +++ b/fs/locks.c > > > @@ -279,7 +279,7 @@ void __locks_copy_lock(struct file_lock *new, const struct file_lock *fl) > > > new->fl_start = fl->fl_start; > > > new->fl_end = fl->fl_end; > > > new->fl_ops = NULL; > > > - new->fl_lmops = NULL; > > > + new->fl_lmops = fl->fl_lmops; > > > } > > > EXPORT_SYMBOL(__locks_copy_lock); > > > > > > @@ -290,7 +290,6 @@ void locks_copy_lock(struct file_lock *new, struct file_lock *fl) > > > __locks_copy_lock(new, fl); > > > new->fl_file = fl->fl_file; > > > new->fl_ops = fl->fl_ops; > > > - new->fl_lmops = fl->fl_lmops; > > > > > > locks_copy_private(new, fl); > > > } > > > > This looks sane to me AFAICT. > > > > I'll run a few tests with it, and put it into I'll plan to pick this up > > since I have some other fs/locks.c related patches slated for 3.17. > > Looks like your mail and Trond's crossed? I'd need to go remind myself > of the __locks_copy_lock callers, but I'm pretty sure Trond's right.... > > --b. Yeah, I missed his earlier reply and have since dropped this patch. The potential race that Trond pointed out trumps the minor problem of not having conflock info, so I'm inclined to wait for a better solution to that problem. -- Jeff Layton -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: linux-nfs-owner@vger.kernel.org Received: from mail-qa0-f41.google.com ([209.85.216.41]:46679 "EHLO mail-qa0-f41.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753438AbaHETU6 (ORCPT ); Tue, 5 Aug 2014 15:20:58 -0400 Received: by mail-qa0-f41.google.com with SMTP id j7so1440153qaq.0 for ; Tue, 05 Aug 2014 12:20:57 -0700 (PDT) Date: Tue, 5 Aug 2014 15:20:54 -0400 From: Jeff Layton To: "J. Bruce Fields" Cc: Kinglong Mee , Linux NFS Mailing List , NeilBrown , Trond Myklebust , linux-fsdevel@vger.kernel.org Subject: Re: [PATCH] fs/locks.c: Copy fl_lmops to conflock for nfsd using Message-ID: <20140805152054.60e00162@tlielax.poochiereds.net> In-Reply-To: <20140805191458.GV23341@fieldses.org> References: <53BAAAC5.9000106@gmail.com> <53DCF97D.3000605@gmail.com> <20140802190505.442f07b8@tlielax.poochiereds.net> <20140805191458.GV23341@fieldses.org> MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Sender: linux-nfs-owner@vger.kernel.org List-ID: On Tue, 5 Aug 2014 15:14:58 -0400 "J. Bruce Fields" wrote: > On Sat, Aug 02, 2014 at 07:05:05PM -0400, Jeff Layton wrote: > > On Sat, 02 Aug 2014 22:45:17 +0800 > > Kinglong Mee wrote: > > > > > Commit d5b9026a67 ([PATCH] knfsd: locks: flag NFSv4-owned locks) > > > using fl_lmops field in file_lock for checking nfsd4 lockowner. > > > > > > But, commit 1a747ee0cc (locks: don't call ->copy_lock methods > > > on return of conflicting locks) causes the fl_lmops of conflock > > > for nfsd4_lock always be NULL. > > > > > > Also, commit 0996905f93 (lockd: posix_test_lock() should not call > > > locks_copy_lock()) caused the fl_lmops of conflock for nfsd4_lockt > > > always be NULL too. > > > > > > So that, nfsd4 lockowner for it always be NULL too. > > > > > > This patch re-coping the fl_lmops to conflock. > > > > > > Signed-off-by: Kinglong Mee > > > --- > > > fs/locks.c | 3 +-- > > > 1 file changed, 1 insertion(+), 2 deletions(-) > > > > > > diff --git a/fs/locks.c b/fs/locks.c > > > index 717fbc4..cc1219a 100644 > > > --- a/fs/locks.c > > > +++ b/fs/locks.c > > > @@ -279,7 +279,7 @@ void __locks_copy_lock(struct file_lock *new, const struct file_lock *fl) > > > new->fl_start = fl->fl_start; > > > new->fl_end = fl->fl_end; > > > new->fl_ops = NULL; > > > - new->fl_lmops = NULL; > > > + new->fl_lmops = fl->fl_lmops; > > > } > > > EXPORT_SYMBOL(__locks_copy_lock); > > > > > > @@ -290,7 +290,6 @@ void locks_copy_lock(struct file_lock *new, struct file_lock *fl) > > > __locks_copy_lock(new, fl); > > > new->fl_file = fl->fl_file; > > > new->fl_ops = fl->fl_ops; > > > - new->fl_lmops = fl->fl_lmops; > > > > > > locks_copy_private(new, fl); > > > } > > > > This looks sane to me AFAICT. > > > > I'll run a few tests with it, and put it into I'll plan to pick this up > > since I have some other fs/locks.c related patches slated for 3.17. > > Looks like your mail and Trond's crossed? I'd need to go remind myself > of the __locks_copy_lock callers, but I'm pretty sure Trond's right.... > > --b. Yeah, I missed his earlier reply and have since dropped this patch. The potential race that Trond pointed out trumps the minor problem of not having conflock info, so I'm inclined to wait for a better solution to that problem. -- Jeff Layton