On Thu, Nov 23, 2023 at 5:14 PM Christoph Hellwig <hch@xxxxxxxxxxxxx> wrote:
>
> On Thu, Nov 23, 2023 at 01:20:13PM +0200, Amir Goldstein wrote:
> > > - first obviously the name, based on the other functions it probably
> > > should be in the __kernel_* namespace unless I'm missing something.
> >
> > Not sure about the best name.
> > I just derived the name from do_iter_readv_writev(), which would be the
> > name of this helper if we split up do_iter_readv_writev() as you suggested.
>
> Well, I don't think do_iter_readv_writev is a particular good name
> even now, but certainly not once it is more than just a static helper
> with two callers.
>
> I can't say __kernel is an all that great name either, but it seems
> to match the existing ones.
>
> That being said - it just is a tiny wrapper anyway, what about just
> open coding it instead of bike shedding? See below for a patch,
> this actually seems pretty reasonable and very readable.
>
Heh! avoiding bike shedding is a worthy cause :)
It works for me.
I will let Christian decide if he prefers this over the existing
small helper with meaningless name.
> ---
> diff --git a/fs/splice.c b/fs/splice.c
> index d983d375ff1130..982a0872fa03e9 100644
> --- a/fs/splice.c
> +++ b/fs/splice.c
> @@ -684,6 +684,7 @@ iter_file_splice_write(struct pipe_inode_info *pipe, struct file *out,
>
> splice_from_pipe_begin(&sd);
> while (sd.total_len) {
> + struct kiocb kiocb;
> struct iov_iter from;
> unsigned int head, tail, mask;
> size_t left;
> @@ -733,7 +734,10 @@ iter_file_splice_write(struct pipe_inode_info *pipe, struct file *out,
> }
>
> iov_iter_bvec(&from, ITER_SOURCE, array, n, sd.total_len - left);
> - ret = vfs_iter_write(out, &from, &sd.pos, 0);
> + init_sync_kiocb(&kiocb, out);
> + kiocb.ki_pos = sd.pos;
> + ret = out->f_op->write_iter(&kiocb, &from);
> + sd.pos = kiocb.ki_pos;
> if (ret <= 0)
> break;
>
Are we open coding call_write_iter() now?
Is that a trend that I am not aware of?
Thanks,
Amir.
