| Message ID | 1357492255.6919.336.camel@edumazet-glaptop |
|---|---|
| State | RFC, archived |
| Delegated to: | David Miller |
| Headers | show |
On Sun, Jan 06, 2013 at 09:10:55AM -0800, Eric Dumazet wrote: > On Sun, 2013-01-06 at 17:44 +0100, Willy Tarreau wrote: > > On Sun, Jan 06, 2013 at 08:39:53AM -0800, Eric Dumazet wrote: > > > Hmm, I'll have to check if this really can be reverted without hurting > > > vmsplice() again. > > > > Looking at the code I've been wondering whether we shouldn't transform > > the condition to perform the push if we can't push more segments, but > > I don't know what to rely on. It would be something like this : > > > > if (copied && > > (!(flags & MSG_SENDPAGE_NOTLAST) || cant_push_more)) > > tcp_push(sk, flags, mss_now, tp->nonagle); > > Good point ! > > Maybe the following fix then ? > > > diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c > index 1ca2536..7ba0717 100644 > --- a/net/ipv4/tcp.c > +++ b/net/ipv4/tcp.c > @@ -941,8 +941,10 @@ out: > return copied; > > do_error: > - if (copied) > + if (copied) { > + flags &= ~MSG_SENDPAGE_NOTLAST; > goto out; > + } > out_err: > return sk_stream_error(sk, flags, err); > } Unfortunately it does not work any better, which means to me that we don't leave via this code path. I tried other tricks which failed too. I need to understand this part better before randomly fiddling with it. Willy -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Sun, 2013-01-06 at 18:35 +0100, Willy Tarreau wrote: > Unfortunately it does not work any better, which means to me > that we don't leave via this code path. I tried other tricks > which failed too. I need to understand this part better before > randomly fiddling with it. > OK, now I have your test program, I can work on a fix, dont worry ;) The MSG_SENDPAGE_NOTLAST logic needs to be tweaked. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Sun, 2013-01-06 at 10:39 -0800, Eric Dumazet wrote: > On Sun, 2013-01-06 at 18:35 +0100, Willy Tarreau wrote: > > > Unfortunately it does not work any better, which means to me > > that we don't leave via this code path. I tried other tricks > > which failed too. I need to understand this part better before > > randomly fiddling with it. > > > > OK, now I have your test program, I can work on a fix, dont worry ;) > > The MSG_SENDPAGE_NOTLAST logic needs to be tweaked. > (sd->len is usually 4096, which is expected, but sd->total_len value is huge in your case, so we always set the flag in fs/splice.c) -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> > (sd->len is usually 4096, which is expected, but sd->total_len value is > huge in your case, so we always set the flag in fs/splice.c) I am testing : if (sd->len < sd->total_len && pipe->nrbufs > 1) more |= MSG_SENDPAGE_NOTLAST; -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c index 1ca2536..7ba0717 100644 --- a/net/ipv4/tcp.c +++ b/net/ipv4/tcp.c @@ -941,8 +941,10 @@ out: return copied; do_error: - if (copied) + if (copied) { + flags &= ~MSG_SENDPAGE_NOTLAST; goto out; + } out_err: return sk_stream_error(sk, flags, err); }