netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Maxime Bizon <mbizon@freebox.fr>
To: Eric Dumazet <eric.dumazet@gmail.com>
Cc: David Madore <david+ml@madore.org>,
	Francois Romieu <romieu@fr.zoreil.com>,
	netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
	Hugh Dickins <hughd@google.com>
Subject: Re: kernel 3.2.27 on arm: WARNING: at mm/page_alloc.c:2109 __alloc_pages_nodemask+0x1d4/0x68c()
Date: Fri, 05 Oct 2012 12:49:54 +0200	[thread overview]
Message-ID: <1349434194.16710.44.camel@sakura.staff.proxad.net> (raw)
In-Reply-To: <1349422868.21172.38.camel@edumazet-glaptop>


On Fri, 2012-10-05 at 09:41 +0200, Eric Dumazet wrote:

> By the way, the commit you pointed has no effect on the reallocation
> performed by pskb_expand_head() :

The commit has a side effect, because the problem appeared after it was
merged (and goes away if I revert it)

> int size = nhead + skb_end_offset(skb) + ntail;
> 
> So pskb_expand_head() always assumed the current head is fully used, and
> because we have some kmalloc-power-of-two contraints, each time
> pskb_expand_head() is called with a non zero (nhead + ntail) we double
> the skb->head ksize.

That is true, but only after the commit I mentioned.

Before that commit, we indeed reallocate skb->head to twice the size,
but skb->end is *not* positioned at the end of newly allocated data. So
on the next pskb_expand_head(), if head and tail are not big values, the
kmalloc() will be of the same size.


The commit adds this after allocation:

size = SKB_WITH_OVERHEAD(ksize(data))
[...]
skb->end      = skb->head + size;

so on the next pskb_expand_head, we are going to allocate twice the size
for sure.

> So why are we using skb_end_offset(skb) here is the question.
> 
> I guess it could be (skb_tail_pointer(skb) - skb->head) on some uses.

I think your patch is wrong, ntail is not the new tailroom size, it's
what missing to the current tailroom size, by adding ntail + nhead +
tail_offset we are removing previous tailroom.

We cannot shrink the skb that way here I guess, a caller may check
needed headroom & tailroom, calls with nhead=1/ntail=0 because only
headroom is missing, but after the call tailroom would be less than
before the call.

Why don't we juste reallocate to this size:

MAX(current_alloc_size, nhead + ntail + current_end - current_head)

-- 
Maxime

  reply	other threads:[~2012-10-05 10:49 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <20120829002548.GA7063@aldebaran.gro-tsen.net>
2012-09-01  2:21 ` kernel 3.2.27 on arm: WARNING: at mm/page_alloc.c:2109 __alloc_pages_nodemask+0x1d4/0x68c() Hugh Dickins
2012-09-01  8:20   ` Francois Romieu
2012-09-02 22:51     ` David Madore
2012-10-04 16:02   ` Maxime Bizon
2012-10-04 16:29     ` Eric Dumazet
2012-10-05  7:41       ` Eric Dumazet
2012-10-05 10:49         ` Maxime Bizon [this message]
2012-10-05 12:22           ` Eric Dumazet
2012-10-05 12:37             ` Eric Dumazet
2012-10-05 12:39               ` Eric Dumazet
2012-10-05 12:51             ` Maxime Bizon
2012-10-05 13:02               ` Eric Dumazet
2012-10-05 14:50                 ` Maxime Bizon
2012-10-05 15:04                   ` Eric Dumazet
2012-10-05 15:15                     ` Maxime Bizon
2012-10-05 15:37                       ` Eric Dumazet
2012-10-05 16:23                         ` [PATCH] net: remove skb recycling Eric Dumazet
2012-10-07  4:41                           ` David Miller
2012-10-04 16:50     ` kernel 3.2.27 on arm: WARNING: at mm/page_alloc.c:2109 __alloc_pages_nodemask+0x1d4/0x68c() Eric Dumazet
2012-10-04 17:09       ` Maxime Bizon
2012-10-04 17:17         ` Eric Dumazet
2012-10-04 17:34           ` Maxime Bizon
2012-10-04 21:27             ` Eric Dumazet

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1349434194.16710.44.camel@sakura.staff.proxad.net \
    --to=mbizon@freebox.fr \
    --cc=david+ml@madore.org \
    --cc=eric.dumazet@gmail.com \
    --cc=hughd@google.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=romieu@fr.zoreil.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).