Re: [PATCH 1/4] fs/splice: enhance direct pipe & splice for moving pages in kernel

[Ming Lei <ming.lei@redhat.com>]

On Sat, Feb 11, 2023 at 10:57:08AM -0800, Linus Torvalds wrote:
> On Sat, Feb 11, 2023 at 7:42 AM Ming Lei <ming.lei@redhat.com> wrote:
> >
> > +/*
> > + * Used by source/sink end only, don't touch them in generic
> > + * splice/pipe code. Set in source side, and check in sink
> > + * side
> > + */
> > +#define PIPE_BUF_PRIV_FLAG_MAY_READ    0x1000 /* sink can read from page */
> > +#define PIPE_BUF_PRIV_FLAG_MAY_WRITE   0x2000 /* sink can write to page */
> > +
> 
> So this sounds much more sane and understandable, but I have two worries:
> 
>  (a) what's the point of MAY_READ? A non-readable page sounds insane
> and wrong. All sinks expect to be able to read.

For example, it is one page which needs sink end to fill data, so
we needn't to zero it in source end every time, just for avoiding
leak kernel data if (unexpected)sink end simply tried to read from
the spliced page instead of writing data to page.

> 
>  (b) what about 'tee()' which duplicates a pipe buffer that has
> MAY_WRITE? Particularly one that may already have been *partially*
> given to something that thinks it can write to it?

The flags added is for kernel code(io_uring) over direct pipe,
and the two pipe buf flags won't be copied cross tee, cause the kernel
code just use spliced pages. (io_uring -> tee())

Also because of the added SPLICE_F_PRIV_FOR_READ[WRITE], pipe buffer
flags won't be copied over tee() too, because tee() can't pass the two
flags to device's ->splice_read().

We may need to document that the two pair flags should be used
together.

thanks,
Ming