public inbox for netdev@vger.kernel.org
 help / color / mirror / Atom feed
From: Peter Zijlstra <peterz@infradead.org>
To: Andrew Morton <akpm@linux-foundation.org>
Cc: David Miller <davem@davemloft.net>,
	herbert@gondor.hengli.com.au, mst@redhat.com, frzhang@redhat.com,
	netdev@vger.kernel.org, amwang@redhat.com, shemminger@vyatta.com,
	mpm@selenic.com, paulmck@linux.vnet.ibm.com, mingo@elte.hu
Subject: Re: [PATCH 6/8] netpoll: Allow netpoll_setup/cleanup recursion
Date: Fri, 25 Jun 2010 10:08:56 +0200	[thread overview]
Message-ID: <1277453336.22715.2154.camel@twins> (raw)
In-Reply-To: <20100624214204.a85c8ba2.akpm@linux-foundation.org>

On Thu, 2010-06-24 at 21:42 -0700, Andrew Morton wrote:
> That being said, I wonder why Herbert didn't hit this in his testing. 
> I suspect that he'd enabled lockdep, which hid the bug.  I haven't
> worked out _why_ lockdep hides the double-mutex_unlock bug, but it's a
> pretty bad thing to do. 

Most weird indeed, lockdep is supposed so shout its lungs out when
someone wants to unlock a lock that isn't actually owned by him (and it
not being locked at all certainly implies you're not the owner).

In fact, the below patch results in the below splat -- its also
something that's tested by the locking self-test:

/*
 * Double unlock:
 */
#define E()                                     \
                                                \
        LOCK(A);                                \
        UNLOCK(A);                              \
        UNLOCK(A); /* fail */


---
 kernel/timer.c |    6 ++++++
 1 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/kernel/timer.c b/kernel/timer.c
index ee3f116..0496f71 100644
--- a/kernel/timer.c
+++ b/kernel/timer.c
@@ -1334,6 +1334,8 @@ SYSCALL_DEFINE0(getpid)
 	return task_tgid_vnr(current);
 }
 
+static DEFINE_MUTEX(foo);
+
 /*
  * Accessing ->real_parent is not SMP-safe, it could
  * change from under us. However, we can use a stale
@@ -1344,6 +1346,10 @@ SYSCALL_DEFINE0(getppid)
 {
 	int pid;
 
+	mutex_lock(&foo);
+	mutex_unlock(&foo);
+	mutex_unlock(&foo);
+
 	rcu_read_lock();
 	pid = task_tgid_vnr(current->real_parent);
 	rcu_read_unlock();



=====================================
[ BUG: bad unlock balance detected! ]
-------------------------------------
init/1 is trying to release lock (foo) at:
[<ffffffff815bf3b6>] mutex_unlock+0xe/0x10
but there are no more locks to release!

other info that might help us debug this:
no locks held by init/1.

stack backtrace:
Pid: 1, comm: init Not tainted 2.6.35-rc3-tip-01034-g5205803-dirty #399
Call Trace:
 [<ffffffff815bf3b6>] ? mutex_unlock+0xe/0x10
 [<ffffffff8106d718>] print_unlock_inbalance_bug+0xd6/0xe1
 [<ffffffff815bf3b6>] ? mutex_unlock+0xe/0x10
 [<ffffffff8106e8c6>] lock_release+0xdb/0x196
 [<ffffffff815bf32f>] __mutex_unlock_slowpath+0xc1/0x13a
 [<ffffffff815bf3b6>] mutex_unlock+0xe/0x10
 [<ffffffff8104f262>] sys_getppid+0x34/0xd8
 [<ffffffff81002cdb>] system_call_fastpath+0x16/0x1b


  parent reply	other threads:[~2010-06-25  8:17 UTC|newest]

Thread overview: 63+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-06-10 12:40 [0/8] netpoll/bridge fixes Herbert Xu
2010-06-10 12:42 ` [PATCH 1/7] netpoll: Set npinfo to NULL even with ndo_netpoll_cleanup Herbert Xu
2010-06-10 12:42 ` [PATCH 2/7] bridge: Remove redundant npinfo NULL setting Herbert Xu
2010-06-10 12:42 ` [PATCH 3/7] netpoll: Fix RCU usage Herbert Xu
2010-06-10 12:42 ` [PATCH 4/7] netpoll: Add locking for netpoll_setup/cleanup Herbert Xu
2010-06-10 12:42 ` [PATCH 5/7] netpoll: Add ndo_netpoll_setup Herbert Xu
2010-06-10 12:42 ` [PATCH 6/7] netpoll: Allow netpoll_setup/cleanup recursion Herbert Xu
2010-06-10 12:42 ` [PATCH 7/7] bridge: Fix netpoll support Herbert Xu
2010-06-10 14:49 ` [0/8] netpoll/bridge fixes Stephen Hemminger
2010-06-10 21:56   ` Herbert Xu
2010-06-10 21:59     ` Stephen Hemminger
2010-06-10 22:48       ` Herbert Xu
2010-06-11  2:11         ` Herbert Xu
2010-06-11  2:12           ` [PATCH 1/8] netpoll: Set npinfo to NULL even with ndo_netpoll_cleanup Herbert Xu
2010-06-11  2:12           ` [PATCH 2/8] bridge: Remove redundant npinfo NULL setting Herbert Xu
2010-06-11  2:12           ` [PATCH 3/8] netpoll: Fix RCU usage Herbert Xu
2010-06-11 23:10             ` Paul E. McKenney
2010-06-11  2:12           ` [PATCH 4/8] netpoll: Add locking for netpoll_setup/cleanup Herbert Xu
2010-06-11  2:12           ` [PATCH 5/8] netpoll: Add ndo_netpoll_setup Herbert Xu
2010-06-11  2:12           ` [PATCH 6/8] netpoll: Allow netpoll_setup/cleanup recursion Herbert Xu
2010-06-25  1:21             ` Andrew Morton
2010-06-25  3:01               ` Herbert Xu
2010-06-25  3:30               ` David Miller
2010-06-25  3:50                 ` Andrew Morton
2010-06-25  4:27                   ` David Miller
2010-06-25  4:42                     ` Andrew Morton
2010-06-25  4:52                       ` David Miller
2010-06-25  8:08                       ` Peter Zijlstra [this message]
2010-06-25  8:42                         ` Andrew Morton
2010-06-25  9:45                           ` Peter Zijlstra
2010-06-25  8:46                       ` Ingo Molnar
2010-06-25 10:08                       ` Nick Piggin
2010-06-11  2:12           ` [PATCH 7/8] netpoll: Add netpoll_tx_running Herbert Xu
2010-06-11  2:12           ` [PATCH 8/8] bridge: Fix netpoll support Herbert Xu
2010-06-11  3:08             ` fired a bug report on bugzilla.redhat.com Qianfeng Zhang
2010-06-15 10:28             ` [PATCH 8/8] bridge: Fix netpoll support Cong Wang
2010-06-17 10:38               ` Herbert Xu
2010-06-17 10:57                 ` Cong Wang
2010-06-17 10:55                   ` Herbert Xu
2010-06-18  3:06                     ` Cong Wang
2010-06-11 20:03           ` [0/8] netpoll/bridge fixes Matt Mackall
2010-06-15 10:17           ` Cong Wang
2010-06-15 18:39           ` David Miller
2010-06-16  2:58             ` Eric Dumazet
2010-06-16  3:03               ` Eric Dumazet
2010-06-16  3:33                 ` Herbert Xu
2010-06-16  4:47                   ` David Miller
2010-06-16 23:02                     ` Paul E. McKenney
2010-06-17 10:18                       ` Michael S. Tsirkin
2010-06-17 21:26                         ` Paul E. McKenney
2010-06-16  6:16                   ` Eric Dumazet
2010-06-16  5:08               ` Paul E. McKenney
2010-06-16  6:21                 ` Eric Dumazet
2010-06-16 16:01                   ` Paul E. McKenney
2010-07-19 10:19           ` Michael S. Tsirkin
2010-07-19 10:53             ` Herbert Xu
2010-07-19 11:54               ` Herbert Xu
2010-07-19 16:05                 ` David Miller
2010-07-19 16:52                   ` Eric Dumazet
2010-07-19 20:35                     ` David Miller
2010-07-20  5:26                   ` Herbert Xu
2010-07-20  6:28                     ` David Miller
2010-06-29 12:53 ` Yanko Kaneti

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=1277453336.22715.2154.camel@twins \
    --to=peterz@infradead.org \
    --cc=akpm@linux-foundation.org \
    --cc=amwang@redhat.com \
    --cc=davem@davemloft.net \
    --cc=frzhang@redhat.com \
    --cc=herbert@gondor.hengli.com.au \
    --cc=mingo@elte.hu \
    --cc=mpm@selenic.com \
    --cc=mst@redhat.com \
    --cc=netdev@vger.kernel.org \
    --cc=paulmck@linux.vnet.ibm.com \
    --cc=shemminger@vyatta.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