[PATCH] cifs: destage any unwritten data to the server before calling copychunk_write

[PATCH] cifs: destage any unwritten data to the server before calling copychunk_write

[Ronnie Sahlberg]

because the copychunk_write might cover a region of the file that has not yet
been sent to the server and thus fail.

A simple way to reproduce this is:
truncate -s 0 /mnt/testfile; strace -f -o x -ttT xfs_io -i -f -c 'pwrite 0k 128k' -c 'fcollapse 16k 24k' /mnt/testfile

the issue is that the 'pwrite 0k 128k' becomes rearranged on the wire with
the 'fcollapse 16k 24k' due to write-back caching.

fcollapse is implemented in cifs.ko as a SMB2 IOCTL(COPYCHUNK_WRITE) call
and it will fail serverside since the file is still 0b in size serverside
until the writes have been destaged.
To avoid this we must ensure that we destage any unwritten data to the
server before calling COPYCHUNK_WRITE.

Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1997373
Reported-by: Xiaoli Feng <xifeng@redhat.com>
Signed-off-by: Ronnie Sahlberg <lsahlber@redhat.com>
---
 fs/cifs/smb2ops.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/fs/cifs/smb2ops.c b/fs/cifs/smb2ops.c
index a67df8eaf702..d6aaeff4a30a 100644
--- a/fs/cifs/smb2ops.c
+++ b/fs/cifs/smb2ops.c
@@ -1858,9 +1858,17 @@ smb2_copychunk_range(const unsigned int xid,
 	int chunks_copied = 0;
 	bool chunk_sizes_updated = false;
 	ssize_t bytes_written, total_bytes_written = 0;
+	struct inode *inode;
 
 	pcchunk = kmalloc(sizeof(struct copychunk_ioctl), GFP_KERNEL);
 
+	/*
+	 * We need to flush all unwritten data before we can send the
+	 * copychunk ioctl to the server.
+	 */
+	inode = d_inode(trgtfile->dentry);
+	filemap_write_and_wait(inode->i_mapping);
+
 	if (pcchunk == NULL)
 		return -ENOMEM;
 
-- 
2.30.2

Re: [PATCH] cifs: destage any unwritten data to the server before calling copychunk_write

[Steve French]

merged into cifs-2.6.git for-next

On Wed, Apr 20, 2022 at 8:15 PM Ronnie Sahlberg <lsahlber@redhat.com> wrote:
>
> because the copychunk_write might cover a region of the file that has not yet
> been sent to the server and thus fail.
>
> A simple way to reproduce this is:
> truncate -s 0 /mnt/testfile; strace -f -o x -ttT xfs_io -i -f -c 'pwrite 0k 128k' -c 'fcollapse 16k 24k' /mnt/testfile
>
> the issue is that the 'pwrite 0k 128k' becomes rearranged on the wire with
> the 'fcollapse 16k 24k' due to write-back caching.
>
> fcollapse is implemented in cifs.ko as a SMB2 IOCTL(COPYCHUNK_WRITE) call
> and it will fail serverside since the file is still 0b in size serverside
> until the writes have been destaged.
> To avoid this we must ensure that we destage any unwritten data to the
> server before calling COPYCHUNK_WRITE.
>
> Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1997373
> Reported-by: Xiaoli Feng <xifeng@redhat.com>
> Signed-off-by: Ronnie Sahlberg <lsahlber@redhat.com>
> ---
>  fs/cifs/smb2ops.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
>
> diff --git a/fs/cifs/smb2ops.c b/fs/cifs/smb2ops.c
> index a67df8eaf702..d6aaeff4a30a 100644
> --- a/fs/cifs/smb2ops.c
> +++ b/fs/cifs/smb2ops.c
> @@ -1858,9 +1858,17 @@ smb2_copychunk_range(const unsigned int xid,
>         int chunks_copied = 0;
>         bool chunk_sizes_updated = false;
>         ssize_t bytes_written, total_bytes_written = 0;
> +       struct inode *inode;
>
>         pcchunk = kmalloc(sizeof(struct copychunk_ioctl), GFP_KERNEL);
>
> +       /*
> +        * We need to flush all unwritten data before we can send the
> +        * copychunk ioctl to the server.
> +        */
> +       inode = d_inode(trgtfile->dentry);
> +       filemap_write_and_wait(inode->i_mapping);
> +
>         if (pcchunk == NULL)
>                 return -ENOMEM;
>
> --
> 2.30.2
>


-- 
Thanks,

Steve