netdev - Re: [PATCH v2] net: avoid the unnecessary kmalloc

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Tue, 30 Nov 2010 15:45:24 +0800
From:	Changli Gao <xiaosuo@...il.com>
To:	Eric Dumazet <eric.dumazet@...il.com>
Cc:	"David S. Miller" <davem@...emloft.net>, netdev@...r.kernel.org
Subject: Re: [PATCH v2] net: avoid the unnecessary kmalloc

On Tue, Nov 30, 2010 at 2:52 PM, Eric Dumazet <eric.dumazet@...il.com> wrote:
> Le mardi 30 novembre 2010 à 08:40 +0800, Changli Gao a écrit :
>> If the old memory allocated by kmalloc() is larger than the new requested,
>> pskb_expand_head() doesn't need to allocate a new one, unless the skb->head
>> is shared.
>>
>> Signed-off-by: Changli Gao <xiaosuo@...il.com>
>
> Seems fine to me, but patch title and changelog are a bit uninformative.
>
>
> skb head being allocated by kmalloc(), it might be larger than what
> actually requested because of discrete kmem caches sizes. Before
> reallocating a new skb head, check if the current one has the needed
> extra size.
>
> Do this check only if skb head is not shared.
>
>

OK, I'll refine it with your comment. Thanks.

>
>> +
>> +     if (fastpath &&
>> +         size + sizeof(struct skb_shared_info) <= ksize(skb->head)) {
>> +             memmove(skb->head + size, skb_shinfo(skb),
>> +                     offsetof(struct skb_shared_info,
>> +                              frags[skb_shinfo(skb)->nr_frags]));
>> +             memmove(skb->head + nhead, skb->head,
>> +                     skb_tail_pointer(skb) - skb->head);
>> +             off = nhead;
>> +             goto adjust_others;
>> +     }
>> +
>
> I suggest doing the max possible resize at this stage ?
>
> Ie moving skb_shared_info at the edge of memory block.
>
> Maybe its not necessary, and a given skb is not expanded multiple times
> in our stack, I dont really know.
>

I don't think it is a good idea, If it is, why not use
ksize(skb->head) to set the tail pointer of skb when allocating a new
skb.

If we do sth. like this, more cache lines maybe involved.

-- 
Regards,
Changli Gao(xiaosuo@...il.com)
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html