From: Ingo Molnar <mingo@kernel.org>
To: Eric Dumazet <eric.dumazet@gmail.com>
Cc: Peter Zijlstra <peterz@infradead.org>,
Christoph Lameter <cl@linux.com>, Tejun Heo <tj@kernel.org>,
akpm@linuxfoundation.org, rostedt@goodmis.org,
linux-kernel@vger.kernel.org,
Thomas Gleixner <tglx@linutronix.de>,
Eric Dumazet <edumazet@google.com>,
David Miller <davem@davemloft.net>
Subject: Re: [PATCH 1/6] net: ip4_datagram_connect: Use correct form of statistics update
Date: Wed, 16 Oct 2013 11:26:01 +0200 [thread overview]
Message-ID: <20131016092601.GB23440@gmail.com> (raw)
In-Reply-To: <1381914869.2045.112.camel@edumazet-glaptop.roam.corp.google.com>
* Eric Dumazet <eric.dumazet@gmail.com> wrote:
> On Wed, 2013-10-16 at 10:35 +0200, Peter Zijlstra wrote:
> > On Tue, Oct 15, 2013 at 12:47:23PM -0500, Christoph Lameter wrote:
> > > ip4_datagram_connect is called with BH processing enabled. Therefore
> > > we cannot use IP_INC_STATS_BH but must use IP_INC_STATS which disables
> > > BH handling before incrementing the counter.
> > >
> > > The following trace is triggered without this patch:
> > >
> > > [ 9293.806634] __this_cpu_add operation in preemptible [00000000] code: ntpd/2150
> >
> > You lost the BUG there; that really needs to be there:
> >
> > - BUG makes people pay attention
> > - This was an actual BUG wasn't it?
> >
> > Sure there can be false positives, but in all cases people should
> > amend the code. Sometimes with a comment explaining why the raw
> > primitive should be used; sometimes to fix an actual bug, but a patch
> > needs to be written. Therefore BUG!
>
> Yep this is a real BUG for linux 2.6.36+ on 32bit arches,
>
> The effect of this bug is that on 32bit arches, we might corrupt a
> seqcount : Later, we can spin forever on it.
Ouch, that's a pretty serious bug ...
The patch title should reflect this fact.
> In linux 2.6.36 we converted IP mib from 32 to 64 bits, therefore this
> fix should be backported up to 2.6.36
>
> Prior to 2.6.36, the bug was that some increments of SNMP stat could be
> lost, because two cpus could access the same location, hardly a
> problem...
Acked-by: Ingo Molnar <mingo@kernel.org>
Thanks,
Ingo
next prev parent reply other threads:[~2013-10-16 9:26 UTC|newest]
Thread overview: 34+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-10-15 17:47 [PATCH 0/6] percpu: Implement Preemption checks for __this_cpu operations V4b Christoph Lameter
2013-10-15 17:47 ` [PATCH 1/6] net: ip4_datagram_connect: Use correct form of statistics update Christoph Lameter
2013-10-15 18:36 ` Eric Dumazet
2013-10-16 6:09 ` Ingo Molnar
2013-10-16 8:35 ` Peter Zijlstra
2013-10-16 9:14 ` Eric Dumazet
2013-10-16 9:26 ` Ingo Molnar [this message]
2013-10-16 14:27 ` Christoph Lameter
2013-10-16 14:37 ` Eric Dumazet
2013-10-15 17:47 ` [PATCH 2/6] percpu: Add raw_cpu_ops Christoph Lameter
2013-10-15 17:47 ` [PATCH 3/6] mm: Use raw_cpu ops for determining current NUMA node Christoph Lameter
2013-10-16 8:38 ` Peter Zijlstra
2013-10-16 14:22 ` Christoph Lameter
2013-10-15 17:47 ` [PATCH 4/6] Use raw_cpu_write for initialization of per cpu refcount Christoph Lameter
2013-10-16 8:43 ` Peter Zijlstra
2013-10-15 17:47 ` [PATCH 5/6] net: __this_cpu_inc in route.c Christoph Lameter
2013-10-16 8:46 ` Peter Zijlstra
2013-10-16 9:22 ` Eric Dumazet
2013-10-16 10:25 ` Peter Zijlstra
2013-10-16 15:07 ` Christoph Lameter
2013-10-15 17:47 ` [PATCH 6/6] percpu: Add preemption checks to __this_cpu ops Christoph Lameter
2013-10-16 8:49 ` Peter Zijlstra
2013-10-16 15:09 ` Christoph Lameter
2013-10-16 15:36 ` Peter Zijlstra
2013-10-16 15:55 ` Christoph Lameter
2013-10-16 16:25 ` Peter Zijlstra
2013-10-16 16:52 ` Steven Rostedt
2013-10-16 17:11 ` Peter Zijlstra
2013-10-16 17:39 ` Steven Rostedt
2013-10-16 18:38 ` Peter Zijlstra
2013-10-17 19:22 ` Christoph Lameter
2013-10-17 21:13 ` Peter Zijlstra
[not found] <20131011175518.634285474@linux.com>
2013-10-11 17:54 ` [PATCH 1/6] net: ip4_datagram_connect: Use correct form of statistics update Christoph Lameter
2013-10-11 18:32 ` 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=20131016092601.GB23440@gmail.com \
--to=mingo@kernel.org \
--cc=akpm@linuxfoundation.org \
--cc=cl@linux.com \
--cc=davem@davemloft.net \
--cc=edumazet@google.com \
--cc=eric.dumazet@gmail.com \
--cc=linux-kernel@vger.kernel.org \
--cc=peterz@infradead.org \
--cc=rostedt@goodmis.org \
--cc=tglx@linutronix.de \
--cc=tj@kernel.org \
/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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.