[PATCH 0/3] splice fixes

[PATCH 0/3] splice fixes

[Miklos Szeredi]

These patches fix bugs that affect real-world users of splice(2) and
sendfile(2) interfaces.

Not sure if some or all of them are appropriate for 2.6.35 and/or
-stable?

Thanks,
Miklos

[PATCH 1/3] splice: direct_splice_actor() should not use pos in sd

[Miklos Szeredi]

From: Changli Gao <xiaosuo@gmail.com>

direct_splice_actor() shouldn't use sd->pos, as sd->pos is for file reading,
file->f_pos should be used instead.

Signed-off-by: Changli Gao <xiaosuo@gmail.com>
Signed-off-by: Miklos Szeredi <mszeredi@suse.cz>
----
 fs/splice.c |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
Index: linux-2.6/fs/splice.c
===================================================================
--- linux-2.6.orig/fs/splice.c	2010-06-29 12:46:27.000000000 +0200
+++ linux-2.6/fs/splice.c	2010-06-29 12:46:28.000000000 +0200
@@ -1282,7 +1282,8 @@ static int direct_splice_actor(struct pi
 {
 	struct file *file = sd->u.file;
 
-	return do_splice_from(pipe, file, &sd->pos, sd->total_len, sd->flags);
+	return do_splice_from(pipe, file, &file->f_pos, sd->total_len,
+			      sd->flags);
 }
 
 /**

[PATCH 2/3] splice: check f_mode for seekable file

[Miklos Szeredi]

From: Changli Gao <xiaosuo@gmail.com>

check f_mode for seekable file

As a seekable file is allowed without a llseek function, so the old way isn't
work any more.

Signed-off-by: Changli Gao <xiaosuo@gmail.com>
Signed-off-by: Miklos Szeredi <mszeredi@suse.cz>
----
 fs/splice.c |    6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)
Index: linux-2.6/fs/splice.c
===================================================================
--- linux-2.6.orig/fs/splice.c	2010-06-29 12:46:28.000000000 +0200
+++ linux-2.6/fs/splice.c	2010-06-29 12:46:30.000000000 +0200
@@ -1372,8 +1372,7 @@ static long do_splice(struct file *in, l
 		if (off_in)
 			return -ESPIPE;
 		if (off_out) {
-			if (!out->f_op || !out->f_op->llseek ||
-			    out->f_op->llseek == no_llseek)
+			if (!(out->f_mode & FMODE_PWRITE))
 				return -EINVAL;
 			if (copy_from_user(&offset, off_out, sizeof(loff_t)))
 				return -EFAULT;
@@ -1393,8 +1392,7 @@ static long do_splice(struct file *in, l
 		if (off_out)
 			return -ESPIPE;
 		if (off_in) {
-			if (!in->f_op || !in->f_op->llseek ||
-			    in->f_op->llseek == no_llseek)
+			if (!(in->f_mode & FMODE_PREAD))
 				return -EINVAL;
 			if (copy_from_user(&offset, off_in, sizeof(loff_t)))
 				return -EFAULT;

[PATCH 3/3] splice: fix misuse of SPLICE_F_NONBLOCK

[Miklos Szeredi]

From: Miklos Szeredi <mszeredi@suse.cz>

SPLICE_F_NONBLOCK is clearly documented to only affect blocking on the
destination pipe and not on the source file.

In __generic_file_splice_read(), however, it returns EAGAIN if the
page is currently being read.

This makes it impossible to write an application that only wants
failure if the pipe buffer is full.  For example if the same process
is handling both ends of a pipe and isn't otherwise able to determine
whether a splice to the pipe will fit or not.

We could make the read non-blocking on O_NONBLOCK or some other splice
flag, but for now this is the simplest fix.

Signed-off-by: Miklos Szeredi <mszeredi@suse.cz>
---
 fs/splice.c |   12 +-----------
 1 file changed, 1 insertion(+), 11 deletions(-)

Index: linux-2.6/fs/splice.c
===================================================================
--- linux-2.6.orig/fs/splice.c	2010-06-29 12:46:30.000000000 +0200
+++ linux-2.6/fs/splice.c	2010-06-29 12:46:32.000000000 +0200
@@ -399,17 +399,7 @@ __generic_file_splice_read(struct file *
 		 * If the page isn't uptodate, we may need to start io on it
 		 */
 		if (!PageUptodate(page)) {
-			/*
-			 * If in nonblock mode then dont block on waiting
-			 * for an in-flight io page
-			 */
-			if (flags & SPLICE_F_NONBLOCK) {
-				if (!trylock_page(page)) {
-					error = -EAGAIN;
-					break;
-				}
-			} else
-				lock_page(page);
+			lock_page(page);
 
 			/*
 			 * Page was truncated, or invalidated by the

Re: [PATCH 0/3] splice fixes

[Jens Axboe]

On 2010-06-29 13:08, Miklos Szeredi wrote:
> These patches fix bugs that affect real-world users of splice(2) and
> sendfile(2) interfaces.
> 
> Not sure if some or all of them are appropriate for 2.6.35 and/or
> -stable?

I think that 1-2 should go into 2.6.35 directly, and 3 for .36 with
a stable backport as well. Agree?

-- 
Jens Axboe

Re: [PATCH 0/3] splice fixes

[Miklos Szeredi]

On Tue, 29 Jun 2010, Jens Axboe wrote:
> On 2010-06-29 13:08, Miklos Szeredi wrote:
> > These patches fix bugs that affect real-world users of splice(2) and
> > sendfile(2) interfaces.
> > 
> > Not sure if some or all of them are appropriate for 2.6.35 and/or
> > -stable?
> 
> I think that 1-2 should go into 2.6.35 directly, and 3 for .36 with
> a stable backport as well. Agree?

Yes, sounds good.

Thanks,
Miklos

Re: [PATCH 0/3] splice fixes

[Jens Axboe]

On 2010-06-29 14:05, Miklos Szeredi wrote:
> On Tue, 29 Jun 2010, Jens Axboe wrote:
>> On 2010-06-29 13:08, Miklos Szeredi wrote:
>>> These patches fix bugs that affect real-world users of splice(2) and
>>> sendfile(2) interfaces.
>>>
>>> Not sure if some or all of them are appropriate for 2.6.35 and/or
>>> -stable?
>>
>> I think that 1-2 should go into 2.6.35 directly, and 3 for .36 with
>> a stable backport as well. Agree?
> 
> Yes, sounds good.

Good, that is what I queued up. I haven't pushed out the updated
for-linus yet, since I asked Linus to pull it this morning.

-- 
Jens Axboe

Re: [PATCH 0/3] splice fixes

[Andrew Morton]

On Tue, 29 Jun 2010 14:05:52 +0200
Jens Axboe <axboe@kernel.dk> wrote:

> On 2010-06-29 14:05, Miklos Szeredi wrote:
> > On Tue, 29 Jun 2010, Jens Axboe wrote:
> >> On 2010-06-29 13:08, Miklos Szeredi wrote:
> >>> These patches fix bugs that affect real-world users of splice(2) and
> >>> sendfile(2) interfaces.
> >>>
> >>> Not sure if some or all of them are appropriate for 2.6.35 and/or
> >>> -stable?
> >>
> >> I think that 1-2 should go into 2.6.35 directly, and 3 for .36 with
> >> a stable backport as well. Agree?
> > 
> > Yes, sounds good.
> 
> Good, that is what I queued up. I haven't pushed out the updated
> for-linus yet, since I asked Linus to pull it this morning.
> 

According to Tim's report on lkml
(http://lkml.org/lkml/2010/6/25/280),
cc56f7de7f00d188c7c4da1e9861581853b9e92f caused a regression. 
cc56f7de7f00d188c7c4da1e9861581853b9e92f is present in 2.6.33 and in
2.6.34 hence the fix should be backported into -stable.

According to http://bugs.launchpad.net/bugs/588861, the patch "splice:
direct_splice_actor() should not use pos in sd" fixes the regression.

However this:

commit 4f9078afb4e083e033aafbbfabe729cb3832aa42
Author: Changli Gao <xiaosuo@gmail.com>
Date:   Tue Jun 29 13:09:18 2010 +0200

    splice: direct_splice_actor() should not use pos in sd

went into mainline without a cc:stable tag.

Re: [PATCH 0/3] splice fixes

[Jens Axboe]

On 2010-07-07 22:38, Andrew Morton wrote:
> On Tue, 29 Jun 2010 14:05:52 +0200
> Jens Axboe <axboe@kernel.dk> wrote:
> 
>> On 2010-06-29 14:05, Miklos Szeredi wrote:
>>> On Tue, 29 Jun 2010, Jens Axboe wrote:
>>>> On 2010-06-29 13:08, Miklos Szeredi wrote:
>>>>> These patches fix bugs that affect real-world users of splice(2) and
>>>>> sendfile(2) interfaces.
>>>>>
>>>>> Not sure if some or all of them are appropriate for 2.6.35 and/or
>>>>> -stable?
>>>>
>>>> I think that 1-2 should go into 2.6.35 directly, and 3 for .36 with
>>>> a stable backport as well. Agree?
>>>
>>> Yes, sounds good.
>>
>> Good, that is what I queued up. I haven't pushed out the updated
>> for-linus yet, since I asked Linus to pull it this morning.
>>
> 
> According to Tim's report on lkml
> (http://lkml.org/lkml/2010/6/25/280),
> cc56f7de7f00d188c7c4da1e9861581853b9e92f caused a regression. 
> cc56f7de7f00d188c7c4da1e9861581853b9e92f is present in 2.6.33 and in
> 2.6.34 hence the fix should be backported into -stable.
> 
> According to http://bugs.launchpad.net/bugs/588861, the patch "splice:
> direct_splice_actor() should not use pos in sd" fixes the regression.
> 
> However this:
> 
> commit 4f9078afb4e083e033aafbbfabe729cb3832aa42
> Author: Changli Gao <xiaosuo@gmail.com>
> Date:   Tue Jun 29 13:09:18 2010 +0200
> 
>     splice: direct_splice_actor() should not use pos in sd
> 
> went into mainline without a cc:stable tag.

I will send an email to stable@.

-- 
Jens Axboe

Re: [PATCH 0/3] splice fixes

[Jens Axboe]

On 2010-07-08 15:36, Jens Axboe wrote:
> On 2010-07-07 22:38, Andrew Morton wrote:
>> On Tue, 29 Jun 2010 14:05:52 +0200
>> Jens Axboe <axboe@kernel.dk> wrote:
>>
>>> On 2010-06-29 14:05, Miklos Szeredi wrote:
>>>> On Tue, 29 Jun 2010, Jens Axboe wrote:
>>>>> On 2010-06-29 13:08, Miklos Szeredi wrote:
>>>>>> These patches fix bugs that affect real-world users of splice(2) and
>>>>>> sendfile(2) interfaces.
>>>>>>
>>>>>> Not sure if some or all of them are appropriate for 2.6.35 and/or
>>>>>> -stable?
>>>>>
>>>>> I think that 1-2 should go into 2.6.35 directly, and 3 for .36 with
>>>>> a stable backport as well. Agree?
>>>>
>>>> Yes, sounds good.
>>>
>>> Good, that is what I queued up. I haven't pushed out the updated
>>> for-linus yet, since I asked Linus to pull it this morning.
>>>
>>
>> According to Tim's report on lkml
>> (http://lkml.org/lkml/2010/6/25/280),
>> cc56f7de7f00d188c7c4da1e9861581853b9e92f caused a regression. 
>> cc56f7de7f00d188c7c4da1e9861581853b9e92f is present in 2.6.33 and in
>> 2.6.34 hence the fix should be backported into -stable.
>>
>> According to http://bugs.launchpad.net/bugs/588861, the patch "splice:
>> direct_splice_actor() should not use pos in sd" fixes the regression.
>>
>> However this:
>>
>> commit 4f9078afb4e083e033aafbbfabe729cb3832aa42
>> Author: Changli Gao <xiaosuo@gmail.com>
>> Date:   Tue Jun 29 13:09:18 2010 +0200
>>
>>     splice: direct_splice_actor() should not use pos in sd
>>
>> went into mainline without a cc:stable tag.
> 
> I will send an email to stable@.

BTW, it's not in mainline yet. I know I forgot to mark it as stable,
but I can't tell them to include it before it's in mainline.


-- 
Jens Axboe


-- 
Jens Axboe

Re: [PATCH 0/3] splice fixes

[Andrew Morton]

On Thu, 08 Jul 2010 15:38:22 +0200 Jens Axboe <axboe@kernel.dk> wrote:

> >> However this:
> >>
> >> commit 4f9078afb4e083e033aafbbfabe729cb3832aa42
> >> Author: Changli Gao <xiaosuo@gmail.com>
> >> Date:   Tue Jun 29 13:09:18 2010 +0200
> >>
> >>     splice: direct_splice_actor() should not use pos in sd
> >>
> >> went into mainline without a cc:stable tag.
> > 
> > I will send an email to stable@.
> 
> BTW, it's not in mainline yet. I know I forgot to mark it as stable,
> but I can't tell them to include it before it's in mainline.

You could redo the changelog then.  But stable@kernel.org is cc'ed on
these emails and they're splendid chaps ;)

Re: [PATCH 0/3] splice fixes

[Jens Axboe]

On 08/07/10 19.25, Andrew Morton wrote:
> On Thu, 08 Jul 2010 15:38:22 +0200 Jens Axboe <axboe@kernel.dk> wrote:
> 
>>>> However this:
>>>>
>>>> commit 4f9078afb4e083e033aafbbfabe729cb3832aa42
>>>> Author: Changli Gao <xiaosuo@gmail.com>
>>>> Date:   Tue Jun 29 13:09:18 2010 +0200
>>>>
>>>>     splice: direct_splice_actor() should not use pos in sd
>>>>
>>>> went into mainline without a cc:stable tag.
>>>
>>> I will send an email to stable@.
>>
>> BTW, it's not in mainline yet. I know I forgot to mark it as stable,
>> but I can't tell them to include it before it's in mainline.
> 
> You could redo the changelog then.  But stable@kernel.org is cc'ed on
> these emails and they're splendid chaps ;)

But then the Linus monster ends up shouting at me, so I'll rather
take a little stable abuse :-)

What I've done in the past is just send stable@ a list of commits
once they are upstream. Less work for them. Best is remembering
the stable CC in the commit, but...

-- 
Jens Axboe