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

[Jens Axboe <jens.axboe@oracle.com>]

On Mon, Jul 16 2007, OGAWA Hirofumi wrote:
> 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