All of lore.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 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.