Linux-CIFS Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH 0/1] add support for fallocate mode 0
@ 2020-01-15  1:23 Ronnie Sahlberg
  2020-01-15  1:23 ` [PATCH] cifs: add support for fallocate mode 0 for non-sparse files Ronnie Sahlberg
  0 siblings, 1 reply; 11+ messages in thread
From: Ronnie Sahlberg @ 2020-01-15  1:23 UTC (permalink / raw)
  To: linux-cifs

Steve,
Please find a small patch that adds support for fallocate mode 0
to extend a non-sparse file.



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

* [PATCH] cifs: add support for fallocate mode 0 for non-sparse files
  2020-01-15  1:23 [PATCH 0/1] add support for fallocate mode 0 Ronnie Sahlberg
@ 2020-01-15  1:23 ` Ronnie Sahlberg
  2020-01-15  1:25   ` Steve French
  0 siblings, 1 reply; 11+ messages in thread
From: Ronnie Sahlberg @ 2020-01-15  1:23 UTC (permalink / raw)
  To: linux-cifs; +Cc: Ronnie Sahlberg

RHBZ 1336264

Signed-off-by: Ronnie Sahlberg <lsahlber@redhat.com>
---
 fs/cifs/smb2ops.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/fs/cifs/smb2ops.c b/fs/cifs/smb2ops.c
index 6250370c1170..91818f7c1b9c 100644
--- a/fs/cifs/smb2ops.c
+++ b/fs/cifs/smb2ops.c
@@ -3106,9 +3106,13 @@ static long smb3_simple_falloc(struct file *file, struct cifs_tcon *tcon,
 		else if (i_size_read(inode) >= off + len)
 			/* not extending file and already not sparse */
 			rc = 0;
-		/* BB: in future add else clause to extend file */
-		else
-			rc = -EOPNOTSUPP;
+		/* extend file */
+		else {
+			eof = cpu_to_le64(off + len);
+			rc = SMB2_set_eof(xid, tcon, cfile->fid.persistent_fid,
+					  cfile->fid.volatile_fid, cfile->pid,
+					  &eof);
+		}
 		if (rc)
 			trace_smb3_falloc_err(xid, cfile->fid.persistent_fid,
 				tcon->tid, tcon->ses->Suid, off, len, rc);
-- 
2.13.6


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

* Re: [PATCH] cifs: add support for fallocate mode 0 for non-sparse files
  2020-01-15  1:23 ` [PATCH] cifs: add support for fallocate mode 0 for non-sparse files Ronnie Sahlberg
@ 2020-01-15  1:25   ` Steve French
  2020-01-15  2:25     ` ronnie sahlberg
  0 siblings, 1 reply; 11+ messages in thread
From: Steve French @ 2020-01-15  1:25 UTC (permalink / raw)
  To: Ronnie Sahlberg; +Cc: linux-cifs

Does it affect (or enable) any xfstests?

On Tue, Jan 14, 2020 at 7:23 PM Ronnie Sahlberg <lsahlber@redhat.com> wrote:
>
> RHBZ 1336264
>
> Signed-off-by: Ronnie Sahlberg <lsahlber@redhat.com>
> ---
>  fs/cifs/smb2ops.c | 10 +++++++---
>  1 file changed, 7 insertions(+), 3 deletions(-)
>
> diff --git a/fs/cifs/smb2ops.c b/fs/cifs/smb2ops.c
> index 6250370c1170..91818f7c1b9c 100644
> --- a/fs/cifs/smb2ops.c
> +++ b/fs/cifs/smb2ops.c
> @@ -3106,9 +3106,13 @@ static long smb3_simple_falloc(struct file *file, struct cifs_tcon *tcon,
>                 else if (i_size_read(inode) >= off + len)
>                         /* not extending file and already not sparse */
>                         rc = 0;
> -               /* BB: in future add else clause to extend file */
> -               else
> -                       rc = -EOPNOTSUPP;
> +               /* extend file */
> +               else {
> +                       eof = cpu_to_le64(off + len);
> +                       rc = SMB2_set_eof(xid, tcon, cfile->fid.persistent_fid,
> +                                         cfile->fid.volatile_fid, cfile->pid,
> +                                         &eof);
> +               }
>                 if (rc)
>                         trace_smb3_falloc_err(xid, cfile->fid.persistent_fid,
>                                 tcon->tid, tcon->ses->Suid, off, len, rc);
> --
> 2.13.6
>


-- 
Thanks,

Steve

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

* Re: [PATCH] cifs: add support for fallocate mode 0 for non-sparse files
  2020-01-15  1:25   ` Steve French
@ 2020-01-15  2:25     ` ronnie sahlberg
  2020-01-15 20:14       ` Steve French
  0 siblings, 1 reply; 11+ messages in thread
From: ronnie sahlberg @ 2020-01-15  2:25 UTC (permalink / raw)
  To: Steve French; +Cc: Ronnie Sahlberg, linux-cifs

On Wed, Jan 15, 2020 at 11:25 AM Steve French <smfrench@gmail.com> wrote:
>
> Does it affect (or enable) any xfstests?

It shouldn't affect any current tests.
It adds support for
   xfs_io -c "falloc 0 512M" <file>

generic/071 now passes with this patch.   Possibly other tests as well
that use "xfs_io -c falloc" as well


>
> On Tue, Jan 14, 2020 at 7:23 PM Ronnie Sahlberg <lsahlber@redhat.com> wrote:
> >
> > RHBZ 1336264
> >
> > Signed-off-by: Ronnie Sahlberg <lsahlber@redhat.com>
> > ---
> >  fs/cifs/smb2ops.c | 10 +++++++---
> >  1 file changed, 7 insertions(+), 3 deletions(-)
> >
> > diff --git a/fs/cifs/smb2ops.c b/fs/cifs/smb2ops.c
> > index 6250370c1170..91818f7c1b9c 100644
> > --- a/fs/cifs/smb2ops.c
> > +++ b/fs/cifs/smb2ops.c
> > @@ -3106,9 +3106,13 @@ static long smb3_simple_falloc(struct file *file, struct cifs_tcon *tcon,
> >                 else if (i_size_read(inode) >= off + len)
> >                         /* not extending file and already not sparse */
> >                         rc = 0;
> > -               /* BB: in future add else clause to extend file */
> > -               else
> > -                       rc = -EOPNOTSUPP;
> > +               /* extend file */
> > +               else {
> > +                       eof = cpu_to_le64(off + len);
> > +                       rc = SMB2_set_eof(xid, tcon, cfile->fid.persistent_fid,
> > +                                         cfile->fid.volatile_fid, cfile->pid,
> > +                                         &eof);
> > +               }
> >                 if (rc)
> >                         trace_smb3_falloc_err(xid, cfile->fid.persistent_fid,
> >                                 tcon->tid, tcon->ses->Suid, off, len, rc);
> > --
> > 2.13.6
> >
>
>
> --
> Thanks,
>
> Steve

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

* Re: [PATCH] cifs: add support for fallocate mode 0 for non-sparse files
  2020-01-15  2:25     ` ronnie sahlberg
@ 2020-01-15 20:14       ` Steve French
  2020-01-16  2:03         ` Steve French
  0 siblings, 1 reply; 11+ messages in thread
From: Steve French @ 2020-01-15 20:14 UTC (permalink / raw)
  To: ronnie sahlberg; +Cc: Ronnie Sahlberg, linux-cifs

Tentatively merged into cifs-2.6.git for-next pending more testing

On Tue, Jan 14, 2020 at 8:25 PM ronnie sahlberg
<ronniesahlberg@gmail.com> wrote:
>
> On Wed, Jan 15, 2020 at 11:25 AM Steve French <smfrench@gmail.com> wrote:
> >
> > Does it affect (or enable) any xfstests?
>
> It shouldn't affect any current tests.
> It adds support for
>    xfs_io -c "falloc 0 512M" <file>
>
> generic/071 now passes with this patch.   Possibly other tests as well
> that use "xfs_io -c falloc" as well
>
>
> >
> > On Tue, Jan 14, 2020 at 7:23 PM Ronnie Sahlberg <lsahlber@redhat.com> wrote:
> > >
> > > RHBZ 1336264
> > >
> > > Signed-off-by: Ronnie Sahlberg <lsahlber@redhat.com>
> > > ---
> > >  fs/cifs/smb2ops.c | 10 +++++++---
> > >  1 file changed, 7 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/fs/cifs/smb2ops.c b/fs/cifs/smb2ops.c
> > > index 6250370c1170..91818f7c1b9c 100644
> > > --- a/fs/cifs/smb2ops.c
> > > +++ b/fs/cifs/smb2ops.c
> > > @@ -3106,9 +3106,13 @@ static long smb3_simple_falloc(struct file *file, struct cifs_tcon *tcon,
> > >                 else if (i_size_read(inode) >= off + len)
> > >                         /* not extending file and already not sparse */
> > >                         rc = 0;
> > > -               /* BB: in future add else clause to extend file */
> > > -               else
> > > -                       rc = -EOPNOTSUPP;
> > > +               /* extend file */
> > > +               else {
> > > +                       eof = cpu_to_le64(off + len);
> > > +                       rc = SMB2_set_eof(xid, tcon, cfile->fid.persistent_fid,
> > > +                                         cfile->fid.volatile_fid, cfile->pid,
> > > +                                         &eof);
> > > +               }
> > >                 if (rc)
> > >                         trace_smb3_falloc_err(xid, cfile->fid.persistent_fid,
> > >                                 tcon->tid, tcon->ses->Suid, off, len, rc);
> > > --
> > > 2.13.6
> > >
> >
> >
> > --
> > Thanks,
> >
> > Steve



-- 
Thanks,

Steve

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

* Re: [PATCH] cifs: add support for fallocate mode 0 for non-sparse files
  2020-01-15 20:14       ` Steve French
@ 2020-01-16  2:03         ` Steve French
  2020-01-16  8:42           ` ronnie sahlberg
  0 siblings, 1 reply; 11+ messages in thread
From: Steve French @ 2020-01-16  2:03 UTC (permalink / raw)
  To: unlisted-recipients:; (no To-header on input); +Cc: Ronnie Sahlberg, linux-cifs

temporarily removed to allow Ronnie to debug a test failure

On Wed, Jan 15, 2020 at 2:14 PM Steve French <smfrench@gmail.com> wrote:
>
> Tentatively merged into cifs-2.6.git for-next pending more testing
>
> On Tue, Jan 14, 2020 at 8:25 PM ronnie sahlberg
> <ronniesahlberg@gmail.com> wrote:
> >
> > On Wed, Jan 15, 2020 at 11:25 AM Steve French <smfrench@gmail.com> wrote:
> > >
> > > Does it affect (or enable) any xfstests?
> >
> > It shouldn't affect any current tests.
> > It adds support for
> >    xfs_io -c "falloc 0 512M" <file>
> >
> > generic/071 now passes with this patch.   Possibly other tests as well
> > that use "xfs_io -c falloc" as well
> >
> >
> > >
> > > On Tue, Jan 14, 2020 at 7:23 PM Ronnie Sahlberg <lsahlber@redhat.com> wrote:
> > > >
> > > > RHBZ 1336264
> > > >
> > > > Signed-off-by: Ronnie Sahlberg <lsahlber@redhat.com>
> > > > ---
> > > >  fs/cifs/smb2ops.c | 10 +++++++---
> > > >  1 file changed, 7 insertions(+), 3 deletions(-)
> > > >
> > > > diff --git a/fs/cifs/smb2ops.c b/fs/cifs/smb2ops.c
> > > > index 6250370c1170..91818f7c1b9c 100644
> > > > --- a/fs/cifs/smb2ops.c
> > > > +++ b/fs/cifs/smb2ops.c
> > > > @@ -3106,9 +3106,13 @@ static long smb3_simple_falloc(struct file *file, struct cifs_tcon *tcon,
> > > >                 else if (i_size_read(inode) >= off + len)
> > > >                         /* not extending file and already not sparse */
> > > >                         rc = 0;
> > > > -               /* BB: in future add else clause to extend file */
> > > > -               else
> > > > -                       rc = -EOPNOTSUPP;
> > > > +               /* extend file */
> > > > +               else {
> > > > +                       eof = cpu_to_le64(off + len);
> > > > +                       rc = SMB2_set_eof(xid, tcon, cfile->fid.persistent_fid,
> > > > +                                         cfile->fid.volatile_fid, cfile->pid,
> > > > +                                         &eof);
> > > > +               }
> > > >                 if (rc)
> > > >                         trace_smb3_falloc_err(xid, cfile->fid.persistent_fid,
> > > >                                 tcon->tid, tcon->ses->Suid, off, len, rc);
> > > > --
> > > > 2.13.6
> > > >
> > >
> > >
> > > --
> > > Thanks,
> > >
> > > Steve
>
>
>
> --
> Thanks,
>
> Steve



-- 
Thanks,

Steve

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

* Re: [PATCH] cifs: add support for fallocate mode 0 for non-sparse files
  2020-01-16  2:03         ` Steve French
@ 2020-01-16  8:42           ` ronnie sahlberg
  2020-01-17  0:27             ` Pavel Shilovsky
  0 siblings, 1 reply; 11+ messages in thread
From: ronnie sahlberg @ 2020-01-16  8:42 UTC (permalink / raw)
  To: Steve French; +Cc: Ronnie Sahlberg, linux-cifs

The bug is basically that if we extend a file by fallocate mode==0
and immediately afterwards mmap() the file we will only mmap() as much
as end-of-file was
prior to the truncate  and then if we try to touch any
address in this extended region userspace dies with bus error.

The patch added "extend a file with fallocate mode==0 for NON-Sparse
files" and caused xfstest to fail.
The fix is to force us to revalidate the file attributes (the size is
the important one) when we extend the file so
mmap() will work properly.
I have fixed this in the patch and will resend tomorrow after some more testing.

Looking for other SMB2_set_eof() callsites I see we already had the
same bug for the case of extending a SPARSE
file using fallocate mode==0. I fixed that too and will audit all
other plases where we use SMB2_set_eof()
to see if they are safe or not before resending.


On Thu, Jan 16, 2020 at 12:33 PM Steve French <smfrench@gmail.com> wrote:
>
> temporarily removed to allow Ronnie to debug a test failure
>
> On Wed, Jan 15, 2020 at 2:14 PM Steve French <smfrench@gmail.com> wrote:
> >
> > Tentatively merged into cifs-2.6.git for-next pending more testing
> >
> > On Tue, Jan 14, 2020 at 8:25 PM ronnie sahlberg
> > <ronniesahlberg@gmail.com> wrote:
> > >
> > > On Wed, Jan 15, 2020 at 11:25 AM Steve French <smfrench@gmail.com> wrote:
> > > >
> > > > Does it affect (or enable) any xfstests?
> > >
> > > It shouldn't affect any current tests.
> > > It adds support for
> > >    xfs_io -c "falloc 0 512M" <file>
> > >
> > > generic/071 now passes with this patch.   Possibly other tests as well
> > > that use "xfs_io -c falloc" as well
> > >
> > >
> > > >
> > > > On Tue, Jan 14, 2020 at 7:23 PM Ronnie Sahlberg <lsahlber@redhat.com> wrote:
> > > > >
> > > > > RHBZ 1336264
> > > > >
> > > > > Signed-off-by: Ronnie Sahlberg <lsahlber@redhat.com>
> > > > > ---
> > > > >  fs/cifs/smb2ops.c | 10 +++++++---
> > > > >  1 file changed, 7 insertions(+), 3 deletions(-)
> > > > >
> > > > > diff --git a/fs/cifs/smb2ops.c b/fs/cifs/smb2ops.c
> > > > > index 6250370c1170..91818f7c1b9c 100644
> > > > > --- a/fs/cifs/smb2ops.c
> > > > > +++ b/fs/cifs/smb2ops.c
> > > > > @@ -3106,9 +3106,13 @@ static long smb3_simple_falloc(struct file *file, struct cifs_tcon *tcon,
> > > > >                 else if (i_size_read(inode) >= off + len)
> > > > >                         /* not extending file and already not sparse */
> > > > >                         rc = 0;
> > > > > -               /* BB: in future add else clause to extend file */
> > > > > -               else
> > > > > -                       rc = -EOPNOTSUPP;
> > > > > +               /* extend file */
> > > > > +               else {
> > > > > +                       eof = cpu_to_le64(off + len);
> > > > > +                       rc = SMB2_set_eof(xid, tcon, cfile->fid.persistent_fid,
> > > > > +                                         cfile->fid.volatile_fid, cfile->pid,
> > > > > +                                         &eof);
> > > > > +               }
> > > > >                 if (rc)
> > > > >                         trace_smb3_falloc_err(xid, cfile->fid.persistent_fid,
> > > > >                                 tcon->tid, tcon->ses->Suid, off, len, rc);
> > > > > --
> > > > > 2.13.6
> > > > >
> > > >
> > > >
> > > > --
> > > > Thanks,
> > > >
> > > > Steve
> >
> >
> >
> > --
> > Thanks,
> >
> > Steve
>
>
>
> --
> Thanks,
>
> Steve

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

* Re: [PATCH] cifs: add support for fallocate mode 0 for non-sparse files
  2020-01-16  8:42           ` ronnie sahlberg
@ 2020-01-17  0:27             ` Pavel Shilovsky
  2020-01-17  0:56               ` ronnie sahlberg
  0 siblings, 1 reply; 11+ messages in thread
From: Pavel Shilovsky @ 2020-01-17  0:27 UTC (permalink / raw)
  To: ronnie sahlberg; +Cc: Steve French, Ronnie Sahlberg, linux-cifs

чт, 16 янв. 2020 г. в 01:05, ronnie sahlberg <ronniesahlberg@gmail.com>:
>
> The bug is basically that if we extend a file by fallocate mode==0
> and immediately afterwards mmap() the file we will only mmap() as much
> as end-of-file was
> prior to the truncate  and then if we try to touch any
> address in this extended region userspace dies with bus error.
>
> The patch added "extend a file with fallocate mode==0 for NON-Sparse
> files" and caused xfstest to fail.
> The fix is to force us to revalidate the file attributes (the size is
> the important one) when we extend the file so
> mmap() will work properly.
> I have fixed this in the patch and will resend tomorrow after some more testing.
>
> Looking for other SMB2_set_eof() callsites I see we already had the
> same bug for the case of extending a SPARSE

I agree that regardless of file being sparse or not, we should somehow
update a size in the VFS after calling SMB2_set_eof().

> file using fallocate mode==0. I fixed that too and will audit all
> other plases where we use SMB2_set_eof()
> to see if they are safe or not before resending.

One of those places where SMB2_set_eof() is called is
cifs_set_file_size() which does call the following after getting a
successful response from the server:

2250 >-------if (rc == 0) {
2251 >------->-------cifsInode->server_eof = attrs->ia_size;
2252 >------->-------cifs_setsize(inode, attrs->ia_size);
2253 >------->-------cifs_truncate_page(inode->i_mapping, inode->i_size);
2254 >-------}

This is called by cifs_setattr_[no]unix() which does the following afterwards:

2569 >-------if ((attrs->ia_valid & ATTR_SIZE) &&
2570 >-------    attrs->ia_size != i_size_read(inode))
2571 >------->-------truncate_setsize(inode, attrs->ia_size);

truncate_setsize() does  similar things as cifs_setsize() besides
setting cinode->time to 0. This code path probably needs to be
refactored. But putting this aside, for the current fallocate fix I
think we should use the same existing mechanism to update a file size
in the VFS without revalidating the inode.

--
Best regards,
Pavel Shilovsky

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

* Re: [PATCH] cifs: add support for fallocate mode 0 for non-sparse files
  2020-01-17  0:27             ` Pavel Shilovsky
@ 2020-01-17  0:56               ` ronnie sahlberg
  0 siblings, 0 replies; 11+ messages in thread
From: ronnie sahlberg @ 2020-01-17  0:56 UTC (permalink / raw)
  To: Pavel Shilovsky; +Cc: Steve French, Ronnie Sahlberg, linux-cifs

On Fri, Jan 17, 2020 at 10:27 AM Pavel Shilovsky <piastryyy@gmail.com> wrote:
>

Thanks.

> чт, 16 янв. 2020 г. в 01:05, ronnie sahlberg <ronniesahlberg@gmail.com>:
> >
> > The bug is basically that if we extend a file by fallocate mode==0
> > and immediately afterwards mmap() the file we will only mmap() as much
> > as end-of-file was
> > prior to the truncate  and then if we try to touch any
> > address in this extended region userspace dies with bus error.
> >
> > The patch added "extend a file with fallocate mode==0 for NON-Sparse
> > files" and caused xfstest to fail.
> > The fix is to force us to revalidate the file attributes (the size is
> > the important one) when we extend the file so
> > mmap() will work properly.
> > I have fixed this in the patch and will resend tomorrow after some more testing.
> >
> > Looking for other SMB2_set_eof() callsites I see we already had the
> > same bug for the case of extending a SPARSE
>
> I agree that regardless of file being sparse or not, we should somehow
> update a size in the VFS after calling SMB2_set_eof().
>
> > file using fallocate mode==0. I fixed that too and will audit all
> > other plases where we use SMB2_set_eof()
> > to see if they are safe or not before resending.
>
> One of those places where SMB2_set_eof() is called is
> cifs_set_file_size() which does call the following after getting a
> successful response from the server:
>
> 2250 >-------if (rc == 0) {
> 2251 >------->-------cifsInode->server_eof = attrs->ia_size;
> 2252 >------->-------cifs_setsize(inode, attrs->ia_size);
> 2253 >------->-------cifs_truncate_page(inode->i_mapping, inode->i_size);
> 2254 >-------}
>
> This is called by cifs_setattr_[no]unix() which does the following afterwards:
>
> 2569 >-------if ((attrs->ia_valid & ATTR_SIZE) &&
> 2570 >-------    attrs->ia_size != i_size_read(inode))
> 2571 >------->-------truncate_setsize(inode, attrs->ia_size);
>
> truncate_setsize() does  similar things as cifs_setsize() besides
> setting cinode->time to 0. This code path probably needs to be
> refactored. But putting this aside, for the current fallocate fix I
> think we should use the same existing mechanism to update a file size
> in the VFS without revalidating the inode.

I can do that change. Note however that since we are still setting
cinode->time to 0
we are still going to trigger a revalidation at some stage.

>
> --
> Best regards,
> Pavel Shilovsky

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

* [PATCH] cifs: add support for fallocate mode 0 for non-sparse files
@ 2020-01-17  1:15 Ronnie Sahlberg
  0 siblings, 0 replies; 11+ messages in thread
From: Ronnie Sahlberg @ 2020-01-17  1:15 UTC (permalink / raw)
  To: linux-cifs; +Cc: Ronnie Sahlberg

RHBZ 1336264

When we extend a file we must also force the size to be updated.

This fixes an issue with holetest in xfs-tests which performs the following
sequence :
1, create a new file
2, use fallocate mode==0 to populate the file
3, mmap the file
4, touch each page by reading the mmapped region.

Signed-off-by: Ronnie Sahlberg <lsahlber@redhat.com>
---
 fs/cifs/cifsfs.h  |  3 +++
 fs/cifs/inode.c   |  4 ++--
 fs/cifs/smb2ops.c | 64 +++++++++++++++++++++++++------------------------------
 3 files changed, 34 insertions(+), 37 deletions(-)

diff --git a/fs/cifs/cifsfs.h b/fs/cifs/cifsfs.h
index b59dc7478130..096a4c18fbd0 100644
--- a/fs/cifs/cifsfs.h
+++ b/fs/cifs/cifsfs.h
@@ -149,6 +149,9 @@ extern ssize_t cifs_file_copychunk_range(unsigned int xid,
 					size_t len, unsigned int flags);
 
 extern long cifs_ioctl(struct file *filep, unsigned int cmd, unsigned long arg);
+extern void cifs_setsize(struct inode *inode, loff_t offset);
+extern int cifs_truncate_page(struct address_space *mapping, loff_t from);
+
 #ifdef CONFIG_CIFS_NFSD_EXPORT
 extern const struct export_operations cifs_export_ops;
 #endif /* CONFIG_CIFS_NFSD_EXPORT */
diff --git a/fs/cifs/inode.c b/fs/cifs/inode.c
index ca76a9287456..9b547f7f5f5d 100644
--- a/fs/cifs/inode.c
+++ b/fs/cifs/inode.c
@@ -2228,7 +2228,7 @@ int cifs_fiemap(struct inode *inode, struct fiemap_extent_info *fei, u64 start,
 	return -ENOTSUPP;
 }
 
-static int cifs_truncate_page(struct address_space *mapping, loff_t from)
+int cifs_truncate_page(struct address_space *mapping, loff_t from)
 {
 	pgoff_t index = from >> PAGE_SHIFT;
 	unsigned offset = from & (PAGE_SIZE - 1);
@@ -2245,7 +2245,7 @@ static int cifs_truncate_page(struct address_space *mapping, loff_t from)
 	return rc;
 }
 
-static void cifs_setsize(struct inode *inode, loff_t offset)
+void cifs_setsize(struct inode *inode, loff_t offset)
 {
 	struct cifsInodeInfo *cifs_i = CIFS_I(inode);
 
diff --git a/fs/cifs/smb2ops.c b/fs/cifs/smb2ops.c
index 6250370c1170..938c985b9919 100644
--- a/fs/cifs/smb2ops.c
+++ b/fs/cifs/smb2ops.c
@@ -12,6 +12,7 @@
 #include <linux/uuid.h>
 #include <linux/sort.h>
 #include <crypto/aead.h>
+#include "cifsfs.h"
 #include "cifsglob.h"
 #include "smb2pdu.h"
 #include "smb2proto.h"
@@ -3095,28 +3096,32 @@ static long smb3_simple_falloc(struct file *file, struct cifs_tcon *tcon,
 		}
 
 	/*
+	 * Extending the file
+	 */
+	if ((keep_size == false) && i_size_read(inode) < off + len) {
+		if ((cifsi->cifsAttrs & FILE_ATTRIBUTE_SPARSE_FILE) == 0)
+			smb2_set_sparse(xid, tcon, cfile, inode, false);
+
+		eof = cpu_to_le64(off + len);
+		rc = SMB2_set_eof(xid, tcon, cfile->fid.persistent_fid,
+				  cfile->fid.volatile_fid, cfile->pid, &eof);
+		if (rc == 0) {
+			cifsi->server_eof = off + len;
+			cifs_setsize(inode, off + len);
+			cifs_truncate_page(inode->i_mapping, inode->i_size);
+			truncate_setsize(inode, off + len);
+		}
+		goto out;
+	}
+
+	/*
 	 * Files are non-sparse by default so falloc may be a no-op
-	 * Must check if file sparse. If not sparse, and not extending
-	 * then no need to do anything since file already allocated
+	 * Must check if file sparse. If not sparse, and since we are not
+	 * extending then no need to do anything since file already allocated
 	 */
 	if ((cifsi->cifsAttrs & FILE_ATTRIBUTE_SPARSE_FILE) == 0) {
-		if (keep_size == true)
-			rc = 0;
-		/* check if extending file */
-		else if (i_size_read(inode) >= off + len)
-			/* not extending file and already not sparse */
-			rc = 0;
-		/* BB: in future add else clause to extend file */
-		else
-			rc = -EOPNOTSUPP;
-		if (rc)
-			trace_smb3_falloc_err(xid, cfile->fid.persistent_fid,
-				tcon->tid, tcon->ses->Suid, off, len, rc);
-		else
-			trace_smb3_falloc_done(xid, cfile->fid.persistent_fid,
-				tcon->tid, tcon->ses->Suid, off, len);
-		free_xid(xid);
-		return rc;
+		rc = 0;
+		goto out;
 	}
 
 	if ((keep_size == true) || (i_size_read(inode) >= off + len)) {
@@ -3130,25 +3135,14 @@ static long smb3_simple_falloc(struct file *file, struct cifs_tcon *tcon,
 		 */
 		if ((off > 8192) || (off + len + 8192 < i_size_read(inode))) {
 			rc = -EOPNOTSUPP;
-			trace_smb3_falloc_err(xid, cfile->fid.persistent_fid,
-				tcon->tid, tcon->ses->Suid, off, len, rc);
-			free_xid(xid);
-			return rc;
-		}
-
-		smb2_set_sparse(xid, tcon, cfile, inode, false);
-		rc = 0;
-	} else {
-		smb2_set_sparse(xid, tcon, cfile, inode, false);
-		rc = 0;
-		if (i_size_read(inode) < off + len) {
-			eof = cpu_to_le64(off + len);
-			rc = SMB2_set_eof(xid, tcon, cfile->fid.persistent_fid,
-					  cfile->fid.volatile_fid, cfile->pid,
-					  &eof);
+			goto out;
 		}
 	}
 
+	smb2_set_sparse(xid, tcon, cfile, inode, false);
+	rc = 0;
+
+out:
 	if (rc)
 		trace_smb3_falloc_err(xid, cfile->fid.persistent_fid, tcon->tid,
 				tcon->ses->Suid, off, len, rc);
-- 
2.13.6


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

* [PATCH] cifs: add support for fallocate mode 0 for non-sparse files
@ 2020-01-17  0:04 Ronnie Sahlberg
  0 siblings, 0 replies; 11+ messages in thread
From: Ronnie Sahlberg @ 2020-01-17  0:04 UTC (permalink / raw)
  To: linux-cifs; +Cc: Ronnie Sahlberg

RHBZ 1336264

When we extend a file we must also force the attributes (the size)
to be revalidated.

This fixes an issue with holetest in xfs-tests which performs the following
sequence :
1, create a new file
2, use fallocate mode==0 to populate the file
3, mmap the file
4, touch each page by reading the mmapped region.

If we don't revalidate the file as part of step 2, the handle will still have
cached attributes where the size is 0 and thus the mmap will not map the
requested region leading to SIGBUS for the application in step 4.

We already had the same crash bug for sparse files but we never triggered that
with our xfstests.

Signed-off-by: Ronnie Sahlberg <lsahlber@redhat.com>
---
 fs/cifs/cifsfs.c  |  6 ++----
 fs/cifs/cifsfs.h  |  2 +-
 fs/cifs/inode.c   |  6 +++---
 fs/cifs/smb2ops.c | 23 ++++++++++++++++++++---
 4 files changed, 26 insertions(+), 11 deletions(-)

diff --git a/fs/cifs/cifsfs.c b/fs/cifs/cifsfs.c
index 5492b9860baa..638337dc04ee 100644
--- a/fs/cifs/cifsfs.c
+++ b/fs/cifs/cifsfs.c
@@ -957,11 +957,9 @@ static loff_t cifs_llseek(struct file *file, loff_t offset, int whence)
 		/*
 		 * Some applications poll for the file length in this strange
 		 * way so we must seek to end on non-oplocked files by
-		 * setting the revalidate time to zero.
+		 * forcing revalidation.
 		 */
-		CIFS_I(inode)->time = 0;
-
-		rc = cifs_revalidate_file_attr(file);
+		rc = cifs_revalidate_file_attr(file, true);
 		if (rc < 0)
 			return (loff_t)rc;
 	}
diff --git a/fs/cifs/cifsfs.h b/fs/cifs/cifsfs.h
index b59dc7478130..e72aec945875 100644
--- a/fs/cifs/cifsfs.h
+++ b/fs/cifs/cifsfs.h
@@ -75,7 +75,7 @@ extern int cifs_mkdir(struct inode *, struct dentry *, umode_t);
 extern int cifs_rmdir(struct inode *, struct dentry *);
 extern int cifs_rename2(struct inode *, struct dentry *, struct inode *,
 			struct dentry *, unsigned int);
-extern int cifs_revalidate_file_attr(struct file *filp);
+extern int cifs_revalidate_file_attr(struct file *filp, bool force);
 extern int cifs_revalidate_dentry_attr(struct dentry *);
 extern int cifs_revalidate_file(struct file *filp);
 extern int cifs_revalidate_dentry(struct dentry *);
diff --git a/fs/cifs/inode.c b/fs/cifs/inode.c
index ca76a9287456..1db84db822ec 100644
--- a/fs/cifs/inode.c
+++ b/fs/cifs/inode.c
@@ -2050,13 +2050,13 @@ cifs_zap_mapping(struct inode *inode)
 	return cifs_revalidate_mapping(inode);
 }
 
-int cifs_revalidate_file_attr(struct file *filp)
+int cifs_revalidate_file_attr(struct file *filp, bool force)
 {
 	int rc = 0;
 	struct inode *inode = file_inode(filp);
 	struct cifsFileInfo *cfile = (struct cifsFileInfo *) filp->private_data;
 
-	if (!cifs_inode_needs_reval(inode))
+	if (!force && !cifs_inode_needs_reval(inode))
 		return rc;
 
 	if (tlink_tcon(cfile->tlink)->unix_ext)
@@ -2112,7 +2112,7 @@ int cifs_revalidate_file(struct file *filp)
 	int rc;
 	struct inode *inode = file_inode(filp);
 
-	rc = cifs_revalidate_file_attr(filp);
+	rc = cifs_revalidate_file_attr(filp, false);
 	if (rc)
 		return rc;
 
diff --git a/fs/cifs/smb2ops.c b/fs/cifs/smb2ops.c
index 6250370c1170..6c0fe6544fd7 100644
--- a/fs/cifs/smb2ops.c
+++ b/fs/cifs/smb2ops.c
@@ -12,6 +12,7 @@
 #include <linux/uuid.h>
 #include <linux/sort.h>
 #include <crypto/aead.h>
+#include "cifsfs.h"
 #include "cifsglob.h"
 #include "smb2pdu.h"
 #include "smb2proto.h"
@@ -3106,9 +3107,19 @@ static long smb3_simple_falloc(struct file *file, struct cifs_tcon *tcon,
 		else if (i_size_read(inode) >= off + len)
 			/* not extending file and already not sparse */
 			rc = 0;
-		/* BB: in future add else clause to extend file */
-		else
-			rc = -EOPNOTSUPP;
+		/* extend file */
+		else {
+			eof = cpu_to_le64(off + len);
+			rc = SMB2_set_eof(xid, tcon, cfile->fid.persistent_fid,
+					  cfile->fid.volatile_fid, cfile->pid,
+					  &eof);
+			if (!rc) {
+				rc = cifs_revalidate_file_attr(file, true);
+				if (rc)
+					cifs_dbg(FYI, "Validation during fallocate "
+						 "failed, error=%ld\n", rc);
+			}
+		}
 		if (rc)
 			trace_smb3_falloc_err(xid, cfile->fid.persistent_fid,
 				tcon->tid, tcon->ses->Suid, off, len, rc);
@@ -3146,6 +3157,12 @@ static long smb3_simple_falloc(struct file *file, struct cifs_tcon *tcon,
 			rc = SMB2_set_eof(xid, tcon, cfile->fid.persistent_fid,
 					  cfile->fid.volatile_fid, cfile->pid,
 					  &eof);
+			if (!rc) {
+				rc = cifs_revalidate_file_attr(file, true);
+				if (rc)
+					cifs_dbg(FYI, "Validation during fallocate "
+						 "failed, error=%ld\n", rc);
+			}
 		}
 	}
 
-- 
2.13.6


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

end of thread, back to index

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-01-15  1:23 [PATCH 0/1] add support for fallocate mode 0 Ronnie Sahlberg
2020-01-15  1:23 ` [PATCH] cifs: add support for fallocate mode 0 for non-sparse files Ronnie Sahlberg
2020-01-15  1:25   ` Steve French
2020-01-15  2:25     ` ronnie sahlberg
2020-01-15 20:14       ` Steve French
2020-01-16  2:03         ` Steve French
2020-01-16  8:42           ` ronnie sahlberg
2020-01-17  0:27             ` Pavel Shilovsky
2020-01-17  0:56               ` ronnie sahlberg
2020-01-17  0:04 Ronnie Sahlberg
2020-01-17  1:15 Ronnie Sahlberg

Linux-CIFS Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-cifs/0 linux-cifs/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-cifs linux-cifs/ https://lore.kernel.org/linux-cifs \
		linux-cifs@vger.kernel.org
	public-inbox-index linux-cifs

Example config snippet for mirrors

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


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