Hi David! On Tue 27-08-24 15:46:55, David Howells wrote: > I'm looking at trying to fix cifs_file_copychunk_range(). Currently, it > invalidates the destination range, apart from a partial folio at either end > which will be flushed, and then tries the copy. But if the copy fails or can > only be partially completed (eg. ENOSPC), we lose any data in the destination > region, so I think it needs to be flushed and invalidated rather than just > being invalidated. > > Now, we have filemap_invalidate_inode() which I can use to flush back and > invalidate the folios under the invalidate_lock (thereby avoiding the need for > launder_folio). However, that doesn't prevent mmap from reinstating the > destination folios with modifications whilst the copy is ongoing the moment > the invalidate_lock is dropped. > > Question is: would it be reasonable to do the copy offload whilst holding the > invalidate_lock for the duration? FWIW yes, I'd expect cifs_file_copychunk_range() to take invalidate_lock on the target file to avoid possible races with page faults. We do it this already for similar operations such as reflink or various fallocate operations... Honza > > Thanks, > David > -- Jan Kara <jack@xxxxxxxx> SUSE Labs, CR
