Re: [PATCH 1/3] splice: default to iter based versions implicitly

[Al Viro <viro@ZenIV.linux.org.uk>]

On Sun, May 28, 2017 at 12:48:05PM +0300, Christoph Hellwig wrote:
> If a file_operations instance has the read or write iter methods we should
> use the iter based splice code by default.  We still allow to overide it
> with explicit ->read_iter and ->write_iter methods, but few of them are
> left, and there is a non-zero chance that we could get rid of them in the
> future.

->read_iter() side:  you are changing behaviour for the following bunch:

arch/s390/hypfs/inode.c:437:    .read_iter      = hypfs_read_iter,
drivers/char/mem.c:798: .read_iter      = read_iter_null,
drivers/char/mem.c:813: .read_iter      = read_iter_zero,
drivers/char/mem.c:824: .read_iter      = read_iter_zero,
drivers/char/raw.c:286: .read_iter      = blkdev_read_iter,
drivers/net/tap.c:1134: .read_iter      = tap_read_iter,
drivers/net/tun.c:2423: .read_iter  = tun_chr_read_iter,
drivers/usb/gadget/function/f_fs.c:1254:        .read_iter =    ffs_epfile_read_iter,
drivers/usb/gadget/legacy/inode.c:704:  .read_iter =    ep_read_iter,
drivers/vhost/net.c:1249:       .read_iter      = vhost_net_chr_read_iter,
fs/9p/vfs_file.c:641:   .read_iter = generic_file_read_iter,
fs/9p/vfs_file.c:652:   .read_iter = generic_file_read_iter,
fs/9p/vfs_file.c:664:   .read_iter = v9fs_file_read_iter,
fs/9p/vfs_file.c:675:   .read_iter = v9fs_file_read_iter,
fs/9p/vfs_file.c:687:   .read_iter = v9fs_mmap_file_read_iter,
fs/9p/vfs_file.c:698:   .read_iter = v9fs_mmap_file_read_iter,
fs/fuse/cuse.c:180:     .read_iter              = cuse_read_iter,
fs/fuse/file.c:3027:    .read_iter      = fuse_direct_read_iter,
fs/hugetlbfs/inode.c:980:       .read_iter              = hugetlbfs_read_iter,
fs/ncpfs/file.c:248:    .read_iter      = ncp_file_read_iter,
fs/orangefs/file.c:743: .read_iter      = orangefs_file_read_iter,
sound/core/pcm_native.c:3696:           .read_iter =            snd_pcm_readv,

All of them get generic_file_splice_read() for ->splice_read() (OK, modulo
your renaming).  For some of them it's clearly the right thing to do;
however, that depends upon their ->read_iter() not doing anything that
wouldn't work on ITER_PIPE ones.  Which deserves an explanation in commit
message, if nothing else.

ncpfs: OK
hugetlbfs: probably, but I would suggest testing that.
orangefs: OK
9p: can get nasty.  It depends upon p9_client_read() being OK with ITER_PIPE.
It probably shouldn't be too hard to get working, but at the moment I won't
bet that it handles full pipe correctly, for example.  At the very least, it
needs testing; the interesting setup is virtio with non-caching 9p over it.
fuse direct/cuse: *probably* should work (unless we already have bugs there,
which is not impossible).  Needs testing.
drivers/char/mem: OK
drivers/char/raw: OK
hypfs: OK
vhost: dubious, but then so's read() there
gadget: might be OK, needs testing.
tun/tap: 100% *NOT* OK.  ITER_PIPE won't work for those - not with
skipping holes in the buffer.  This is hopelessly broken; leaving
a chunk of userland buffer unmodified is one thing, but doing that
to page we are putting into pipe?  No go.  At the very least, it
would need to turn that into iov_iter_zero(), followed by serious
testing.
sound: fuck, no.