tcp_do_sendmsg() allocation still broken ?

tcp_do_sendmsg() allocation still broken ?

[Marcelo Tosatti]

David,

tcp_do_sendmsg() still allocates using GFP_KERNEL when it can't, it seems: 

int tcp_do_sendmsg(struct sock *sk, struct msghdr *msg)
{
...
                     TCP_SKB_CB(skb)->seq = tp->write_seq;
                        TCP_SKB_CB(skb)->end_seq = TCP_SKB_CB(skb)->seq +
copy;

                        /* This advances tp->write_seq for us. */
                        tcp_send_skb(sk, skb, queue_it);
			^^^^^^^^^^^^
 
                }
        }
        sk->err = 0;
...
}


Now what we can find at tcp_send_skb? :)


void tcp_send_skb(struct sock *sk, struct sk_buff *skb, int force_queue,
unsigned cur_mss) {

...

    if (!force_queue && tp->send_head == NULL && tcp_snd_test(tp, skb,
cur_mss, 1)) {
                /* Send it out now. */
                TCP_SKB_CB(skb)->when = tcp_time_stamp;
                if (tcp_transmit_skb(sk, skb_clone(skb, GFP_KERNEL)) == 0) {
							^^^^^^^^^^^^
                        tp->snd_nxt = TCP_SKB_CB(skb)->end_seq;

...




-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
Please read the FAQ at http://www.tux.org/lkml/

Re: tcp_do_sendmsg() allocation still broken ?

[Marcelo Tosatti]

On Sun, 29 Oct 2000, Andi Kleen wrote:

> On Sat, Oct 28, 2000 at 07:12:44PM -0200, Marcelo Tosatti wrote:
> > 
> > David,
> > 
> > tcp_do_sendmsg() still allocates using GFP_KERNEL when it can't, it seems: 
> 
> tcp_do_sendmsg() should only be called from process context, because it can
> sleep for other reasons anyways. 
> 
> If someone calls it from interrupt context it needs to be fixed.

Andi,

Think about nbd. 

It allocates memory inside its request function, and since we do write
throttling in try_to_free_pages() we can end up with this deadlock in low 
memory conditions:

nbd_do_request -> nbd_send_req -> nbd_xmit -> sock_sendmsg -> ... ->
tcp_do_sendmsg -> tcp_send_skb -> skb_copy -> alloc_skb ->
kmalloc(GFP_KERNEL) -> kmem_cache_grow -> get_free_pages ->
try_to_free_pages -> shrink_mmap -> try_to_free_buffers ->
sync_page_buffers -> ll_rw_block -> make_request -> add_request ->
nbd_do_request -> nbd_send_req -> ...

until there are no more free requests on the request queue and writer
processes stuck on __get_request_wait.



-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
Please read the FAQ at http://www.tux.org/lkml/

Re: tcp_do_sendmsg() allocation still broken ?

[Marcelo Tosatti]

On Sun, 29 Oct 2000, Andi Kleen wrote:

> On Sat, Oct 28, 2000 at 09:13:27PM -0200, Marcelo Tosatti wrote:
> > 
> > 
> > On Sun, 29 Oct 2000, Andi Kleen wrote:
> > 
> > > On Sat, Oct 28, 2000 at 07:12:44PM -0200, Marcelo Tosatti wrote:
> > > > 
> > > > David,
> > > > 
> > > > tcp_do_sendmsg() still allocates using GFP_KERNEL when it can't, it seems: 
> > > 
> > > tcp_do_sendmsg() should only be called from process context, because it can
> > > sleep for other reasons anyways. 
> > > 
> > > If someone calls it from interrupt context it needs to be fixed.
> > 
> > Andi,
> > 
> > Think about nbd. 
> 
> Making tcp_do_sendmsg use GFP_ATOMIC would make it too unreliable for other
> situations. Penalizing the whole system just for nbd is not a good idea.

I agree that using GFP_ATOMIC is bad there.

I've fixed 2.2 with GFP_BUFFER. 

> 
> > 
> > It allocates memory inside its request function, and since we do write
> > throttling in try_to_free_pages() we can end up with this deadlock in low 
> > memory conditions:
> > 
> > nbd_do_request -> nbd_send_req -> nbd_xmit -> sock_sendmsg -> ... ->
> > tcp_do_sendmsg -> tcp_send_skb -> skb_copy -> alloc_skb ->
> > kmalloc(GFP_KERNEL) -> kmem_cache_grow -> get_free_pages ->
> > try_to_free_pages -> shrink_mmap -> try_to_free_buffers ->
> > sync_page_buffers -> ll_rw_block -> make_request -> add_request ->
> > nbd_do_request -> nbd_send_req -> ...
> > 
> > until there are no more free requests on the request queue and writer
> > processes stuck on __get_request_wait.
> 
> Looks like a job for PF_MEMALLOC and making GFP_KERNEL fail earlier.

IMHO GFP_BUFFER is ok. 




-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
Please read the FAQ at http://www.tux.org/lkml/

Re: tcp_do_sendmsg() allocation still broken ?

[Andi Kleen]

On Sat, Oct 28, 2000 at 07:12:44PM -0200, Marcelo Tosatti wrote:
> 
> David,
> 
> tcp_do_sendmsg() still allocates using GFP_KERNEL when it can't, it seems: 

tcp_do_sendmsg() should only be called from process context, because it can
sleep for other reasons anyways. 

If someone calls it from interrupt context it needs to be fixed.


-Andi
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
Please read the FAQ at http://www.tux.org/lkml/

Re: tcp_do_sendmsg() allocation still broken ?

[Andi Kleen]

On Sat, Oct 28, 2000 at 09:13:27PM -0200, Marcelo Tosatti wrote:
> 
> 
> On Sun, 29 Oct 2000, Andi Kleen wrote:
> 
> > On Sat, Oct 28, 2000 at 07:12:44PM -0200, Marcelo Tosatti wrote:
> > > 
> > > David,
> > > 
> > > tcp_do_sendmsg() still allocates using GFP_KERNEL when it can't, it seems: 
> > 
> > tcp_do_sendmsg() should only be called from process context, because it can
> > sleep for other reasons anyways. 
> > 
> > If someone calls it from interrupt context it needs to be fixed.
> 
> Andi,
> 
> Think about nbd. 

Making tcp_do_sendmsg use GFP_ATOMIC would make it too unreliable for other
situations. Penalizing the whole system just for nbd is not a good idea.

> 
> It allocates memory inside its request function, and since we do write
> throttling in try_to_free_pages() we can end up with this deadlock in low 
> memory conditions:
> 
> nbd_do_request -> nbd_send_req -> nbd_xmit -> sock_sendmsg -> ... ->
> tcp_do_sendmsg -> tcp_send_skb -> skb_copy -> alloc_skb ->
> kmalloc(GFP_KERNEL) -> kmem_cache_grow -> get_free_pages ->
> try_to_free_pages -> shrink_mmap -> try_to_free_buffers ->
> sync_page_buffers -> ll_rw_block -> make_request -> add_request ->
> nbd_do_request -> nbd_send_req -> ...
> 
> until there are no more free requests on the request queue and writer
> processes stuck on __get_request_wait.

Looks like a job for PF_MEMALLOC and making GFP_KERNEL fail earlier.

-Andi
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
Please read the FAQ at http://www.tux.org/lkml/

Re: tcp_do_sendmsg() allocation still broken ?

[David S. Miller]

   Date: Sun, 29 Oct 2000 02:03:11 +0100
   From: Andi Kleen <ak@suse.de>

   tcp_do_sendmsg() should only be called from process context,
   because it can sleep for other reasons anyways.

   If someone calls it from interrupt context it needs to be fixed.

He is calling it from process context, he needs GFP_ATOMIC for
other reasons (VM deadlocks when using NBD).

Later,
David S. Miller
davem@redhat.com
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
Please read the FAQ at http://www.tux.org/lkml/

Re: tcp_do_sendmsg() allocation still broken ?

[David S. Miller]

   Date: Sun, 29 Oct 2000 02:16:42 +0100
   From: Andi Kleen <ak@suse.de>

   On Sat, Oct 28, 2000 at 09:13:27PM -0200, Marcelo Tosatti wrote:
   > Think about nbd. 

   Making tcp_do_sendmsg use GFP_ATOMIC would make it too unreliable for other
   situations. Penalizing the whole system just for nbd is not a good idea.

Andi, sk->allocation not GFP_ATOMIC.

This is such an old topic Andi, I am surprised you forget all these details
so quickly.

Later,
David S. Miller
davem@redhat.com
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
Please read the FAQ at http://www.tux.org/lkml/

Re: tcp_do_sendmsg() allocation still broken ?

[Rik van Riel]

On Sun, 29 Oct 2000, Andi Kleen wrote:
> On Sat, Oct 28, 2000 at 07:12:44PM -0200, Marcelo Tosatti wrote:
> > 
> > tcp_do_sendmsg() still allocates using GFP_KERNEL when it can't, it seems: 
> 
> tcp_do_sendmsg() should only be called from process context,
> because it can sleep for other reasons anyways.

Andi, if sk->allocation is there to be ignored, why 
don't you remove that element from the structure?

> If someone calls it from interrupt context it needs to be fixed.

Think about nbd.
Think about swap-over-network.
Think about the reasons why sk->allocation exists.

regards,

Rik
--
"What you're running that piece of shit Gnome?!?!"
       -- Miguel de Icaza, UKUUG 2000

http://www.conectiva.com/		http://www.surriel.com/

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
Please read the FAQ at http://www.tux.org/lkml/

Re: tcp_do_sendmsg() allocation still broken ?

[Andi Kleen]

On Sat, Oct 28, 2000 at 11:46:37PM -0200, Rik van Riel wrote:
> Think about swap-over-network.

swap-over-network is unlikely to ever work properly without a preallocated 
skb pool, no matter if you use sk->allocation or not. 

-Andi

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
Please read the FAQ at http://www.tux.org/lkml/

Re: tcp_do_sendmsg() allocation still broken ?

[Alan Cox]

> > Think about nbd. 
> 
> Making tcp_do_sendmsg use GFP_ATOMIC would make it too unreliable for other
> situations. Penalizing the whole system just for nbd is not a good idea.

Nobody is saying that. tcp_do_sendmsg should honour the existing 
sk->allocation flag


-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
Please read the FAQ at http://www.tux.org/lkml/