mbox series

[v3,0/5] iov_iter: Adjust styling/location of new splice functions

Message ID 20230214083710.2547248-1-dhowells@redhat.com (mailing list archive)
Headers show
Series iov_iter: Adjust styling/location of new splice functions | expand

Message

David Howells Feb. 14, 2023, 8:37 a.m. UTC
Hi Jens, Al, Christoph,

Here are patches to make some changes that Christoph requested[1] to the
new generic file splice functions that I implemented[2].  Apart from one
functional change, they just altering the styling and move one of the
functions to a different file:

 (1) Rename the main functions:

	generic_file_buffered_splice_read() -> filemap_splice_read()
	generic_file_direct_splice_read()   -> direct_splice_read()

 (2) Abstract out the calculation of the location of the head pipe buffer
     into a helper function in linux/pipe_fs_i.h.

 (3) Use init_sync_kiocb() in filemap_splice_read().

     This is where the functional change is.  Some kiocb fields are then
     filled in where they were set to 0 before, including setting ki_flags
     from f_iocb_flags.

 (4) Move filemap_splice_read() to mm/filemap.c.  filemap_get_pages() can
     then be made static again.

 (5) Fix splice-read for a number of filesystems that don't provide a
     ->read_folio() op and for which filemap_get_pages() cannot be used.

I've pushed the patches here also:

	https://git.kernel.org/pub/scm/linux/kernel/git/dhowells/linux-fs.git/log/?h=iov-extract-3

I've also updated worked the changes into the commits on my iov-extract
branch if that would be preferable, though that means Jens would need to
update his for-6.3/iov-extract again.

David

Link: https://lore.kernel.org/r/Y+n0n2UE8BQa/OwW@infradead.org/ [1]
Link: https://lore.kernel.org/r/20230207171305.3716974-1-dhowells@redhat.com/ [2]

Changes
=======
ver #3)
 - Fix filesystems/drivers that don't have ->read_folio().

ver #2)
 - Don't attempt to filter IOCB_* flags in filemap_splice_read().

Link: https://lore.kernel.org/r/20230213134619.2198965-1-dhowells@redhat.com/ # v1

David Howells (5):
  splice: Rename new splice functions
  splice: Provide pipe_head_buf() helper
  splice: Use init_sync_kiocb() in filemap_splice_read()
  splice: Move filemap_read_splice() to mm/filemap.c
  shmem, overlayfs, coda, tty, proc, kernfs, random: Fix splice-read

 drivers/char/random.c     |   4 +-
 drivers/tty/tty_io.c      |   4 +-
 fs/coda/file.c            |  36 +++++++++-
 fs/kernfs/file.c          |   2 +-
 fs/overlayfs/file.c       |  36 +++++++++-
 fs/proc/inode.c           |   4 +-
 fs/proc/proc_sysctl.c     |   2 +-
 fs/proc_namespace.c       |   6 +-
 fs/splice.c               | 146 ++------------------------------------
 include/linux/fs.h        |   6 ++
 include/linux/pagemap.h   |   2 -
 include/linux/pipe_fs_i.h |  20 ++++++
 mm/filemap.c              | 136 +++++++++++++++++++++++++++++++++--
 mm/internal.h             |   6 ++
 mm/shmem.c                | 124 +++++++++++++++++++++++++++++++-
 15 files changed, 373 insertions(+), 161 deletions(-)

Comments

Jens Axboe Feb. 14, 2023, 3:36 p.m. UTC | #1
On 2/14/23 2:07?AM, David Hildenbrand wrote:
> On 14.02.23 09:37, David Howells wrote:
>> Hi Jens, Al, Christoph,
>>
>> Here are patches to make some changes that Christoph requested[1] to the
>> new generic file splice functions that I implemented[2].  Apart from one
>> functional change, they just altering the styling and move one of the
>> functions to a different file:
>>
>>   (1) Rename the main functions:
>>
>>     generic_file_buffered_splice_read() -> filemap_splice_read()
>>     generic_file_direct_splice_read()   -> direct_splice_read()
>>
>>   (2) Abstract out the calculation of the location of the head pipe buffer
>>       into a helper function in linux/pipe_fs_i.h.
>>
>>   (3) Use init_sync_kiocb() in filemap_splice_read().
>>
>>       This is where the functional change is.  Some kiocb fields are then
>>       filled in where they were set to 0 before, including setting ki_flags
>>       from f_iocb_flags.
>>
>>   (4) Move filemap_splice_read() to mm/filemap.c.  filemap_get_pages() can
>>       then be made static again.
>>
>>   (5) Fix splice-read for a number of filesystems that don't provide a
>>       ->read_folio() op and for which filemap_get_pages() cannot be used.
>>
>> I've pushed the patches here also:
>>
>>     https://git.kernel.org/pub/scm/linux/kernel/git/dhowells/linux-fs.git/log/?h=iov-extract-3
>>
>> I've also updated worked the changes into the commits on my iov-extract
>> branch if that would be preferable, though that means Jens would need to
>> update his for-6.3/iov-extract again.
>>
>> David
>>
>> Link: https://lore.kernel.org/r/Y+n0n2UE8BQa/OwW@infradead.org/ [1]
>> Link: https://lore.kernel.org/r/20230207171305.3716974-1-dhowells@redhat.com/ [2]
>>
>> Changes
>> =======
>> ver #3)
>>   - Fix filesystems/drivers that don't have ->read_folio().
>>
>> ver #2)
>>   - Don't attempt to filter IOCB_* flags in filemap_splice_read().
>>
>> Link: https://lore.kernel.org/r/20230213134619.2198965-1-dhowells@redhat.com/ # v1
>>
> 
> You ignored my RB's :(
> 
> .. but unrelated, what's the plan with this now? As Jens mentioned, it
> might be better to wait for 6.4 for the full series, in which case
> folding this series into the other series would be better.

That is indeed the question, and unanswered so far... Let's turn it into
one clean series, and get it stuffed into for-next and most likely
target 6.4 for inclusion at this point.
David Howells Feb. 14, 2023, 3:49 p.m. UTC | #2
Jens Axboe <axboe@kernel.dk> wrote:

> That is indeed the question, and unanswered so far... Let's turn it into
> one clean series, and get it stuffed into for-next and most likely
> target 6.4 for inclusion at this point.

I was waiting to see if the patch worked for Daniel (which it does) and
Guenter (no answer yet) before answering.  It appears to fix shmem - I've
tested it with:

	dd if=/dev/zero of=/tmp/sparse count=1 seek=401 bs=4096
	just-splice /tmp/sparse 11234000 | sha1sum

where just-splice.c is attached (note that piping the output into another
program is important to make the splice work).

Meanwhile, I'm working on working the changes into my patchset at appropriate
points.

David
---
#define _GNU_SOURCE 
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <unistd.h>
#include <fcntl.h>
#include <sys/sendfile.h>
#include <sys/wait.h>

static char *prog;

int main(int argc, char *argv[])
{
        unsigned int iflags = 0;
        ssize_t spliced, remain;
        int in, out;

        prog = argv[0];
        if (argc > 1 && strcmp(argv[1], "-d") == 0) {
                iflags |= O_DIRECT;
                argv++;
                argc--;
        }

        if (argc != 3 || !argv[1][0] || !argv[2][0]) {
                fprintf(stderr, "Usage: %s <file> <amount>\n", prog);
                exit(2);
        }

        in = open(argv[1], O_RDONLY | O_NOFOLLOW | iflags);
        if (in < 0) {
                perror("open");
                exit(1);
        }

        remain = strtoul(argv[2], NULL, 0);
        while (remain > 0) {
                spliced = splice(in, NULL, 1, NULL, remain, 0);
                if (spliced < 0) {
                        perror("splice");
                        exit(1);
                }
                if (spliced == 0)
                        break;
                remain -= spliced;
        }

        exit(0);
}