Skip to content

Commit 358bd5f

Browse files
dhowellsgregkh
authored andcommitted
cifs: Fix flushing, invalidation and file size with FICLONE
commit c54fc3a upstream. Fix a number of issues in the cifs filesystem implementation of the FICLONE ioctl in cifs_remap_file_range(). This is analogous to the previously fixed bug in cifs_file_copychunk_range() and can share the helper functions. Firstly, the invalidation of the destination range is handled incorrectly: We shouldn't just invalidate the whole file as dirty data in the file may get lost and we can't just call truncate_inode_pages_range() to invalidate the destination range as that will erase parts of a partial folio at each end whilst invalidating and discarding all the folios in the middle. We need to force all the folios covering the range to be reloaded, but we mustn't lose dirty data in them that's not in the destination range. Further, we shouldn't simply round out the range to PAGE_SIZE at each end as cifs should move to support multipage folios. Secondly, there's an issue whereby a write may have extended the file locally, but not have been written back yet. This can leaves the local idea of the EOF at a later point than the server's EOF. If a clone request is issued, this will fail on the server with STATUS_INVALID_VIEW_SIZE (which gets translated to -EIO locally) if the clone source extends past the server's EOF. Fix this by: (0) Flush the source region (already done). The flush does nothing and the EOF isn't moved if the source region has no dirty data. (1) Move the EOF to the end of the source region if it isn't already at least at this point. If we can't do this, for instance if the server doesn't support it, just flush the entire source file. (2) Find the folio (if present) at each end of the range, flushing it and increasing the region-to-be-invalidated to cover those in their entirety. (3) Fully discard all the folios covering the range as we want them to be reloaded. (4) Then perform the extent duplication. Thirdly, set i_size after doing the duplicate_extents operation as this value may be used by various things internally. stat() hides the issue because setting ->time to 0 causes cifs_getatr() to revalidate the attributes. These were causing the cifs/001 xfstest to fail. Fixes: 04b38d6 ("vfs: pull btrfs clone API to vfs layer") Signed-off-by: David Howells <dhowells@redhat.com> Cc: stable@vger.kernel.org cc: Christoph Hellwig <hch@lst.de> cc: Paulo Alcantara <pc@manguebit.com> cc: Shyam Prasad N <nspmangalore@gmail.com> cc: Rohith Surabattula <rohiths.msft@gmail.com> cc: Matthew Wilcox <willy@infradead.org> cc: Jeff Layton <jlayton@kernel.org> cc: linux-cifs@vger.kernel.org cc: linux-mm@kvack.org Signed-off-by: David Howells <dhowells@redhat.com> Signed-off-by: Steve French <stfrench@microsoft.com> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
1 parent 18b02e4 commit 358bd5f

1 file changed

Lines changed: 57 additions & 11 deletions

File tree

fs/smb/client/cifsfs.c

Lines changed: 57 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1263,9 +1263,12 @@ static loff_t cifs_remap_file_range(struct file *src_file, loff_t off,
12631263
{
12641264
struct inode *src_inode = file_inode(src_file);
12651265
struct inode *target_inode = file_inode(dst_file);
1266+
struct cifsInodeInfo *src_cifsi = CIFS_I(src_inode);
1267+
struct cifsInodeInfo *target_cifsi = CIFS_I(target_inode);
12661268
struct cifsFileInfo *smb_file_src = src_file->private_data;
1267-
struct cifsFileInfo *smb_file_target;
1268-
struct cifs_tcon *target_tcon;
1269+
struct cifsFileInfo *smb_file_target = dst_file->private_data;
1270+
struct cifs_tcon *target_tcon, *src_tcon;
1271+
unsigned long long destend, fstart, fend, new_size;
12691272
unsigned int xid;
12701273
int rc;
12711274

@@ -1278,13 +1281,13 @@ static loff_t cifs_remap_file_range(struct file *src_file, loff_t off,
12781281

12791282
xid = get_xid();
12801283

1281-
if (!src_file->private_data || !dst_file->private_data) {
1284+
if (!smb_file_src || !smb_file_target) {
12821285
rc = -EBADF;
12831286
cifs_dbg(VFS, "missing cifsFileInfo on copy range src file\n");
12841287
goto out;
12851288
}
12861289

1287-
smb_file_target = dst_file->private_data;
1290+
src_tcon = tlink_tcon(smb_file_src->tlink);
12881291
target_tcon = tlink_tcon(smb_file_target->tlink);
12891292

12901293
/*
@@ -1297,20 +1300,63 @@ static loff_t cifs_remap_file_range(struct file *src_file, loff_t off,
12971300
if (len == 0)
12981301
len = src_inode->i_size - off;
12991302

1300-
cifs_dbg(FYI, "about to flush pages\n");
1301-
/* should we flush first and last page first */
1302-
truncate_inode_pages_range(&target_inode->i_data, destoff,
1303-
PAGE_ALIGN(destoff + len)-1);
1303+
cifs_dbg(FYI, "clone range\n");
1304+
1305+
/* Flush the source buffer */
1306+
rc = filemap_write_and_wait_range(src_inode->i_mapping, off,
1307+
off + len - 1);
1308+
if (rc)
1309+
goto unlock;
1310+
1311+
/* The server-side copy will fail if the source crosses the EOF marker.
1312+
* Advance the EOF marker after the flush above to the end of the range
1313+
* if it's short of that.
1314+
*/
1315+
if (src_cifsi->netfs.remote_i_size < off + len) {
1316+
rc = cifs_precopy_set_eof(src_inode, src_cifsi, src_tcon, xid, off + len);
1317+
if (rc < 0)
1318+
goto unlock;
1319+
}
1320+
1321+
new_size = destoff + len;
1322+
destend = destoff + len - 1;
13041323

1305-
if (target_tcon->ses->server->ops->duplicate_extents)
1324+
/* Flush the folios at either end of the destination range to prevent
1325+
* accidental loss of dirty data outside of the range.
1326+
*/
1327+
fstart = destoff;
1328+
fend = destend;
1329+
1330+
rc = cifs_flush_folio(target_inode, destoff, &fstart, &fend, true);
1331+
if (rc)
1332+
goto unlock;
1333+
rc = cifs_flush_folio(target_inode, destend, &fstart, &fend, false);
1334+
if (rc)
1335+
goto unlock;
1336+
1337+
/* Discard all the folios that overlap the destination region. */
1338+
cifs_dbg(FYI, "about to discard pages %llx-%llx\n", fstart, fend);
1339+
truncate_inode_pages_range(&target_inode->i_data, fstart, fend);
1340+
1341+
fscache_invalidate(cifs_inode_cookie(target_inode), NULL,
1342+
i_size_read(target_inode), 0);
1343+
1344+
rc = -EOPNOTSUPP;
1345+
if (target_tcon->ses->server->ops->duplicate_extents) {
13061346
rc = target_tcon->ses->server->ops->duplicate_extents(xid,
13071347
smb_file_src, smb_file_target, off, len, destoff);
1308-
else
1309-
rc = -EOPNOTSUPP;
1348+
if (rc == 0 && new_size > i_size_read(target_inode)) {
1349+
truncate_setsize(target_inode, new_size);
1350+
netfs_resize_file(&target_cifsi->netfs, new_size);
1351+
fscache_resize_cookie(cifs_inode_cookie(target_inode),
1352+
new_size);
1353+
}
1354+
}
13101355

13111356
/* force revalidate of size and timestamps of target file now
13121357
that target is updated on the server */
13131358
CIFS_I(target_inode)->time = 0;
1359+
unlock:
13141360
/* although unlocking in the reverse order from locking is not
13151361
strictly necessary here it is a little cleaner to be consistent */
13161362
unlock_two_nondirectories(src_inode, target_inode);

0 commit comments

Comments
 (0)