[185/222] tcp: fix MSG_SENDPAGE_NOTLAST logic

Submitted by Herton Ronaldo Krzesinski on Jan. 16, 2013, 3:56 p.m.

Details

Message ID 1358351822-7675-186-git-send-email-herton.krzesinski@canonical.com
State New
Headers show

Commit Message

Herton Ronaldo Krzesinski Jan. 16, 2013, 3:56 p.m.
3.5.7.3 -stable review patch.  If anyone has any objections, please let me know.

------------------

From: Eric Dumazet <edumazet@google.com>

commit ae62ca7b03217be5e74759dc6d7698c95df498b3 upstream.

commit 35f9c09fe9c72e (tcp: tcp_sendpages() should call tcp_push() once)
added an internal flag : MSG_SENDPAGE_NOTLAST meant to be set on all
frags but the last one for a splice() call.

The condition used to set the flag in pipe_to_sendpage() relied on
splice() user passing the exact number of bytes present in the pipe,
or a smaller one.

But some programs pass an arbitrary high value, and the test fails.

The effect of this bug is a lack of tcp_push() at the end of a
splice(pipe -> socket) call, and possibly very slow or erratic TCP
sessions.

We should both test sd->total_len and fact that another fragment
is in the pipe (pipe->nrbufs > 1)

Many thanks to Willy for providing very clear bug report, bisection
and test programs.

Reported-by: Willy Tarreau <w@1wt.eu>
Bisected-by: Willy Tarreau <w@1wt.eu>
Tested-by: Willy Tarreau <w@1wt.eu>
Signed-off-by: Eric Dumazet <edumazet@google.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
Signed-off-by: Herton Ronaldo Krzesinski <herton.krzesinski@canonical.com>
---
 fs/splice.c |    4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

Comments

Ben Hutchings Feb. 1, 2013, 5:40 p.m.
On Wed, 2013-01-16 at 13:56 -0200, Herton Ronaldo Krzesinski wrote:
> 3.5.7.3 -stable review patch.  If anyone has any objections, please let me know.
> 
> ------------------
> 
> From: Eric Dumazet <edumazet@google.com>
> 
> commit ae62ca7b03217be5e74759dc6d7698c95df498b3 upstream.
> 
> commit 35f9c09fe9c72e (tcp: tcp_sendpages() should call tcp_push() once)
> added an internal flag : MSG_SENDPAGE_NOTLAST meant to be set on all
> frags but the last one for a splice() call.
[...]

David, I think this is needed for 3.0 and 3.2 as well since
35f9c09fe9c72e was backported to them.  You previously included it in
updates for 3.4 and 3.7.

Ben.
David Miller Feb. 12, 2013, 7:36 a.m.
From: Ben Hutchings <ben@decadent.org.uk>
Date: Fri, 01 Feb 2013 18:40:57 +0100

> On Wed, 2013-01-16 at 13:56 -0200, Herton Ronaldo Krzesinski wrote:
>> 3.5.7.3 -stable review patch.  If anyone has any objections, please let me know.
>> 
>> ------------------
>> 
>> From: Eric Dumazet <edumazet@google.com>
>> 
>> commit ae62ca7b03217be5e74759dc6d7698c95df498b3 upstream.
>> 
>> commit 35f9c09fe9c72e (tcp: tcp_sendpages() should call tcp_push() once)
>> added an internal flag : MSG_SENDPAGE_NOTLAST meant to be set on all
>> frags but the last one for a splice() call.
> [...]
> 
> David, I think this is needed for 3.0 and 3.2 as well since
> 35f9c09fe9c72e was backported to them.  You previously included it in
> updates for 3.4 and 3.7.

Ok, queued up, thanks.

Patch hide | download patch | download mbox

diff --git a/fs/splice.c b/fs/splice.c
index 7bf08fa..6cb3acb 100644
--- a/fs/splice.c
+++ b/fs/splice.c
@@ -696,8 +696,10 @@  static int pipe_to_sendpage(struct pipe_inode_info *pipe,
 		return -EINVAL;
 
 	more = (sd->flags & SPLICE_F_MORE) ? MSG_MORE : 0;
-	if (sd->len < sd->total_len)
+
+	if (sd->len < sd->total_len && pipe->nrbufs > 1)
 		more |= MSG_SENDPAGE_NOTLAST;
+
 	return file->f_op->sendpage(file, buf->page, buf->offset,
 				    sd->len, &pos, more);
 }