Skip to content

Commit 91d1dfa

Browse files
dhowellsSteve French
authored andcommitted
cifs: Fix FALLOC_FL_ZERO_RANGE to preflush buffered part of target region
Under certain conditions, the range to be cleared by FALLOC_FL_ZERO_RANGE may only be buffered locally and not yet have been flushed to the server. For example: xfs_io -f -t -c "pwrite -S 0x41 0 4k" \ -c "pwrite -S 0x42 4k 4k" \ -c "fzero 0 4k" \ -c "pread -v 0 8k" /xfstest.test/foo will write two 4KiB blocks of data, which get buffered in the pagecache, and then fallocate() is used to clear the first 4KiB block on the server - but we don't flush the data first, which means the EOF position on the server is wrong, and so the FSCTL_SET_ZERO_DATA RPC fails (and xfs_io ignores the error), but then when we try to read it, we see the old data. Fix this by preflushing any part of the target region that above the server's idea of the EOF position to force the server to update its EOF position. Note, however, that we don't want to simply expand the file by moving the EOF before doing the FSCTL_SET_ZERO_DATA[*] because someone else might see the zeroed region or if the RPC fails we then have to try to clean it up or risk getting corruption. [*] And we have to move the EOF first otherwise FSCTL_SET_ZERO_DATA won't do what we want. This fixes the generic/008 xfstest. [!] Note: A better way to do this might be to split the operation into two parts: we only do FSCTL_SET_ZERO_DATA for the part of the range below the server's EOF and then, if that worked, invalidate the buffered pages for the part above the range. Fixes: 6b69040 ("cifs/smb3: Fix data inconsistent when zero file range") Signed-off-by: David Howells <dhowells@redhat.com> cc: Steve French <stfrench@microsoft.com> cc: Zhang Xiaoxu <zhangxiaoxu5@huawei.com> cc: Pavel Shilovsky <pshilov@microsoft.com> cc: Paulo Alcantara <pc@manguebit.com> cc: Shyam Prasad N <nspmangalore@gmail.com> cc: Rohith Surabattula <rohiths.msft@gmail.com> cc: Jeff Layton <jlayton@kernel.org> cc: linux-cifs@vger.kernel.org cc: linux-mm@kvack.org Signed-off-by: Steve French <stfrench@microsoft.com>
1 parent 8101d6e commit 91d1dfa

File tree

1 file changed

+14
-2
lines changed

1 file changed

+14
-2
lines changed

fs/smb/client/smb2ops.c

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3237,13 +3237,15 @@ static long smb3_zero_data(struct file *file, struct cifs_tcon *tcon,
32373237
}
32383238

32393239
static long smb3_zero_range(struct file *file, struct cifs_tcon *tcon,
3240-
loff_t offset, loff_t len, bool keep_size)
3240+
unsigned long long offset, unsigned long long len,
3241+
bool keep_size)
32413242
{
32423243
struct cifs_ses *ses = tcon->ses;
32433244
struct inode *inode = file_inode(file);
32443245
struct cifsInodeInfo *cifsi = CIFS_I(inode);
32453246
struct cifsFileInfo *cfile = file->private_data;
3246-
unsigned long long new_size;
3247+
struct netfs_inode *ictx = netfs_inode(inode);
3248+
unsigned long long i_size, new_size, remote_size;
32473249
long rc;
32483250
unsigned int xid;
32493251

@@ -3255,6 +3257,16 @@ static long smb3_zero_range(struct file *file, struct cifs_tcon *tcon,
32553257
inode_lock(inode);
32563258
filemap_invalidate_lock(inode->i_mapping);
32573259

3260+
i_size = i_size_read(inode);
3261+
remote_size = ictx->remote_i_size;
3262+
if (offset + len >= remote_size && offset < i_size) {
3263+
unsigned long long top = umin(offset + len, i_size);
3264+
3265+
rc = filemap_write_and_wait_range(inode->i_mapping, offset, top - 1);
3266+
if (rc < 0)
3267+
goto zero_range_exit;
3268+
}
3269+
32583270
/*
32593271
* We zero the range through ioctl, so we need remove the page caches
32603272
* first, otherwise the data may be inconsistent with the server.

0 commit comments

Comments
 (0)