Re: [PATCH] splice: fix wrong __splice_from_pipe() usage

Jens Axboe
2007-07-16T11:16:20Z

> Jens Axboe <jens.axboe@oracle.com> writes:
>
> > On Mon, Jul 16 2007, OGAWA Hirofumi wrote:
> >> Hi,
> >>
> >> I've noticed the nfsd read corruption by recent change. And this patch
> >> fixes the problem for me, is this right fix?
> >> --
> >> OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>
> >>
> >>
> >> __splice_from_pipe() is updating the sd->pos for the actor, but those
> >> functions are passing the sd of reader side directory. So, splice
> >> updates sd->pos twice.
> >>
> >> This fixes usage of __splice_from_pipe().
> >
> > For sendfile() usage, or the nfsd path that uses splice to send?
>
> nfsd_vfs_read() path.
>
> nfsd_vfs_read()
> splice_direct_to_actor()
> while(len) {
> do_splice_to() [update sd->pos]
> -> generic_file_splice_read() [read from sd->pos]
> nfsd_direct_splice_actor()
> -> __splice_from_pipe() [update sd->pos]
> }
>
> do_splice_to() updates sd->pos for read, and pass updated sd to
> __splice_from_pipe(), and __splice_from_pipe() updates sd->pos for
> write. So, next do_splice_to() read from wrong position.

Ah, I see. Copying the structure over would work, but it seems like a
big work-around for something that is essentially bad behaviour in the
splice core for direct splicing.

Can you check if this works for you?

diff --git a/fs/splice.c b/fs/splice.c
index 6c98286..5e5071a 100644
--- a/fs/splice.c
+++ b/fs/splice.c
@@ -1061,8 +1061,9 @@ ssize_t splice_direct_to_actor(struct file *in, struct splice_desc *sd,

while (len) {
size_t read_len;
+ loff_t pos = sd->pos;

- ret = do_splice_to(in, &sd->pos, pipe, len, flags);
+ ret = do_splice_to(in, &pos, pipe, len, flags);
if (unlikely(ret <= 0))
goto out_release;

@@ -1080,6 +1081,7 @@ ssize_t splice_direct_to_actor(struct file *in, struct splice_desc *sd,

bytes += ret;
len -= ret;
+ sd->pos = pos;

if (ret < read_len)
goto out_release;

--
Jens Axboe