* Re: [PATCH] pipe_to_sendpage: Ensure that MSG_MORE is set if we set MSG_SENDPAGE_NOTLAST
2013-11-25 1:25 ` Eric Dumazet
@ 2013-11-25 1:40 ` Shawn Landden
2013-11-25 2:04 ` [PATCH] Commit 35f9c09fe (tcp: tcp_sendpages() should call tcp_push() once) added an internal flag MSG_SENDPAGE_NOTLAST, similar to MSG_MORE Shawn Landden
` (2 subsequent siblings)
3 siblings, 0 replies; 15+ messages in thread
From: Shawn Landden @ 2013-11-25 1:40 UTC (permalink / raw)
To: Eric Dumazet
Cc: Richard Weinberger, Linux Kernel Mailing List, linux-fsdevel,
viro, linux-crypto, netdev, Herbert Xu, Tom Herbert,
David S. Miller, stable
On Sun, Nov 24, 2013 at 5:25 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> On Mon, 2013-11-25 at 00:42 +0100, Richard Weinberger wrote:
>> Commit 35f9c09fe (tcp: tcp_sendpages() should call tcp_push() once)
>> added an internal flag MSG_SENDPAGE_NOTLAST.
>> We have to ensure that MSG_MORE is also set if we set MSG_SENDPAGE_NOTLAST.
>> Otherwise users that check against MSG_MORE will not see it.
>>
>> This fixes sendfile() on AF_ALG.
>>
>> Cc: Tom Herbert <therbert@google.com>
>> Cc: Eric Dumazet <eric.dumazet@gmail.com>
>> Cc: David S. Miller <davem@davemloft.net>
>> Cc: <stable@vger.kernel.org> # 3.4.x
>> Reported-and-tested-by: Shawn Landden <shawnlandden@gmail.com>
>> Signed-off-by: Richard Weinberger <richard@nod.at>
>> ---
>> fs/splice.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/fs/splice.c b/fs/splice.c
>> index 3b7ee65..b93f1b8 100644
>> --- a/fs/splice.c
>> +++ b/fs/splice.c
>> @@ -701,7 +701,7 @@ static int pipe_to_sendpage(struct pipe_inode_info *pipe,
>> more = (sd->flags & SPLICE_F_MORE) ? MSG_MORE : 0;
>>
>> if (sd->len < sd->total_len && pipe->nrbufs > 1)
>> - more |= MSG_SENDPAGE_NOTLAST;
>> + more |= MSG_SENDPAGE_NOTLAST | MSG_MORE;
>>
>> return file->f_op->sendpage(file, buf->page, buf->offset,
>> sd->len, &pos, more);
>
> I do not think this patch is right. It looks like a revert of a useful
> patch for TCP zero copy. Given the time it took to discover this
> regression, I bet tcp zero copy has more users than AF_ALG, by 5 or 6
> order of magnitude ;)
>
> Here we want to make the difference between the two flags, not merge
> them.
>
> If AF_ALG do not care of the difference, try instead :
>
> diff --git a/crypto/algif_hash.c b/crypto/algif_hash.c
> index ef5356cd280a..850246206b12 100644
> --- a/crypto/algif_hash.c
> +++ b/crypto/algif_hash.c
> @@ -114,6 +114,9 @@ static ssize_t hash_sendpage(struct socket *sock, struct page *page,
> struct hash_ctx *ctx = ask->private;
> int err;
>
> + if (flags & MSG_SENDPAGE_NOTLAST)
> + flags |= MSG_MORE;
> +
> lock_sock(sk);
> sg_init_table(ctx->sgl.sg, 1);
> sg_set_page(ctx->sgl.sg, page, size, offset);
>
>From my testing this works.
--
---
Shawn Landden
+1 360 389 3001 (SMS preferred)
^ permalink raw reply [flat|nested] 15+ messages in thread* [PATCH] Commit 35f9c09fe (tcp: tcp_sendpages() should call tcp_push() once) added an internal flag MSG_SENDPAGE_NOTLAST, similar to MSG_MORE.
2013-11-25 1:25 ` Eric Dumazet
2013-11-25 1:40 ` Shawn Landden
@ 2013-11-25 2:04 ` Shawn Landden
2013-11-25 2:08 ` [PATCH] update consumers of MSG_MORE to recognize MSG_SENDPAGE_NOTLAST Shawn Landden
2013-11-25 7:42 ` [PATCH] pipe_to_sendpage: Ensure that MSG_MORE is set if we set MSG_SENDPAGE_NOTLAST Richard Weinberger
3 siblings, 0 replies; 15+ messages in thread
From: Shawn Landden @ 2013-11-25 2:04 UTC (permalink / raw)
To: Eric Dumazet
Cc: Linux Kernel Mailing List, linux-fsdevel, viro, linux-crypto,
netdev, Herbert Xu, Tom Herbert, David S. Miller, stable,
Shawn Landden
algif_hash and algif_skcipher used MSG_MORE from tcp_sendpages()
and need to see the new flag as identical to MSG_MORE.
This fixes sendfile() on AF_ALG.
Cc: Tom Herbert <therbert@google.com>
Cc: Eric Dumazet <eric.dumazet@gmail.com>
Cc: David S. Miller <davem@davemloft.net>
Cc: <stable@vger.kernel.org> # 3.4.x + 3.2.x
Reported-and-tested-by: Shawn Landden <shawnlandden@gmail.com>
Original-patch: Richard Weinberger <richard@nod.at>
Signed-off-by: Shawn Landden <shawn@churchofgit.com>
---
crypto/algif_hash.c | 3 +++
crypto/algif_skcipher.c | 3 +++
2 files changed, 6 insertions(+)
diff --git a/crypto/algif_hash.c b/crypto/algif_hash.c
index ef5356c..8502462 100644
--- a/crypto/algif_hash.c
+++ b/crypto/algif_hash.c
@@ -114,6 +114,9 @@ static ssize_t hash_sendpage(struct socket *sock, struct page *page,
struct hash_ctx *ctx = ask->private;
int err;
+ if (flags & MSG_SENDPAGE_NOTLAST)
+ flags |= MSG_MORE;
+
lock_sock(sk);
sg_init_table(ctx->sgl.sg, 1);
sg_set_page(ctx->sgl.sg, page, size, offset);
diff --git a/crypto/algif_skcipher.c b/crypto/algif_skcipher.c
index 6a6dfc0..a19c027 100644
--- a/crypto/algif_skcipher.c
+++ b/crypto/algif_skcipher.c
@@ -378,6 +378,9 @@ static ssize_t skcipher_sendpage(struct socket *sock, struct page *page,
struct skcipher_sg_list *sgl;
int err = -EINVAL;
+ if (flags & MSG_SENDPAGE_NOTLAST)
+ flags |= MSG_MORE;
+
lock_sock(sk);
if (!ctx->more && ctx->used)
goto unlock;
--
1.8.4.4
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH] update consumers of MSG_MORE to recognize MSG_SENDPAGE_NOTLAST
2013-11-25 1:25 ` Eric Dumazet
2013-11-25 1:40 ` Shawn Landden
2013-11-25 2:04 ` [PATCH] Commit 35f9c09fe (tcp: tcp_sendpages() should call tcp_push() once) added an internal flag MSG_SENDPAGE_NOTLAST, similar to MSG_MORE Shawn Landden
@ 2013-11-25 2:08 ` Shawn Landden
2013-11-25 4:26 ` Hannes Frederic Sowa
2013-11-25 7:42 ` [PATCH] pipe_to_sendpage: Ensure that MSG_MORE is set if we set MSG_SENDPAGE_NOTLAST Richard Weinberger
3 siblings, 1 reply; 15+ messages in thread
From: Shawn Landden @ 2013-11-25 2:08 UTC (permalink / raw)
To: Eric Dumazet
Cc: Linux Kernel Mailing List, linux-fsdevel, viro, linux-crypto,
netdev, Herbert Xu, Tom Herbert, David S. Miller, stable,
Shawn Landden
Commit 35f9c09fe (tcp: tcp_sendpages() should call tcp_push() once)
added an internal flag MSG_SENDPAGE_NOTLAST, similar to
MSG_MORE.
algif_hash and algif_skcipher used MSG_MORE from tcp_sendpages()
and need to see the new flag as identical to MSG_MORE.
This fixes sendfile() on AF_ALG.
Cc: Tom Herbert <therbert@google.com>
Cc: Eric Dumazet <eric.dumazet@gmail.com>
Cc: David S. Miller <davem@davemloft.net>
Cc: <stable@vger.kernel.org> # 3.4.x + 3.2.x
Reported-and-tested-by: Shawn Landden <shawnlandden@gmail.com>
Original-patch: Richard Weinberger <richard@nod.at>
Signed-off-by: Shawn Landden <shawn@churchofgit.com>
---
crypto/algif_hash.c | 3 +++
crypto/algif_skcipher.c | 3 +++
2 files changed, 6 insertions(+)
diff --git a/crypto/algif_hash.c b/crypto/algif_hash.c
index ef5356c..8502462 100644
--- a/crypto/algif_hash.c
+++ b/crypto/algif_hash.c
@@ -114,6 +114,9 @@ static ssize_t hash_sendpage(struct socket *sock, struct page *page,
struct hash_ctx *ctx = ask->private;
int err;
+ if (flags & MSG_SENDPAGE_NOTLAST)
+ flags |= MSG_MORE;
+
lock_sock(sk);
sg_init_table(ctx->sgl.sg, 1);
sg_set_page(ctx->sgl.sg, page, size, offset);
diff --git a/crypto/algif_skcipher.c b/crypto/algif_skcipher.c
index 6a6dfc0..a19c027 100644
--- a/crypto/algif_skcipher.c
+++ b/crypto/algif_skcipher.c
@@ -378,6 +378,9 @@ static ssize_t skcipher_sendpage(struct socket *sock, struct page *page,
struct skcipher_sg_list *sgl;
int err = -EINVAL;
+ if (flags & MSG_SENDPAGE_NOTLAST)
+ flags |= MSG_MORE;
+
lock_sock(sk);
if (!ctx->more && ctx->used)
goto unlock;
--
1.8.4.4
^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH] update consumers of MSG_MORE to recognize MSG_SENDPAGE_NOTLAST
2013-11-25 2:08 ` [PATCH] update consumers of MSG_MORE to recognize MSG_SENDPAGE_NOTLAST Shawn Landden
@ 2013-11-25 4:26 ` Hannes Frederic Sowa
2013-11-25 6:36 ` Shawn Landden
0 siblings, 1 reply; 15+ messages in thread
From: Hannes Frederic Sowa @ 2013-11-25 4:26 UTC (permalink / raw)
To: Shawn Landden
Cc: Eric Dumazet, Linux Kernel Mailing List, linux-fsdevel, viro,
linux-crypto, netdev, Herbert Xu, Tom Herbert, David S. Miller,
stable
On Sun, Nov 24, 2013 at 06:08:59PM -0800, Shawn Landden wrote:
> Commit 35f9c09fe (tcp: tcp_sendpages() should call tcp_push() once)
> added an internal flag MSG_SENDPAGE_NOTLAST, similar to
> MSG_MORE.
>
> algif_hash and algif_skcipher used MSG_MORE from tcp_sendpages()
> and need to see the new flag as identical to MSG_MORE.
>
> This fixes sendfile() on AF_ALG.
Don't we need a similar fix for udp_sendpage?
Greetings,
Hannes
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH] update consumers of MSG_MORE to recognize MSG_SENDPAGE_NOTLAST
2013-11-25 4:26 ` Hannes Frederic Sowa
@ 2013-11-25 6:36 ` Shawn Landden
2013-11-25 23:27 ` Richard Weinberger
` (2 more replies)
0 siblings, 3 replies; 15+ messages in thread
From: Shawn Landden @ 2013-11-25 6:36 UTC (permalink / raw)
To: Hannes Frederic Sowa
Cc: Eric Dumazet, linux-kernel, linux-fsdevel, viro, linux-crypto,
netdev, Herbert Xu, Tom Herbert, David S. Miller, stable,
Shawn Landden
Commit 35f9c09fe (tcp: tcp_sendpages() should call tcp_push() once)
added an internal flag MSG_SENDPAGE_NOTLAST, similar to
MSG_MORE.
algif_hash, algif_skcipher, and udp used MSG_MORE from tcp_sendpages()
and need to see the new flag as identical to MSG_MORE.
This fixes sendfile() on AF_ALG.
v3: also fix udp
Cc: Tom Herbert <therbert@google.com>
Cc: Eric Dumazet <eric.dumazet@gmail.com>
Cc: David S. Miller <davem@davemloft.net>
Cc: <stable@vger.kernel.org> # 3.4.x + 3.2.x
Reported-and-tested-by: Shawn Landden <shawnlandden@gmail.com>
Original-patch: Richard Weinberger <richard@nod.at>
Signed-off-by: Shawn Landden <shawn@churchofgit.com>
---
crypto/algif_hash.c | 3 +++
crypto/algif_skcipher.c | 3 +++
net/ipv4/udp.c | 3 +++
3 files changed, 9 insertions(+)
diff --git a/crypto/algif_hash.c b/crypto/algif_hash.c
index ef5356c..8502462 100644
--- a/crypto/algif_hash.c
+++ b/crypto/algif_hash.c
@@ -114,6 +114,9 @@ static ssize_t hash_sendpage(struct socket *sock, struct page *page,
struct hash_ctx *ctx = ask->private;
int err;
+ if (flags & MSG_SENDPAGE_NOTLAST)
+ flags |= MSG_MORE;
+
lock_sock(sk);
sg_init_table(ctx->sgl.sg, 1);
sg_set_page(ctx->sgl.sg, page, size, offset);
diff --git a/crypto/algif_skcipher.c b/crypto/algif_skcipher.c
index 6a6dfc0..a19c027 100644
--- a/crypto/algif_skcipher.c
+++ b/crypto/algif_skcipher.c
@@ -378,6 +378,9 @@ static ssize_t skcipher_sendpage(struct socket *sock, struct page *page,
struct skcipher_sg_list *sgl;
int err = -EINVAL;
+ if (flags & MSG_SENDPAGE_NOTLAST)
+ flags |= MSG_MORE;
+
lock_sock(sk);
if (!ctx->more && ctx->used)
goto unlock;
diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c
index 5944d7d..8bd04df 100644
--- a/net/ipv4/udp.c
+++ b/net/ipv4/udp.c
@@ -1098,6 +1098,9 @@ int udp_sendpage(struct sock *sk, struct page *page, int offset,
struct udp_sock *up = udp_sk(sk);
int ret;
+ if (flags & MSG_SENDPAGE_NOTLAST)
+ flags |= MSG_MORE;
+
if (!up->pending) {
struct msghdr msg = { .msg_flags = flags|MSG_MORE };
--
1.8.4.4
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [PATCH] update consumers of MSG_MORE to recognize MSG_SENDPAGE_NOTLAST
2013-11-25 6:36 ` Shawn Landden
@ 2013-11-25 23:27 ` Richard Weinberger
2013-11-29 5:47 ` Hannes Frederic Sowa
2013-11-29 21:33 ` David Miller
2 siblings, 0 replies; 15+ messages in thread
From: Richard Weinberger @ 2013-11-25 23:27 UTC (permalink / raw)
To: Shawn Landden
Cc: Hannes Frederic Sowa, Eric Dumazet, LKML, linux-fsdevel, Al Viro,
linux-crypto, netdev@vger.kernel.org, Herbert Xu, Tom Herbert,
David S. Miller, stable
On Mon, Nov 25, 2013 at 7:36 AM, Shawn Landden <shawn@churchofgit.com> wrote:
> Commit 35f9c09fe (tcp: tcp_sendpages() should call tcp_push() once)
> added an internal flag MSG_SENDPAGE_NOTLAST, similar to
> MSG_MORE.
>
> algif_hash, algif_skcipher, and udp used MSG_MORE from tcp_sendpages()
> and need to see the new flag as identical to MSG_MORE.
>
> This fixes sendfile() on AF_ALG.
>
> v3: also fix udp
>
> Cc: Tom Herbert <therbert@google.com>
> Cc: Eric Dumazet <eric.dumazet@gmail.com>
> Cc: David S. Miller <davem@davemloft.net>
> Cc: <stable@vger.kernel.org> # 3.4.x + 3.2.x
> Reported-and-tested-by: Shawn Landden <shawnlandden@gmail.com>
> Original-patch: Richard Weinberger <richard@nod.at>
> Signed-off-by: Shawn Landden <shawn@churchofgit.com>
May I ask why you took over the my patch without even CC'in me nor
replying to the original
thread "[PATCH] pipe_to_sendpage: Ensure that MSG_MORE is set if we
set MSG_SENDPAGE_NOTLAST"?
You are acting very rude.
The discussion at the original thread is not done.
Does skcipher_sendpage() really also need fixing? or UDP?
I didn't send another patch because I'm waiting for Eric's answer first.
Thanks,
//richard
> ---
> crypto/algif_hash.c | 3 +++
> crypto/algif_skcipher.c | 3 +++
> net/ipv4/udp.c | 3 +++
> 3 files changed, 9 insertions(+)
>
> diff --git a/crypto/algif_hash.c b/crypto/algif_hash.c
> index ef5356c..8502462 100644
> --- a/crypto/algif_hash.c
> +++ b/crypto/algif_hash.c
> @@ -114,6 +114,9 @@ static ssize_t hash_sendpage(struct socket *sock, struct page *page,
> struct hash_ctx *ctx = ask->private;
> int err;
>
> + if (flags & MSG_SENDPAGE_NOTLAST)
> + flags |= MSG_MORE;
> +
> lock_sock(sk);
> sg_init_table(ctx->sgl.sg, 1);
> sg_set_page(ctx->sgl.sg, page, size, offset);
> diff --git a/crypto/algif_skcipher.c b/crypto/algif_skcipher.c
> index 6a6dfc0..a19c027 100644
> --- a/crypto/algif_skcipher.c
> +++ b/crypto/algif_skcipher.c
> @@ -378,6 +378,9 @@ static ssize_t skcipher_sendpage(struct socket *sock, struct page *page,
> struct skcipher_sg_list *sgl;
> int err = -EINVAL;
>
> + if (flags & MSG_SENDPAGE_NOTLAST)
> + flags |= MSG_MORE;
> +
> lock_sock(sk);
> if (!ctx->more && ctx->used)
> goto unlock;
> diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c
> index 5944d7d..8bd04df 100644
> --- a/net/ipv4/udp.c
> +++ b/net/ipv4/udp.c
> @@ -1098,6 +1098,9 @@ int udp_sendpage(struct sock *sk, struct page *page, int offset,
> struct udp_sock *up = udp_sk(sk);
> int ret;
>
> + if (flags & MSG_SENDPAGE_NOTLAST)
> + flags |= MSG_MORE;
> +
> if (!up->pending) {
> struct msghdr msg = { .msg_flags = flags|MSG_MORE };
>
> --
> 1.8.4.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
Thanks,
//richard
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH] update consumers of MSG_MORE to recognize MSG_SENDPAGE_NOTLAST
2013-11-25 6:36 ` Shawn Landden
2013-11-25 23:27 ` Richard Weinberger
@ 2013-11-29 5:47 ` Hannes Frederic Sowa
2013-11-29 8:50 ` Richard Weinberger
2013-11-29 21:33 ` David Miller
2 siblings, 1 reply; 15+ messages in thread
From: Hannes Frederic Sowa @ 2013-11-29 5:47 UTC (permalink / raw)
To: Shawn Landden
Cc: Eric Dumazet, linux-kernel, linux-fsdevel, viro, linux-crypto,
netdev, Herbert Xu, Tom Herbert, David S. Miller, stable,
richard.weinberger
On Sun, Nov 24, 2013 at 10:36:28PM -0800, Shawn Landden wrote:
> Commit 35f9c09fe (tcp: tcp_sendpages() should call tcp_push() once)
> added an internal flag MSG_SENDPAGE_NOTLAST, similar to
> MSG_MORE.
>
> algif_hash, algif_skcipher, and udp used MSG_MORE from tcp_sendpages()
> and need to see the new flag as identical to MSG_MORE.
>
> This fixes sendfile() on AF_ALG.
>
> v3: also fix udp
The UDP bits look fine to me.
Greetings,
Hannes
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] update consumers of MSG_MORE to recognize MSG_SENDPAGE_NOTLAST
2013-11-29 5:47 ` Hannes Frederic Sowa
@ 2013-11-29 8:50 ` Richard Weinberger
0 siblings, 0 replies; 15+ messages in thread
From: Richard Weinberger @ 2013-11-29 8:50 UTC (permalink / raw)
To: Hannes Frederic Sowa
Cc: Shawn Landden, Eric Dumazet, linux-kernel, linux-fsdevel, viro,
linux-crypto, netdev, Herbert Xu, Tom Herbert, David S. Miller,
stable
Shawn,
Am Freitag, 29. November 2013, 06:47:04 schrieb Hannes Frederic Sowa:
> On Sun, Nov 24, 2013 at 10:36:28PM -0800, Shawn Landden wrote:
> > Commit 35f9c09fe (tcp: tcp_sendpages() should call tcp_push() once)
> > added an internal flag MSG_SENDPAGE_NOTLAST, similar to
> > MSG_MORE.
> >
> > algif_hash, algif_skcipher, and udp used MSG_MORE from tcp_sendpages()
> > and need to see the new flag as identical to MSG_MORE.
> >
> > This fixes sendfile() on AF_ALG.
> >
> > v3: also fix udp
>
> The UDP bits look fine to me.
please don't forget to resend the patch (only the UDP fix).
I sent the AF_ALG part already.
Thanks,
//richard
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] update consumers of MSG_MORE to recognize MSG_SENDPAGE_NOTLAST
2013-11-25 6:36 ` Shawn Landden
2013-11-25 23:27 ` Richard Weinberger
2013-11-29 5:47 ` Hannes Frederic Sowa
@ 2013-11-29 21:33 ` David Miller
2 siblings, 0 replies; 15+ messages in thread
From: David Miller @ 2013-11-29 21:33 UTC (permalink / raw)
To: shawn
Cc: hannes, eric.dumazet, linux-kernel, linux-fsdevel, viro,
linux-crypto, netdev, herbert, therbert, stable
From: Shawn Landden <shawn@churchofgit.com>
Date: Sun, 24 Nov 2013 22:36:28 -0800
> Commit 35f9c09fe (tcp: tcp_sendpages() should call tcp_push() once)
> added an internal flag MSG_SENDPAGE_NOTLAST, similar to
> MSG_MORE.
>
> algif_hash, algif_skcipher, and udp used MSG_MORE from tcp_sendpages()
> and need to see the new flag as identical to MSG_MORE.
>
> This fixes sendfile() on AF_ALG.
>
> v3: also fix udp
>
> Cc: Tom Herbert <therbert@google.com>
> Cc: Eric Dumazet <eric.dumazet@gmail.com>
> Cc: David S. Miller <davem@davemloft.net>
> Cc: <stable@vger.kernel.org> # 3.4.x + 3.2.x
> Reported-and-tested-by: Shawn Landden <shawnlandden@gmail.com>
> Original-patch: Richard Weinberger <richard@nod.at>
> Signed-off-by: Shawn Landden <shawn@churchofgit.com>
Applied and queued up for -stable, thanks everyone.
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] pipe_to_sendpage: Ensure that MSG_MORE is set if we set MSG_SENDPAGE_NOTLAST
2013-11-25 1:25 ` Eric Dumazet
` (2 preceding siblings ...)
2013-11-25 2:08 ` [PATCH] update consumers of MSG_MORE to recognize MSG_SENDPAGE_NOTLAST Shawn Landden
@ 2013-11-25 7:42 ` Richard Weinberger
2013-11-26 0:02 ` Eric Dumazet
3 siblings, 1 reply; 15+ messages in thread
From: Richard Weinberger @ 2013-11-25 7:42 UTC (permalink / raw)
To: Eric Dumazet
Cc: linux-kernel, linux-fsdevel, viro, shawnlandden, linux-crypto,
netdev, herbert, Tom Herbert, David S. Miller, stable
Am Sonntag, 24. November 2013, 17:25:06 schrieb Eric Dumazet:
> On Mon, 2013-11-25 at 00:42 +0100, Richard Weinberger wrote:
> > Commit 35f9c09fe (tcp: tcp_sendpages() should call tcp_push() once)
> > added an internal flag MSG_SENDPAGE_NOTLAST.
> > We have to ensure that MSG_MORE is also set if we set
> > MSG_SENDPAGE_NOTLAST.
> > Otherwise users that check against MSG_MORE will not see it.
> >
> > This fixes sendfile() on AF_ALG.
> >
> > Cc: Tom Herbert <therbert@google.com>
> > Cc: Eric Dumazet <eric.dumazet@gmail.com>
> > Cc: David S. Miller <davem@davemloft.net>
> > Cc: <stable@vger.kernel.org> # 3.4.x
> > Reported-and-tested-by: Shawn Landden <shawnlandden@gmail.com>
> > Signed-off-by: Richard Weinberger <richard@nod.at>
> > ---
> >
> > fs/splice.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/fs/splice.c b/fs/splice.c
> > index 3b7ee65..b93f1b8 100644
> > --- a/fs/splice.c
> > +++ b/fs/splice.c
> > @@ -701,7 +701,7 @@ static int pipe_to_sendpage(struct pipe_inode_info
> > *pipe,>
> > more = (sd->flags & SPLICE_F_MORE) ? MSG_MORE : 0;
> >
> > if (sd->len < sd->total_len && pipe->nrbufs > 1)
> >
> > - more |= MSG_SENDPAGE_NOTLAST;
> > + more |= MSG_SENDPAGE_NOTLAST | MSG_MORE;
> >
> > return file->f_op->sendpage(file, buf->page, buf->offset,
> >
> > sd->len, &pos, more);
>
> I do not think this patch is right. It looks like a revert of a useful
> patch for TCP zero copy. Given the time it took to discover this
> regression, I bet tcp zero copy has more users than AF_ALG, by 5 or 6
> order of magnitude ;)
Yeah, but AF_ALG broke. That's why I did the patch.
> Here we want to make the difference between the two flags, not merge
> them.
>
> If AF_ALG do not care of the difference, try instead :
>
> diff --git a/crypto/algif_hash.c b/crypto/algif_hash.c
> index ef5356cd280a..850246206b12 100644
> --- a/crypto/algif_hash.c
> +++ b/crypto/algif_hash.c
> @@ -114,6 +114,9 @@ static ssize_t hash_sendpage(struct socket *sock, struct
> page *page, struct hash_ctx *ctx = ask->private;
> int err;
>
> + if (flags & MSG_SENDPAGE_NOTLAST)
> + flags |= MSG_MORE;
> +
In the commit message of your patch you wrote "For all sendpage() providers,
its a transparent change.". Why does AF_ALG need special handling?
If users have to care about MSG_SENDPAGE_NOTLAST it is no longer really an
internal flag.
Thanks,
//richard
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH] pipe_to_sendpage: Ensure that MSG_MORE is set if we set MSG_SENDPAGE_NOTLAST
2013-11-25 7:42 ` [PATCH] pipe_to_sendpage: Ensure that MSG_MORE is set if we set MSG_SENDPAGE_NOTLAST Richard Weinberger
@ 2013-11-26 0:02 ` Eric Dumazet
0 siblings, 0 replies; 15+ messages in thread
From: Eric Dumazet @ 2013-11-26 0:02 UTC (permalink / raw)
To: Richard Weinberger
Cc: linux-kernel, linux-fsdevel, viro, shawnlandden, linux-crypto,
netdev, herbert, Tom Herbert, David S. Miller, stable
On Mon, 2013-11-25 at 08:42 +0100, Richard Weinberger wrote:
> In the commit message of your patch you wrote "For all sendpage() providers,
> its a transparent change.". Why does AF_ALG need special handling?
> If users have to care about MSG_SENDPAGE_NOTLAST it is no longer really an
> internal flag.
MSG_SENDPAGE_NOTLAST is an internal (kernel) flag.
Fact that I missed some MSG_MORE 'users' in the kernel was an oversight.
I am not saying your patch is not needed, I am only saying it reverts
a useful TCP optimization, and we can do better, dont we ?
^ permalink raw reply [flat|nested] 15+ messages in thread