| Message ID | 20200707174801.4162712-23-hch@lst.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [01/23] cachefiles: switch to kernel_write | expand |
On Tue, Jul 07, 2020 at 07:48:00PM +0200, Christoph Hellwig wrote: > If a file implements the ->read_iter method, the iter based splice read > works and is always preferred over the ->read based one. Use it by > default in do_splice_to and remove all the direct assignment of > generic_file_splice_read to file_operations. The worst problem here is the assumption that all ->read_iter() instances will take pipe-backed destination; that's _not_ automatically true. In particular, it's almost certainly false for tap_read_iter() (as well as tun_chr_read_iter() in IFF_VNET_HDR case). Other potentially interesting cases: cuse and hugetlbfs. But in any case, that blind assertion ("iter based splice read works") really needs to be backed by something.
On Thu, Jul 30, 2020 at 01:05:44AM +0100, Al Viro wrote: > On Tue, Jul 07, 2020 at 07:48:00PM +0200, Christoph Hellwig wrote: > > If a file implements the ->read_iter method, the iter based splice read > > works and is always preferred over the ->read based one. Use it by > > default in do_splice_to and remove all the direct assignment of > > generic_file_splice_read to file_operations. > > The worst problem here is the assumption that all ->read_iter() instances > will take pipe-backed destination; that's _not_ automatically true. > In particular, it's almost certainly false for tap_read_iter() (as > well as tun_chr_read_iter() in IFF_VNET_HDR case). > > Other potentially interesting cases: cuse and hugetlbfs. > > But in any case, that blind assertion ("iter based splice read works") > really needs to be backed by something. I think we need to fix that in the instances, as we really expect ->splice_read to just work instead of the caller knowing what could work and what might not.
On Thu, Jul 30, 2020 at 09:03:29AM +0200, Christoph Hellwig wrote: > On Thu, Jul 30, 2020 at 01:05:44AM +0100, Al Viro wrote: > > On Tue, Jul 07, 2020 at 07:48:00PM +0200, Christoph Hellwig wrote: > > > If a file implements the ->read_iter method, the iter based splice read > > > works and is always preferred over the ->read based one. Use it by > > > default in do_splice_to and remove all the direct assignment of > > > generic_file_splice_read to file_operations. > > > > The worst problem here is the assumption that all ->read_iter() instances > > will take pipe-backed destination; that's _not_ automatically true. > > In particular, it's almost certainly false for tap_read_iter() (as > > well as tun_chr_read_iter() in IFF_VNET_HDR case). > > > > Other potentially interesting cases: cuse and hugetlbfs. > > > > But in any case, that blind assertion ("iter based splice read works") > > really needs to be backed by something. > > I think we need to fix that in the instances, as we really expect > ->splice_read to just work instead of the caller knowing what could > work and what might not. Er... generic_file_splice_read() is a library helper; the decision to use is up to the filesystem/driver/protocol in question, and so's making sure it's not used with ->read_iter() that isn't fit for it. Note that we *do* have instances where we have different ->splice_read() (sometimes using generic_file_splice_read(), sometimes not) even though ->read_iter() is there. Your patch ignores those (thankfully), but commit message is rather misleading - it strongly implies that generic_file_splice_read() is *always* the right thing when ->read_iter() is there, not just that in such cases it makes a better fallback than default_file_splice_read(). And even the latter assumption is not obvious - AFAICS, we do have counterexamples. I'm not saying that e.g. tun/tap don't need fixing for other reasons and it's quite possible that they will become suitable for generic_file_splice_read() after that's done. But I'm really unhappy about the implied change of generic_file_splice_read() role; if nothing else, commit message should be very clear that if you have ->read_iter() and generic_file_splice_read() won't do the right thing, you MUST provide ->splice_read() of your own. Probably worth Documentation/filesystem/porting entry as well. Alternatively, if you really want to change the role of that thing, we need to go through all instances that are *not* generic_file_splice_read() and see what's going on in those. Starting with the sockets. The list right now is: fs/fuse/dev.c:2263: .splice_read = fuse_dev_splice_read, fs/overlayfs/file.c:786: .splice_read = ovl_splice_read, net/socket.c:164: .splice_read = sock_splice_read, kernel/relay.c:1331: .splice_read = relay_file_splice_read, kernel/trace/trace.c:7081: .splice_read = tracing_splice_read_pipe, kernel/trace/trace.c:7149: .splice_read = tracing_buffers_splice_read, kernel/trace/trace.c:7712: .splice_read = tracing_buffers_splice_read, The first 3 have ->read_iter(); the rest (kernel/* stuff) doesn't. Socket case uses generic_file_splice_read() unless the protocol provides an override; SMC, TCP, TCPv6, AF_UNIX STREAM and KCM SEQPACKET do that. I hadn't looked into the socket side of things for 5 years or so, so I'd have to dig the notes out first. It wasn't pleasant...
On Thu, Jul 30, 2020 at 04:08:26PM +0100, Al Viro wrote: > > I think we need to fix that in the instances, as we really expect > > ->splice_read to just work instead of the caller knowing what could > > work and what might not. > > Er... generic_file_splice_read() is a library helper; the decision to use > is up to the filesystem/driver/protocol in question, and so's making sure > it's not used with ->read_iter() that isn't fit for it. Yes, but.. The problem is that while right now generic_file_splice_read is the only user of ITER_PIPE there is absolutely not guarantee that it remains the only user. Having ->read_iter instances lingering that can't deal with it is at best a mine field waiting for victims. Fortunately I think the fix is pretty easy - remove the special pipe zero copy optimization from copy_page_to_iter, and just have the callers actually want it because they have pagecache or similar refcountable pages use it explicitly for the ITER_PIPE case. That gives us a safe default with an opt-in into the optimized variant. I'm currently auditing all the users of for how it is used and that looks pretty promising. > Note that we *do* have instances where we have different ->splice_read() > (sometimes using generic_file_splice_read(), sometimes not) even though > ->read_iter() is there. > > Your patch ignores those (thankfully), but commit message is rather > misleading - it strongly implies that generic_file_splice_read() is > *always* the right thing when ->read_iter() is there, not just that > in such cases it makes a better fallback than default_file_splice_read(). I don't think it always is right. Not without a major audit and more work at least.
On Thu, Jul 30, 2020 at 05:20:46PM +0200, Christoph Hellwig wrote: > Fortunately I think the fix is pretty easy - remove the special pipe > zero copy optimization from copy_page_to_iter, and just have the > callers actually want it because they have pagecache or similar > refcountable pages use it explicitly for the ITER_PIPE case. That gives > us a safe default with an opt-in into the optimized variant. I'm > currently auditing all the users of for how it is used and that looks > pretty promising. Huh? What does that have to do with anything?
On Thu, Jul 30, 2020 at 05:17:01PM +0100, Al Viro wrote: > On Thu, Jul 30, 2020 at 05:20:46PM +0200, Christoph Hellwig wrote: > > > Fortunately I think the fix is pretty easy - remove the special pipe > > zero copy optimization from copy_page_to_iter, and just have the > > callers actually want it because they have pagecache or similar > > refcountable pages use it explicitly for the ITER_PIPE case. That gives > > us a safe default with an opt-in into the optimized variant. I'm > > currently auditing all the users of for how it is used and that looks > > pretty promising. > > Huh? What does that have to do with anything? FWIW, none of the dubious (and outright broken) cases I've found go anywhere near that. And it definitely won't help tun/tap...
On Thu, Jul 30, 2020 at 05:22:19PM +0100, Al Viro wrote: > FWIW, none of the dubious (and outright broken) cases I've found go anywhere > near that. And it definitely won't help tun/tap... Then I'm missing something obvious - what is the problem with tun/tap?
diff --git a/fs/adfs/file.c b/fs/adfs/file.c index 754afb14a6ff74..b089b91c1870ae 100644 --- a/fs/adfs/file.c +++ b/fs/adfs/file.c @@ -28,7 +28,6 @@ const struct file_operations adfs_file_operations = { .mmap = generic_file_mmap, .fsync = generic_file_fsync, .write_iter = generic_file_write_iter, - .splice_read = generic_file_splice_read, }; const struct inode_operations adfs_file_inode_operations = { diff --git a/fs/affs/file.c b/fs/affs/file.c index a85817f54483f7..7d51cc2e3dabfa 100644 --- a/fs/affs/file.c +++ b/fs/affs/file.c @@ -975,7 +975,6 @@ const struct file_operations affs_file_operations = { .open = affs_file_open, .release = affs_file_release, .fsync = affs_file_fsync, - .splice_read = generic_file_splice_read, }; const struct inode_operations affs_file_inode_operations = { diff --git a/fs/afs/file.c b/fs/afs/file.c index 6f6ed1605cfe30..2476f10383fbdd 100644 --- a/fs/afs/file.c +++ b/fs/afs/file.c @@ -32,7 +32,6 @@ const struct file_operations afs_file_operations = { .read_iter = generic_file_read_iter, .write_iter = afs_file_write, .mmap = afs_file_mmap, - .splice_read = generic_file_splice_read, .fsync = afs_fsync, .lock = afs_lock, .flock = afs_flock, diff --git a/fs/bfs/file.c b/fs/bfs/file.c index 0dceefc54b48ab..39088cc7492308 100644 --- a/fs/bfs/file.c +++ b/fs/bfs/file.c @@ -27,7 +27,6 @@ const struct file_operations bfs_file_operations = { .read_iter = generic_file_read_iter, .write_iter = generic_file_write_iter, .mmap = generic_file_mmap, - .splice_read = generic_file_splice_read, }; static int bfs_move_block(unsigned long from, unsigned long to, diff --git a/fs/block_dev.c b/fs/block_dev.c index 0ae656e022fd57..0aa66a6075eb11 100644 --- a/fs/block_dev.c +++ b/fs/block_dev.c @@ -2160,7 +2160,6 @@ const struct file_operations def_blk_fops = { #ifdef CONFIG_COMPAT .compat_ioctl = compat_blkdev_ioctl, #endif - .splice_read = generic_file_splice_read, .splice_write = iter_file_splice_write, .fallocate = blkdev_fallocate, }; diff --git a/fs/btrfs/file.c b/fs/btrfs/file.c index 2520605afc256e..322cc65902d107 100644 --- a/fs/btrfs/file.c +++ b/fs/btrfs/file.c @@ -3507,7 +3507,6 @@ static int btrfs_file_open(struct inode *inode, struct file *filp) const struct file_operations btrfs_file_operations = { .llseek = btrfs_file_llseek, .read_iter = generic_file_read_iter, - .splice_read = generic_file_splice_read, .write_iter = btrfs_file_write_iter, .mmap = btrfs_file_mmap, .open = btrfs_file_open, diff --git a/fs/ceph/file.c b/fs/ceph/file.c index 160644ddaeed70..e28c27751e6b3b 100644 --- a/fs/ceph/file.c +++ b/fs/ceph/file.c @@ -2507,7 +2507,6 @@ const struct file_operations ceph_file_fops = { .fsync = ceph_fsync, .lock = ceph_lock, .flock = ceph_flock, - .splice_read = generic_file_splice_read, .splice_write = iter_file_splice_write, .unlocked_ioctl = ceph_ioctl, .compat_ioctl = compat_ptr_ioctl, diff --git a/fs/cifs/cifsfs.c b/fs/cifs/cifsfs.c index 0fb99d25e8a8a0..74da1dfe08c6fa 100644 --- a/fs/cifs/cifsfs.c +++ b/fs/cifs/cifsfs.c @@ -1235,7 +1235,6 @@ const struct file_operations cifs_file_ops = { .fsync = cifs_fsync, .flush = cifs_flush, .mmap = cifs_file_mmap, - .splice_read = generic_file_splice_read, .splice_write = iter_file_splice_write, .llseek = cifs_llseek, .unlocked_ioctl = cifs_ioctl, @@ -1255,7 +1254,6 @@ const struct file_operations cifs_file_strict_ops = { .fsync = cifs_strict_fsync, .flush = cifs_flush, .mmap = cifs_file_strict_mmap, - .splice_read = generic_file_splice_read, .splice_write = iter_file_splice_write, .llseek = cifs_llseek, .unlocked_ioctl = cifs_ioctl, @@ -1275,7 +1273,6 @@ const struct file_operations cifs_file_direct_ops = { .fsync = cifs_fsync, .flush = cifs_flush, .mmap = cifs_file_mmap, - .splice_read = generic_file_splice_read, .splice_write = iter_file_splice_write, .unlocked_ioctl = cifs_ioctl, .copy_file_range = cifs_copy_file_range, @@ -1293,7 +1290,6 @@ const struct file_operations cifs_file_nobrl_ops = { .fsync = cifs_fsync, .flush = cifs_flush, .mmap = cifs_file_mmap, - .splice_read = generic_file_splice_read, .splice_write = iter_file_splice_write, .llseek = cifs_llseek, .unlocked_ioctl = cifs_ioctl, @@ -1311,7 +1307,6 @@ const struct file_operations cifs_file_strict_nobrl_ops = { .fsync = cifs_strict_fsync, .flush = cifs_flush, .mmap = cifs_file_strict_mmap, - .splice_read = generic_file_splice_read, .splice_write = iter_file_splice_write, .llseek = cifs_llseek, .unlocked_ioctl = cifs_ioctl, @@ -1329,7 +1324,6 @@ const struct file_operations cifs_file_direct_nobrl_ops = { .fsync = cifs_fsync, .flush = cifs_flush, .mmap = cifs_file_mmap, - .splice_read = generic_file_splice_read, .splice_write = iter_file_splice_write, .unlocked_ioctl = cifs_ioctl, .copy_file_range = cifs_copy_file_range, diff --git a/fs/coda/file.c b/fs/coda/file.c index 128d63df5bfb62..8dd438f2c09fe2 100644 --- a/fs/coda/file.c +++ b/fs/coda/file.c @@ -301,5 +301,4 @@ const struct file_operations coda_file_operations = { .open = coda_open, .release = coda_release, .fsync = coda_fsync, - .splice_read = generic_file_splice_read, }; diff --git a/fs/cramfs/inode.c b/fs/cramfs/inode.c index 912308600d393d..0645c1af27c07d 100644 --- a/fs/cramfs/inode.c +++ b/fs/cramfs/inode.c @@ -485,7 +485,6 @@ static unsigned int cramfs_physmem_mmap_capabilities(struct file *file) static const struct file_operations cramfs_physmem_fops = { .llseek = generic_file_llseek, .read_iter = generic_file_read_iter, - .splice_read = generic_file_splice_read, .mmap = cramfs_physmem_mmap, #ifndef CONFIG_MMU .get_unmapped_area = cramfs_physmem_get_unmapped_area, diff --git a/fs/ecryptfs/file.c b/fs/ecryptfs/file.c index 5fb45d865ce511..03210a02fe6c00 100644 --- a/fs/ecryptfs/file.c +++ b/fs/ecryptfs/file.c @@ -420,5 +420,4 @@ const struct file_operations ecryptfs_main_fops = { .release = ecryptfs_release, .fsync = ecryptfs_fsync, .fasync = ecryptfs_fasync, - .splice_read = generic_file_splice_read, }; diff --git a/fs/exfat/file.c b/fs/exfat/file.c index 3b7fea465fd41e..8fe5df8a9ccbca 100644 --- a/fs/exfat/file.c +++ b/fs/exfat/file.c @@ -369,7 +369,6 @@ const struct file_operations exfat_file_operations = { .write_iter = generic_file_write_iter, .mmap = generic_file_mmap, .fsync = exfat_file_fsync, - .splice_read = generic_file_splice_read, .splice_write = iter_file_splice_write, }; diff --git a/fs/ext2/file.c b/fs/ext2/file.c index 60378ddf1424b0..1c0828e0198440 100644 --- a/fs/ext2/file.c +++ b/fs/ext2/file.c @@ -191,7 +191,6 @@ const struct file_operations ext2_file_operations = { .release = ext2_release_file, .fsync = ext2_fsync, .get_unmapped_area = thp_get_unmapped_area, - .splice_read = generic_file_splice_read, .splice_write = iter_file_splice_write, }; diff --git a/fs/ext4/file.c b/fs/ext4/file.c index 2a01e31a032c4c..f5dc9a4e0937d1 100644 --- a/fs/ext4/file.c +++ b/fs/ext4/file.c @@ -896,7 +896,6 @@ const struct file_operations ext4_file_operations = { .release = ext4_release_file, .fsync = ext4_sync_file, .get_unmapped_area = thp_get_unmapped_area, - .splice_read = generic_file_splice_read, .splice_write = iter_file_splice_write, .fallocate = ext4_fallocate, }; diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c index 3268f8dd59bbaf..6b34caf13b5668 100644 --- a/fs/f2fs/file.c +++ b/fs/f2fs/file.c @@ -4033,6 +4033,5 @@ const struct file_operations f2fs_file_operations = { #ifdef CONFIG_COMPAT .compat_ioctl = f2fs_compat_ioctl, #endif - .splice_read = generic_file_splice_read, .splice_write = iter_file_splice_write, }; diff --git a/fs/fat/file.c b/fs/fat/file.c index 42134c58c87e19..e7a0342ccfe1f0 100644 --- a/fs/fat/file.c +++ b/fs/fat/file.c @@ -208,7 +208,6 @@ const struct file_operations fat_file_operations = { .unlocked_ioctl = fat_generic_ioctl, .compat_ioctl = compat_ptr_ioctl, .fsync = fat_file_fsync, - .splice_read = generic_file_splice_read, .splice_write = iter_file_splice_write, .fallocate = fat_fallocate, }; diff --git a/fs/fuse/file.c b/fs/fuse/file.c index e573b0cd2737dc..a404e147bb2cf7 100644 --- a/fs/fuse/file.c +++ b/fs/fuse/file.c @@ -3382,7 +3382,6 @@ static const struct file_operations fuse_file_operations = { .fsync = fuse_fsync, .lock = fuse_file_lock, .flock = fuse_file_flock, - .splice_read = generic_file_splice_read, .splice_write = iter_file_splice_write, .unlocked_ioctl = fuse_file_ioctl, .compat_ioctl = fuse_file_compat_ioctl, diff --git a/fs/gfs2/file.c b/fs/gfs2/file.c index fe305e4bfd3734..d23babc0c292b8 100644 --- a/fs/gfs2/file.c +++ b/fs/gfs2/file.c @@ -1323,7 +1323,6 @@ const struct file_operations gfs2_file_fops = { .fsync = gfs2_fsync, .lock = gfs2_lock, .flock = gfs2_flock, - .splice_read = generic_file_splice_read, .splice_write = gfs2_file_splice_write, .setlease = simple_nosetlease, .fallocate = gfs2_fallocate, @@ -1354,7 +1353,6 @@ const struct file_operations gfs2_file_fops_nolock = { .open = gfs2_open, .release = gfs2_release, .fsync = gfs2_fsync, - .splice_read = generic_file_splice_read, .splice_write = gfs2_file_splice_write, .setlease = generic_setlease, .fallocate = gfs2_fallocate, diff --git a/fs/hfs/inode.c b/fs/hfs/inode.c index 2f224b98ee94a6..6181d3818e17c0 100644 --- a/fs/hfs/inode.c +++ b/fs/hfs/inode.c @@ -682,7 +682,6 @@ static const struct file_operations hfs_file_operations = { .read_iter = generic_file_read_iter, .write_iter = generic_file_write_iter, .mmap = generic_file_mmap, - .splice_read = generic_file_splice_read, .fsync = hfs_file_fsync, .open = hfs_file_open, .release = hfs_file_release, diff --git a/fs/hfsplus/inode.c b/fs/hfsplus/inode.c index e3da9e96b83578..7bd61ba08fbc9e 100644 --- a/fs/hfsplus/inode.c +++ b/fs/hfsplus/inode.c @@ -358,7 +358,6 @@ static const struct file_operations hfsplus_file_operations = { .read_iter = generic_file_read_iter, .write_iter = generic_file_write_iter, .mmap = generic_file_mmap, - .splice_read = generic_file_splice_read, .fsync = hfsplus_file_fsync, .open = hfsplus_file_open, .release = hfsplus_file_release, diff --git a/fs/hostfs/hostfs_kern.c b/fs/hostfs/hostfs_kern.c index c070c0d8e3e977..c6453e3768294c 100644 --- a/fs/hostfs/hostfs_kern.c +++ b/fs/hostfs/hostfs_kern.c @@ -379,7 +379,6 @@ static int hostfs_fsync(struct file *file, loff_t start, loff_t end, static const struct file_operations hostfs_file_fops = { .llseek = generic_file_llseek, - .splice_read = generic_file_splice_read, .read_iter = generic_file_read_iter, .write_iter = generic_file_write_iter, .mmap = generic_file_mmap, diff --git a/fs/hpfs/file.c b/fs/hpfs/file.c index 077c25128eb741..53951f29f25e50 100644 --- a/fs/hpfs/file.c +++ b/fs/hpfs/file.c @@ -213,7 +213,6 @@ const struct file_operations hpfs_file_ops = .mmap = generic_file_mmap, .release = hpfs_file_release, .fsync = hpfs_file_fsync, - .splice_read = generic_file_splice_read, .unlocked_ioctl = hpfs_ioctl, .compat_ioctl = compat_ptr_ioctl, }; diff --git a/fs/jffs2/file.c b/fs/jffs2/file.c index f8fb89b10227ce..6e986a99669779 100644 --- a/fs/jffs2/file.c +++ b/fs/jffs2/file.c @@ -56,7 +56,6 @@ const struct file_operations jffs2_file_operations = .unlocked_ioctl=jffs2_ioctl, .mmap = generic_file_readonly_mmap, .fsync = jffs2_fsync, - .splice_read = generic_file_splice_read, }; /* jffs2_file_inode_operations */ diff --git a/fs/jfs/file.c b/fs/jfs/file.c index 930d2701f2062b..fb209673943697 100644 --- a/fs/jfs/file.c +++ b/fs/jfs/file.c @@ -141,7 +141,6 @@ const struct file_operations jfs_file_operations = { .read_iter = generic_file_read_iter, .write_iter = generic_file_write_iter, .mmap = generic_file_mmap, - .splice_read = generic_file_splice_read, .splice_write = iter_file_splice_write, .fsync = jfs_fsync, .release = jfs_release, diff --git a/fs/minix/file.c b/fs/minix/file.c index c50b0a20fcd9c1..e787789b43fa95 100644 --- a/fs/minix/file.c +++ b/fs/minix/file.c @@ -19,7 +19,6 @@ const struct file_operations minix_file_operations = { .write_iter = generic_file_write_iter, .mmap = generic_file_mmap, .fsync = generic_file_fsync, - .splice_read = generic_file_splice_read, }; static int minix_setattr(struct dentry *dentry, struct iattr *attr) diff --git a/fs/nfs/file.c b/fs/nfs/file.c index ccd6c1637b270b..8ba06f6c3ec5af 100644 --- a/fs/nfs/file.c +++ b/fs/nfs/file.c @@ -851,7 +851,6 @@ const struct file_operations nfs_file_operations = { .fsync = nfs_file_fsync, .lock = nfs_lock, .flock = nfs_flock, - .splice_read = generic_file_splice_read, .splice_write = iter_file_splice_write, .check_flags = nfs_check_flags, .setlease = simple_nosetlease, diff --git a/fs/nfs/nfs4file.c b/fs/nfs/nfs4file.c index 8e5d6223ddd359..3e3793cb217ec1 100644 --- a/fs/nfs/nfs4file.c +++ b/fs/nfs/nfs4file.c @@ -415,7 +415,6 @@ const struct file_operations nfs4_file_operations = { .fsync = nfs_file_fsync, .lock = nfs_lock, .flock = nfs_flock, - .splice_read = generic_file_splice_read, .splice_write = iter_file_splice_write, .check_flags = nfs_check_flags, .setlease = simple_nosetlease, diff --git a/fs/nilfs2/file.c b/fs/nilfs2/file.c index 64bc81363c6cc0..cb3269c52dabc7 100644 --- a/fs/nilfs2/file.c +++ b/fs/nilfs2/file.c @@ -140,7 +140,6 @@ const struct file_operations nilfs_file_operations = { .open = generic_file_open, /* .release = nilfs_release_file, */ .fsync = nilfs_sync_file, - .splice_read = generic_file_splice_read, }; const struct inode_operations nilfs_file_inode_operations = { diff --git a/fs/ntfs/file.c b/fs/ntfs/file.c index f42967b738eb67..8c1759e9185dd7 100644 --- a/fs/ntfs/file.c +++ b/fs/ntfs/file.c @@ -2012,7 +2012,6 @@ const struct file_operations ntfs_file_ops = { #endif /* NTFS_RW */ .mmap = generic_file_mmap, .open = ntfs_file_open, - .splice_read = generic_file_splice_read, }; const struct inode_operations ntfs_file_inode_ops = { diff --git a/fs/ocfs2/file.c b/fs/ocfs2/file.c index 85979e2214b39d..86069cae29047e 100644 --- a/fs/ocfs2/file.c +++ b/fs/ocfs2/file.c @@ -2671,7 +2671,6 @@ const struct file_operations ocfs2_fops = { #endif .lock = ocfs2_lock, .flock = ocfs2_flock, - .splice_read = generic_file_splice_read, .splice_write = iter_file_splice_write, .fallocate = ocfs2_fallocate, .remap_file_range = ocfs2_remap_file_range, @@ -2717,7 +2716,6 @@ const struct file_operations ocfs2_fops_no_plocks = { .compat_ioctl = ocfs2_compat_ioctl, #endif .flock = ocfs2_flock, - .splice_read = generic_file_splice_read, .splice_write = iter_file_splice_write, .fallocate = ocfs2_fallocate, .remap_file_range = ocfs2_remap_file_range, diff --git a/fs/omfs/file.c b/fs/omfs/file.c index d7b5f09d298c9d..0dddc6c644c10c 100644 --- a/fs/omfs/file.c +++ b/fs/omfs/file.c @@ -340,7 +340,6 @@ const struct file_operations omfs_file_operations = { .write_iter = generic_file_write_iter, .mmap = generic_file_mmap, .fsync = generic_file_fsync, - .splice_read = generic_file_splice_read, }; static int omfs_setattr(struct dentry *dentry, struct iattr *attr) diff --git a/fs/ramfs/file-mmu.c b/fs/ramfs/file-mmu.c index 12af0490322f9d..d1e76267e9c323 100644 --- a/fs/ramfs/file-mmu.c +++ b/fs/ramfs/file-mmu.c @@ -43,7 +43,6 @@ const struct file_operations ramfs_file_operations = { .write_iter = generic_file_write_iter, .mmap = generic_file_mmap, .fsync = noop_fsync, - .splice_read = generic_file_splice_read, .splice_write = iter_file_splice_write, .llseek = generic_file_llseek, .get_unmapped_area = ramfs_mmu_get_unmapped_area, diff --git a/fs/ramfs/file-nommu.c b/fs/ramfs/file-nommu.c index 41469545495608..9336086e60fefd 100644 --- a/fs/ramfs/file-nommu.c +++ b/fs/ramfs/file-nommu.c @@ -43,7 +43,6 @@ const struct file_operations ramfs_file_operations = { .read_iter = generic_file_read_iter, .write_iter = generic_file_write_iter, .fsync = noop_fsync, - .splice_read = generic_file_splice_read, .splice_write = iter_file_splice_write, .llseek = generic_file_llseek, }; diff --git a/fs/read_write.c b/fs/read_write.c index 11c55547cfc9d6..8d8113ae8561e6 100644 --- a/fs/read_write.c +++ b/fs/read_write.c @@ -29,7 +29,6 @@ const struct file_operations generic_ro_fops = { .llseek = generic_file_llseek, .read_iter = generic_file_read_iter, .mmap = generic_file_readonly_mmap, - .splice_read = generic_file_splice_read, }; EXPORT_SYMBOL(generic_ro_fops); diff --git a/fs/reiserfs/file.c b/fs/reiserfs/file.c index 0b641ae694f123..4f71c3aca2b8c1 100644 --- a/fs/reiserfs/file.c +++ b/fs/reiserfs/file.c @@ -247,7 +247,6 @@ const struct file_operations reiserfs_file_operations = { .fsync = reiserfs_sync_file, .read_iter = generic_file_read_iter, .write_iter = generic_file_write_iter, - .splice_read = generic_file_splice_read, .splice_write = iter_file_splice_write, .llseek = generic_file_llseek, }; diff --git a/fs/romfs/mmap-nommu.c b/fs/romfs/mmap-nommu.c index 2c4a23113fb5f2..f37e27fa0c1084 100644 --- a/fs/romfs/mmap-nommu.c +++ b/fs/romfs/mmap-nommu.c @@ -78,7 +78,6 @@ static unsigned romfs_mmap_capabilities(struct file *file) const struct file_operations romfs_ro_fops = { .llseek = generic_file_llseek, .read_iter = generic_file_read_iter, - .splice_read = generic_file_splice_read, .mmap = romfs_mmap, .get_unmapped_area = romfs_get_unmapped_area, .mmap_capabilities = romfs_mmap_capabilities, diff --git a/fs/splice.c b/fs/splice.c index d7c8a7c4db07ff..52485158023778 100644 --- a/fs/splice.c +++ b/fs/splice.c @@ -868,6 +868,8 @@ static long do_splice_to(struct file *in, loff_t *ppos, if (in->f_op->splice_read) return in->f_op->splice_read(in, ppos, pipe, len, flags); + if (in->f_op->read_iter) + return generic_file_splice_read(in, ppos, pipe, len, flags); return default_file_splice_read(in, ppos, pipe, len, flags); } diff --git a/fs/sysv/file.c b/fs/sysv/file.c index 45fc79a18594f1..d023922c0e44c7 100644 --- a/fs/sysv/file.c +++ b/fs/sysv/file.c @@ -26,7 +26,6 @@ const struct file_operations sysv_file_operations = { .write_iter = generic_file_write_iter, .mmap = generic_file_mmap, .fsync = generic_file_fsync, - .splice_read = generic_file_splice_read, }; static int sysv_setattr(struct dentry *dentry, struct iattr *attr) diff --git a/fs/ubifs/file.c b/fs/ubifs/file.c index 49fe062ce45ec2..a3af46b3950811 100644 --- a/fs/ubifs/file.c +++ b/fs/ubifs/file.c @@ -1668,7 +1668,6 @@ const struct file_operations ubifs_file_operations = { .mmap = ubifs_file_mmap, .fsync = ubifs_fsync, .unlocked_ioctl = ubifs_ioctl, - .splice_read = generic_file_splice_read, .splice_write = iter_file_splice_write, .open = fscrypt_file_open, #ifdef CONFIG_COMPAT diff --git a/fs/udf/file.c b/fs/udf/file.c index 628941a6b79afb..6c796ef2bd8331 100644 --- a/fs/udf/file.c +++ b/fs/udf/file.c @@ -250,7 +250,6 @@ const struct file_operations udf_file_operations = { .write_iter = udf_file_write_iter, .release = udf_release_file, .fsync = generic_file_fsync, - .splice_read = generic_file_splice_read, .llseek = generic_file_llseek, }; diff --git a/fs/ufs/file.c b/fs/ufs/file.c index 7e087581be7e0c..7a6dbb32d22cb6 100644 --- a/fs/ufs/file.c +++ b/fs/ufs/file.c @@ -41,5 +41,4 @@ const struct file_operations ufs_file_operations = { .mmap = generic_file_mmap, .open = generic_file_open, .fsync = generic_file_fsync, - .splice_read = generic_file_splice_read, }; diff --git a/fs/vboxsf/file.c b/fs/vboxsf/file.c index c4ab5996d97a83..30671e1226dbed 100644 --- a/fs/vboxsf/file.c +++ b/fs/vboxsf/file.c @@ -200,7 +200,6 @@ const struct file_operations vboxsf_reg_fops = { .open = vboxsf_file_open, .release = vboxsf_file_release, .fsync = noop_fsync, - .splice_read = generic_file_splice_read, }; const struct inode_operations vboxsf_reg_iops = { diff --git a/fs/xfs/xfs_file.c b/fs/xfs/xfs_file.c index 00db81eac80d6c..964bc733e765a4 100644 --- a/fs/xfs/xfs_file.c +++ b/fs/xfs/xfs_file.c @@ -1297,7 +1297,6 @@ const struct file_operations xfs_file_operations = { .llseek = xfs_file_llseek, .read_iter = xfs_file_read_iter, .write_iter = xfs_file_write_iter, - .splice_read = generic_file_splice_read, .splice_write = iter_file_splice_write, .iopoll = iomap_dio_iopoll, .unlocked_ioctl = xfs_file_ioctl, diff --git a/fs/zonefs/super.c b/fs/zonefs/super.c index 07bc42d62673ce..d9f5fbeb55062e 100644 --- a/fs/zonefs/super.c +++ b/fs/zonefs/super.c @@ -869,7 +869,6 @@ static const struct file_operations zonefs_file_operations = { .llseek = zonefs_file_llseek, .read_iter = zonefs_file_read_iter, .write_iter = zonefs_file_write_iter, - .splice_read = generic_file_splice_read, .splice_write = iter_file_splice_write, .iopoll = iomap_dio_iopoll, }; diff --git a/mm/shmem.c b/mm/shmem.c index a0dbe62f8042e7..f019ff50084403 100644 --- a/mm/shmem.c +++ b/mm/shmem.c @@ -3756,7 +3756,6 @@ static const struct file_operations shmem_file_operations = { .read_iter = shmem_file_read_iter, .write_iter = generic_file_write_iter, .fsync = noop_fsync, - .splice_read = generic_file_splice_read, .splice_write = iter_file_splice_write, .fallocate = shmem_fallocate, #endif
If a file implements the ->read_iter method, the iter based splice read works and is always preferred over the ->read based one. Use it by default in do_splice_to and remove all the direct assignment of generic_file_splice_read to file_operations. Signed-off-by: Christoph Hellwig <hch@lst.de> --- fs/adfs/file.c | 1 - fs/affs/file.c | 1 - fs/afs/file.c | 1 - fs/bfs/file.c | 1 - fs/block_dev.c | 1 - fs/btrfs/file.c | 1 - fs/ceph/file.c | 1 - fs/cifs/cifsfs.c | 6 ------ fs/coda/file.c | 1 - fs/cramfs/inode.c | 1 - fs/ecryptfs/file.c | 1 - fs/exfat/file.c | 1 - fs/ext2/file.c | 1 - fs/ext4/file.c | 1 - fs/f2fs/file.c | 1 - fs/fat/file.c | 1 - fs/fuse/file.c | 1 - fs/gfs2/file.c | 2 -- fs/hfs/inode.c | 1 - fs/hfsplus/inode.c | 1 - fs/hostfs/hostfs_kern.c | 1 - fs/hpfs/file.c | 1 - fs/jffs2/file.c | 1 - fs/jfs/file.c | 1 - fs/minix/file.c | 1 - fs/nfs/file.c | 1 - fs/nfs/nfs4file.c | 1 - fs/nilfs2/file.c | 1 - fs/ntfs/file.c | 1 - fs/ocfs2/file.c | 2 -- fs/omfs/file.c | 1 - fs/ramfs/file-mmu.c | 1 - fs/ramfs/file-nommu.c | 1 - fs/read_write.c | 1 - fs/reiserfs/file.c | 1 - fs/romfs/mmap-nommu.c | 1 - fs/splice.c | 2 ++ fs/sysv/file.c | 1 - fs/ubifs/file.c | 1 - fs/udf/file.c | 1 - fs/ufs/file.c | 1 - fs/vboxsf/file.c | 1 - fs/xfs/xfs_file.c | 1 - fs/zonefs/super.c | 1 - mm/shmem.c | 1 - 45 files changed, 2 insertions(+), 51 deletions(-)