[v2] cifs: allow unlock flock and OFD lock across fork
diff mbox series

Message ID 20200221023001.vcoc5f43rdqqeifn@xzhoux.usersys.redhat.com
State New
Headers show
Series
  • [v2] cifs: allow unlock flock and OFD lock across fork
Related show

Commit Message

Murphy Zhou Feb. 21, 2020, 2:30 a.m. UTC
Since commit d0677992d2af ("cifs: add support for flock") added
support for flock, LTP/flock03[1] testcase started to fail.

This testcase is testing flock lock and unlock across fork.
The parent locks file and starts the child process, in which
it unlock the same fd and lock the same file with another fd
again. All the lock and unlock operation should succeed.

Now the child process does not actually unlock the file, so
the following lock fails. Fix this by allowing flock and OFD
lock go through the unlock routine, not skipping if the unlock
request comes from another process.

Patch has been tested by LTP/xfstests on samba and Windows
server, v3.11, with or without cache=none mount option.

[1] https://github.com/linux-test-project/ltp/blob/master/testcases/kernel/syscalls/flock/flock03.c
Signed-off-by: Murphy Zhou <jencce.kernel@gmail.com>
---
 fs/cifs/smb2file.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

Comments

Murphy Zhou Feb. 26, 2020, 3:39 p.m. UTC | #1
On Fri, Feb 21, 2020 at 10:30:01AM +0800, Murphy Zhou wrote:
> Since commit d0677992d2af ("cifs: add support for flock") added
> support for flock, LTP/flock03[1] testcase started to fail.
> 
> This testcase is testing flock lock and unlock across fork.
> The parent locks file and starts the child process, in which
> it unlock the same fd and lock the same file with another fd
> again. All the lock and unlock operation should succeed.
> 
> Now the child process does not actually unlock the file, so
> the following lock fails. Fix this by allowing flock and OFD
> lock go through the unlock routine, not skipping if the unlock
> request comes from another process.
> 
> Patch has been tested by LTP/xfstests on samba and Windows
> server, v3.11, with or without cache=none mount option.

Also tested with or without "nolease" mount option. No new
issue shows.

Thanks!
> 
> [1] https://github.com/linux-test-project/ltp/blob/master/testcases/kernel/syscalls/flock/flock03.c
> Signed-off-by: Murphy Zhou <jencce.kernel@gmail.com>
> ---
>  fs/cifs/smb2file.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/cifs/smb2file.c b/fs/cifs/smb2file.c
> index afe1f03aabe3..eebfbf3a8c80 100644
> --- a/fs/cifs/smb2file.c
> +++ b/fs/cifs/smb2file.c
> @@ -152,7 +152,12 @@ smb2_unlock_range(struct cifsFileInfo *cfile, struct file_lock *flock,
>  		    (li->offset + li->length))
>  			continue;
>  		if (current->tgid != li->pid)
> -			continue;
> +			/*
> +			 * flock and OFD lock are associated with an open
> +			 * file description, not the process.
> +			 */
> +			if (!(flock->fl_flags & (FL_FLOCK | FL_OFDLCK)))
> +				continue;
>  		if (cinode->can_cache_brlcks) {
>  			/*
>  			 * We can cache brlock requests - simply remove a lock
> -- 
> 2.20.1
> 
>
Pavel Shilovsky Feb. 26, 2020, 6:41 p.m. UTC | #2
ср, 26 февр. 2020 г. в 07:39, Murphy Zhou <jencce.kernel@gmail.com>:
>
> On Fri, Feb 21, 2020 at 10:30:01AM +0800, Murphy Zhou wrote:
> > Since commit d0677992d2af ("cifs: add support for flock") added
> > support for flock, LTP/flock03[1] testcase started to fail.
> >
> > This testcase is testing flock lock and unlock across fork.
> > The parent locks file and starts the child process, in which
> > it unlock the same fd and lock the same file with another fd
> > again. All the lock and unlock operation should succeed.
> >
> > Now the child process does not actually unlock the file, so
> > the following lock fails. Fix this by allowing flock and OFD
> > lock go through the unlock routine, not skipping if the unlock
> > request comes from another process.
> >
> > Patch has been tested by LTP/xfstests on samba and Windows
> > server, v3.11, with or without cache=none mount option.
>
> Also tested with or without "nolease" mount option. No new
> issue shows.
>

Great, thanks!
--
Best regards,
Pavel Shilovsky
Murphy Zhou March 4, 2020, 2:33 a.m. UTC | #3
On Wed, Feb 26, 2020 at 11:39:41PM +0800, Murphy Zhou wrote:
> On Fri, Feb 21, 2020 at 10:30:01AM +0800, Murphy Zhou wrote:
> > Since commit d0677992d2af ("cifs: add support for flock") added
> > support for flock, LTP/flock03[1] testcase started to fail.

Ping on this one?

> > 
> > This testcase is testing flock lock and unlock across fork.
> > The parent locks file and starts the child process, in which
> > it unlock the same fd and lock the same file with another fd
> > again. All the lock and unlock operation should succeed.
> > 
> > Now the child process does not actually unlock the file, so
> > the following lock fails. Fix this by allowing flock and OFD
> > lock go through the unlock routine, not skipping if the unlock
> > request comes from another process.
> > 
> > Patch has been tested by LTP/xfstests on samba and Windows
> > server, v3.11, with or without cache=none mount option.
> 
> Also tested with or without "nolease" mount option. No new
> issue shows.
> 
> Thanks!
> > 
> > [1] https://github.com/linux-test-project/ltp/blob/master/testcases/kernel/syscalls/flock/flock03.c
> > Signed-off-by: Murphy Zhou <jencce.kernel@gmail.com>
> > ---
> >  fs/cifs/smb2file.c | 7 ++++++-
> >  1 file changed, 6 insertions(+), 1 deletion(-)
> > 
> > diff --git a/fs/cifs/smb2file.c b/fs/cifs/smb2file.c
> > index afe1f03aabe3..eebfbf3a8c80 100644
> > --- a/fs/cifs/smb2file.c
> > +++ b/fs/cifs/smb2file.c
> > @@ -152,7 +152,12 @@ smb2_unlock_range(struct cifsFileInfo *cfile, struct file_lock *flock,
> >  		    (li->offset + li->length))
> >  			continue;
> >  		if (current->tgid != li->pid)
> > -			continue;
> > +			/*
> > +			 * flock and OFD lock are associated with an open
> > +			 * file description, not the process.
> > +			 */
> > +			if (!(flock->fl_flags & (FL_FLOCK | FL_OFDLCK)))
> > +				continue;
> >  		if (cinode->can_cache_brlcks) {
> >  			/*
> >  			 * We can cache brlock requests - simply remove a lock
> > -- 
> > 2.20.1
> > 
> > 
> 
> -- 
> Murphy
Pavel Shilovsky March 4, 2020, 4:42 a.m. UTC | #4
It looks that I forgot to ack the patch:

Acked-by: Pavel Shilovsky <pshilov@microsoft.com>

--
Best regards,
Pavel Shilovsky

чт, 20 февр. 2020 г. в 18:30, Murphy Zhou <jencce.kernel@gmail.com>:
>
> Since commit d0677992d2af ("cifs: add support for flock") added
> support for flock, LTP/flock03[1] testcase started to fail.
>
> This testcase is testing flock lock and unlock across fork.
> The parent locks file and starts the child process, in which
> it unlock the same fd and lock the same file with another fd
> again. All the lock and unlock operation should succeed.
>
> Now the child process does not actually unlock the file, so
> the following lock fails. Fix this by allowing flock and OFD
> lock go through the unlock routine, not skipping if the unlock
> request comes from another process.
>
> Patch has been tested by LTP/xfstests on samba and Windows
> server, v3.11, with or without cache=none mount option.
>
> [1] https://github.com/linux-test-project/ltp/blob/master/testcases/kernel/syscalls/flock/flock03.c
> Signed-off-by: Murphy Zhou <jencce.kernel@gmail.com>
> ---
>  fs/cifs/smb2file.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/fs/cifs/smb2file.c b/fs/cifs/smb2file.c
> index afe1f03aabe3..eebfbf3a8c80 100644
> --- a/fs/cifs/smb2file.c
> +++ b/fs/cifs/smb2file.c
> @@ -152,7 +152,12 @@ smb2_unlock_range(struct cifsFileInfo *cfile, struct file_lock *flock,
>                     (li->offset + li->length))
>                         continue;
>                 if (current->tgid != li->pid)
> -                       continue;
> +                       /*
> +                        * flock and OFD lock are associated with an open
> +                        * file description, not the process.
> +                        */
> +                       if (!(flock->fl_flags & (FL_FLOCK | FL_OFDLCK)))
> +                               continue;
>                 if (cinode->can_cache_brlcks) {
>                         /*
>                          * We can cache brlock requests - simply remove a lock
> --
> 2.20.1
>
>
Steve French March 4, 2020, 6:18 a.m. UTC | #5
tentatively merged into cifs-2.6.git for-next pending more testing

On Thu, Feb 20, 2020 at 8:30 PM Murphy Zhou <jencce.kernel@gmail.com> wrote:
>
> Since commit d0677992d2af ("cifs: add support for flock") added
> support for flock, LTP/flock03[1] testcase started to fail.
>
> This testcase is testing flock lock and unlock across fork.
> The parent locks file and starts the child process, in which
> it unlock the same fd and lock the same file with another fd
> again. All the lock and unlock operation should succeed.
>
> Now the child process does not actually unlock the file, so
> the following lock fails. Fix this by allowing flock and OFD
> lock go through the unlock routine, not skipping if the unlock
> request comes from another process.
>
> Patch has been tested by LTP/xfstests on samba and Windows
> server, v3.11, with or without cache=none mount option.
>
> [1] https://github.com/linux-test-project/ltp/blob/master/testcases/kernel/syscalls/flock/flock03.c
> Signed-off-by: Murphy Zhou <jencce.kernel@gmail.com>
> ---
>  fs/cifs/smb2file.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/fs/cifs/smb2file.c b/fs/cifs/smb2file.c
> index afe1f03aabe3..eebfbf3a8c80 100644
> --- a/fs/cifs/smb2file.c
> +++ b/fs/cifs/smb2file.c
> @@ -152,7 +152,12 @@ smb2_unlock_range(struct cifsFileInfo *cfile, struct file_lock *flock,
>                     (li->offset + li->length))
>                         continue;
>                 if (current->tgid != li->pid)
> -                       continue;
> +                       /*
> +                        * flock and OFD lock are associated with an open
> +                        * file description, not the process.
> +                        */
> +                       if (!(flock->fl_flags & (FL_FLOCK | FL_OFDLCK)))
> +                               continue;
>                 if (cinode->can_cache_brlcks) {
>                         /*
>                          * We can cache brlock requests - simply remove a lock
> --
> 2.20.1
>
>

Patch
diff mbox series

diff --git a/fs/cifs/smb2file.c b/fs/cifs/smb2file.c
index afe1f03aabe3..eebfbf3a8c80 100644
--- a/fs/cifs/smb2file.c
+++ b/fs/cifs/smb2file.c
@@ -152,7 +152,12 @@  smb2_unlock_range(struct cifsFileInfo *cfile, struct file_lock *flock,
 		    (li->offset + li->length))
 			continue;
 		if (current->tgid != li->pid)
-			continue;
+			/*
+			 * flock and OFD lock are associated with an open
+			 * file description, not the process.
+			 */
+			if (!(flock->fl_flags & (FL_FLOCK | FL_OFDLCK)))
+				continue;
 		if (cinode->can_cache_brlcks) {
 			/*
 			 * We can cache brlock requests - simply remove a lock