On Tue, Mar 22, 2022 at 1:55 PM Josef Bacik <josef@xxxxxxxxxxxxxx> wrote: > > This more fine-grained checking is handled by generic_remap_file_range_prep() to > make sure we don't try to dedup a directory or pipe or some other nonsense. Yeah, that does seem to take care of the obvious cases, and requires that both files be regular files at least. I'm still not a huge fan of how we use the 'f_op->remap_file_range' of the source file, without really checking that the destination file is ok with remap_file_range. They end up _superficially_ very similar, yes, but I can point to filesystems that use different f_op's for different files. And some of those depend on - wait for it - how the filesystem was mounted. See for example cifs: if (cifs_sb->mnt_cifs_flags & CIFS_MOUNT_NO_BRL) file->f_op = &cifs_file_direct_nobrl_ops; else file->f_op = &cifs_file_direct_ops; so I'm just thinking "what about doing remap_file_range between two regular files that act differently - either due to mount options or other details". In that cifs example, read_iter and write_iter are different. Yes, copy/remap_file_range uses the same function pointer, but it still worries me about copying from a mount to another if there might be different semantics for IO between them. I think in this cifs case, the superblock ends up being the same, so the mnt_cifs_flags end up being the same, and the above is not actually a real issue. But conceptually I could imagine cases where that wasn't the case - or even cases like /proc that have fundamentally different file operations for different files) Linus