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>
