Subject: Re: query: TCPCT setsockopt kmalloc's

From: Eric Dumazet <[email-masked]>
Date: Tue, 06 Oct 2009 18:31:16 +0200

William Allen Simpson a écrit :
> On both the client and server side, the setsockopt does a kmalloc().  Only
> once per connect on the client side, once per listen on the server side.

But if server has to generate a random data for a connection, you'll have to clone
the data to be able to give it at application request ?

> 
> However, after Miller's expressed concern, it seems possible to reorganize
> the code to do the kmalloc() before the lock_sock().  Does that mean
> that it
> should be GFP_KERNEL?  Or should it still be GFP_ATOMIC?

GFP_KERNEL is better in this context, it allows the requestor to sleep a bit if necessary.

> 
> [Not that anybody cares, but based on recent discussion on the list about
> internal kernel coding standards, I've changed Adam's old sizeof(struct)
> to sizeof(*tcvp).  Previously, I was trying to make as few changes as
> possible, thinking everything was already correct.]

> 
> ===
> new:
> 
> +        /* Allocate ancillary memory before locking.
> +         */
> +        if ((0 < tcd.tcpcd_used

Ah, could you please reverse all your conditions ?

Their form are unusual in linux kernel.

if (tcd.tcpcd_used > 0)

> ...
> +         && NULL == (tcvp = kmalloc(sizeof(*tcvp) + tcd.tcpcd_used,
> +                        GFP_KERNEL))) {
> +            return -ENOMEM;
> +        }
>
 
tcvp = kmalloc(sizeof(*tcvp) + tcd.tcpcd_used, GFP_KERNEL));
if (tcvp == NULL) ...