updated all three patches with the acked-by and put in cifs-2.6.git for-next On Wed, Nov 29, 2023 at 4:28 PM Paulo Alcantara <pc@xxxxxxxxxxxxx> wrote: > > David Howells <dhowells@xxxxxxxxxx> writes: > > > Fix a number of issues in the cifs filesystem implementation of the > > copy_file_range() syscall in cifs_file_copychunk_range(). > > > > 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 copy request > > is issued, this will fail on the server with STATUS_INVALID_VIEW_SIZE > > (which gets translated to -EIO locally) if the copy 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. > > > > [!] Rather than moving the EOF, it might be better to split the copy > > range into a part to be copied and a part to be cleared with > > FSCTL_SET_ZERO_DATA. > > > > (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 copy. > > > > Thirdly, set i_size after doing the copychunk_range 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 generic/075 xfstest to fail. > > > > Fixes: 620d8745b35d ("Introduce cifs_copy_file_range()") > > Signed-off-by: David Howells <dhowells@xxxxxxxxxx> > > cc: Steve French <sfrench@xxxxxxxxx> > > cc: Paulo Alcantara <pc@xxxxxxxxxxxxx> > > cc: Shyam Prasad N <nspmangalore@xxxxxxxxx> > > cc: Rohith Surabattula <rohiths.msft@xxxxxxxxx> > > cc: Matthew Wilcox <willy@xxxxxxxxxxxxx> > > cc: Jeff Layton <jlayton@xxxxxxxxxx> > > cc: linux-cifs@xxxxxxxxxxxxxxx > > cc: linux-mm@xxxxxxxxx > > --- > > fs/smb/client/cifsfs.c | 80 ++++++++++++++++++++++++++++++++++++++++-- > > 1 file changed, 77 insertions(+), 3 deletions(-) > > Looks good, > > Acked-by: Paulo Alcantara (SUSE) <pc@xxxxxxxxxxxxx> > -- Thanks, Steve