On Thu, Oct 25, 2018 at 7:00 PM Olga Kornievskaia <olga.kornievskaia@xxxxxxxxx> wrote: > > On Thu, Oct 25, 2018 at 11:58 AM Olga Kornievskaia > <olga.kornievskaia@xxxxxxxxx> wrote: > > > > On Thu, Oct 25, 2018 at 12:59 AM Amir Goldstein <amir73il@xxxxxxxxx> wrote: > > > > > > On Wed, Oct 24, 2018 at 10:59 PM Olga Kornievskaia > > > <olga.kornievskaia@xxxxxxxxx> wrote: > > > > > > > [...] > > > > It feels like folks are now ok with either the check being in the > > > > drivers or doing the check in the VFS layer. > > > > > > > > I'm picking the choice of not doing the check in the VFS layer because > > > > it allows for do_splice_direct() by any caller. > > > > > > I'm sorry, but this reasoning in flawed and this is not the reason that > > > Matthew promoted not doing same fs type check in vfs. > > > > I stated the reason why I picked to do the check at the driver layer. > > Looking at your version of the sb type check to be only applied to the > > copy_file_range indeed makes my argument invalid. I was under the > > impression that sb type check was being proposed as a standalone check > > (just like the sb check was standalone). Thus, yes I didn't completely > > understand what you proposed. > > > > > You did not understand the option that I was promoting to begin with. > > > What I suggested was: > > > > > > 1. Remove current same sb check in beginning of vfs_copy_file_range() > > > 2. Check sb && ->clone_file_range > > > 3. Check same sb type && ->copy_file_range > > > 4. Cross fs do_splice_direct() > > > > > > It's fine that you chose not to check for same fs type in VFS before calling > > > copy_file_range() method, but still requires an ACK from Al that he agrees > > > with passing in file * of another filesystem on the interface. > > > > Al, can you please provide a final decision as to which way you would > > prefer for this to be done. > > > > > > I'm about to submit > > > > the new version of the patches (this time I will include the NFS patch > > > > series). We can continue with the discussion on the new version. > > > > > > > > I have added checks for the CIFS and OverlayFS to be consistent with > > > > the previous behavior -- no cross-device copy_offload, I assume if and > > > > when those file systems are ready to make use of it they'll remove the > > > > check. > > > > > > > > > > Actually overlayfs code is "ready" for cross sb copy, but neither nfs nor > > > cifs are supported as upper file system, so it doesn't matter much. > > > > So then the commit statement is still true. When overlayfs will have > > upper file systems that do support it, this check can be removed. > > Ops sorry I meant them as questions. Do you feel that commit message > needs to be changed then? Commit message is fine by me. Thanks, Amir.
