* recursive slqb_lock
@ 2009-02-26 5:34 Wu Fengguang
2009-03-01 15:07 ` Nick Piggin
0 siblings, 1 reply; 4+ messages in thread
From: Wu Fengguang @ 2009-02-26 5:34 UTC (permalink / raw)
To: Nick Piggin; +Cc: linux-kernel
Hi Nick,
I got a lockdep warning. It looks like the slqb_lock will be taken
twice in the call chain:
s_start() => take slqb_lock
s_show()
gather_stats() => take slqb_lock again
Thanks,
Fengguang
---
[ 2229.013986] =============================================
[ 2229.017469] [ INFO: possible recursive locking detected ]
[ 2229.017469] 2.6.29-rc5-next-20090220 #57
[ 2229.017469] ---------------------------------------------
[ 2229.017469] mcookie/3167 is trying to acquire lock:
[ 2229.017469] (slqb_lock){+++++.}, at: [<ffffffff810d7960>] s_show+0x60/0x1f0
[ 2229.017469]
[ 2229.017469] but task is already holding lock:
[ 2229.017469] (slqb_lock){+++++.}, at: [<ffffffff810d7c09>] s_start+0x29/0xa0
[ 2229.017469]
[ 2229.017469] other info that might help us debug this:
[ 2229.017469] 2 locks held by mcookie/3167:
[ 2229.017469] #0: (&p->lock){+.+.+.}, at: [<ffffffff810fa79a>] seq_read+0x3a/0x3c0
[ 2229.017469] #1: (slqb_lock){+++++.}, at: [<ffffffff810d7c09>] s_start+0x29/0xa0
[ 2229.017469]
[ 2229.017469] stack backtrace:
[ 2229.017469] Pid: 3167, comm: mcookie Not tainted 2.6.29-rc5-next-20090220 #57
[ 2229.017469] Call Trace:
[ 2229.017469] [<ffffffff810705f0>] __lock_acquire+0xf90/0x1ad0
[ 2229.017469] [<ffffffff8106f605>] ? check_irq_usage+0xa5/0x100
[ 2229.017469] [<ffffffff8106c97c>] ? lockdep_init_map+0x4c/0x640
[ 2229.017469] [<ffffffff81071194>] lock_acquire+0x64/0x90
[ 2229.017469] [<ffffffff810d7960>] ? s_show+0x60/0x1f0
[ 2229.017469] [<ffffffff8149666b>] down_read+0x4b/0x80
[ 2229.017469] [<ffffffff810d7960>] ? s_show+0x60/0x1f0
[ 2229.017469] [<ffffffff810d7960>] s_show+0x60/0x1f0
[ 2229.017469] [<ffffffff810fa84b>] seq_read+0xeb/0x3c0
[ 2229.017469] [<ffffffff811ffead>] ? _raw_spin_unlock+0x7d/0xa0
[ 2229.017469] [<ffffffff810fa760>] ? seq_read+0x0/0x3c0
[ 2229.017469] [<ffffffff8111f469>] proc_reg_read+0x79/0xb0
[ 2229.017469] [<ffffffff810decc8>] vfs_read+0xc8/0x180
[ 2229.017469] [<ffffffff810dee70>] sys_read+0x50/0x90
[ 2229.017469] [<ffffffff8100c3f2>] system_call_fastpath+0x16/0x1b
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: recursive slqb_lock 2009-02-26 5:34 recursive slqb_lock Wu Fengguang @ 2009-03-01 15:07 ` Nick Piggin 2009-03-01 17:00 ` Nick Piggin 0 siblings, 1 reply; 4+ messages in thread From: Nick Piggin @ 2009-03-01 15:07 UTC (permalink / raw) To: Wu Fengguang; +Cc: linux-kernel, Pekka Enberg On Thu, Feb 26, 2009 at 01:34:50PM +0800, Wu Fengguang wrote: > Hi Nick, > > I got a lockdep warning. It looks like the slqb_lock will be taken > twice in the call chain: > > s_start() => take slqb_lock > s_show() > gather_stats() => take slqb_lock again Hey, thanks for this. The following patch should fix it. -- Fix the lockdep error reported by Fengguang where down_read slqb_lock is taken twice, with the possibility for a deadlock. Signed-off-by: Nick Piggin <npiggin@suse.de> --- Index: linux-2.6/mm/slqb.c =================================================================== --- linux-2.6.orig/mm/slqb.c 2009-03-02 02:05:16.000000000 +1100 +++ linux-2.6/mm/slqb.c 2009-03-02 02:05:45.000000000 +1100 @@ -3079,7 +3079,9 @@ static void __gather_stats(void *arg) spin_unlock(&gather->lock); } -static void gather_stats(struct kmem_cache *s, struct stats_gather *stats) +/* must be called with slqb_lock held */ +static void gather_stats_locked(struct kmem_cache *s, + struct stats_gather *stats) { #ifdef CONFIG_NUMA int node; @@ -3089,8 +3091,6 @@ static void gather_stats(struct kmem_cac stats->s = s; spin_lock_init(&stats->lock); - down_read(&slqb_lock); /* hold off hotplug */ - on_each_cpu(__gather_stats, stats, 1); #ifdef CONFIG_NUMA @@ -3119,10 +3119,15 @@ static void gather_stats(struct kmem_cac } #endif - up_read(&slqb_lock); - stats->nr_objects = stats->nr_slabs * s->objects; } + +static void gather_stats(struct kmem_cache *s, struct stats_gather *stats) +{ + down_read(&slqb_lock); /* hold off hotplug */ + gather_stats(s, stats); + up_read(&slqb_lock); +} #endif /* @@ -3175,7 +3180,7 @@ static int s_show(struct seq_file *m, vo s = list_entry(p, struct kmem_cache, list); - gather_stats(s, &stats); + gather_stats_locked(s, &stats); seq_printf(m, "%-17s %6lu %6lu %6u %4u %4d", s->name, stats.nr_inuse, stats.nr_objects, s->size, s->objects, (1 << s->order)); ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: recursive slqb_lock 2009-03-01 15:07 ` Nick Piggin @ 2009-03-01 17:00 ` Nick Piggin 2009-03-02 3:36 ` Wu Fengguang 0 siblings, 1 reply; 4+ messages in thread From: Nick Piggin @ 2009-03-01 17:00 UTC (permalink / raw) To: Wu Fengguang; +Cc: linux-kernel, Pekka Enberg On Sun, Mar 01, 2009 at 04:07:25PM +0100, Nick Piggin wrote: > On Thu, Feb 26, 2009 at 01:34:50PM +0800, Wu Fengguang wrote: > > Hi Nick, > > > > I got a lockdep warning. It looks like the slqb_lock will be taken > > twice in the call chain: > > > > s_start() => take slqb_lock > > s_show() > > gather_stats() => take slqb_lock again > > Hey, thanks for this. The following patch should fix it. > > -- > Fix the lockdep error reported by Fengguang where down_read slqb_lock > is taken twice, with the possibility for a deadlock. > > Signed-off-by: Nick Piggin <npiggin@suse.de> > --- > Index: linux-2.6/mm/slqb.c > =================================================================== > --- linux-2.6.orig/mm/slqb.c 2009-03-02 02:05:16.000000000 +1100 > +++ linux-2.6/mm/slqb.c 2009-03-02 02:05:45.000000000 +1100 > @@ -3079,7 +3079,9 @@ static void __gather_stats(void *arg) > spin_unlock(&gather->lock); > } > > -static void gather_stats(struct kmem_cache *s, struct stats_gather *stats) > +/* must be called with slqb_lock held */ > +static void gather_stats_locked(struct kmem_cache *s, > + struct stats_gather *stats) > { > #ifdef CONFIG_NUMA > int node; > @@ -3089,8 +3091,6 @@ static void gather_stats(struct kmem_cac > stats->s = s; > spin_lock_init(&stats->lock); > > - down_read(&slqb_lock); /* hold off hotplug */ > - > on_each_cpu(__gather_stats, stats, 1); > > #ifdef CONFIG_NUMA > @@ -3119,10 +3119,15 @@ static void gather_stats(struct kmem_cac > } > #endif > > - up_read(&slqb_lock); > - > stats->nr_objects = stats->nr_slabs * s->objects; > } > + > +static void gather_stats(struct kmem_cache *s, struct stats_gather *stats) > +{ > + down_read(&slqb_lock); /* hold off hotplug */ > + gather_stats(s, stats); > + up_read(&slqb_lock); > +} Oops, I didn't test it. That call should be to gather_stats_locked of course. ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: recursive slqb_lock 2009-03-01 17:00 ` Nick Piggin @ 2009-03-02 3:36 ` Wu Fengguang 0 siblings, 0 replies; 4+ messages in thread From: Wu Fengguang @ 2009-03-02 3:36 UTC (permalink / raw) To: Nick Piggin; +Cc: linux-kernel, Pekka Enberg Hi Nick, On Sun, Mar 01, 2009 at 07:00:15PM +0200, Nick Piggin wrote: > On Sun, Mar 01, 2009 at 04:07:25PM +0100, Nick Piggin wrote: > > On Thu, Feb 26, 2009 at 01:34:50PM +0800, Wu Fengguang wrote: > > > Hi Nick, > > > > > > I got a lockdep warning. It looks like the slqb_lock will be taken > > > twice in the call chain: > > > > > > s_start() => take slqb_lock > > > s_show() > > > gather_stats() => take slqb_lock again > > > > Hey, thanks for this. The following patch should fix it. > > > > -- > > Fix the lockdep error reported by Fengguang where down_read slqb_lock > > is taken twice, with the possibility for a deadlock. > > > > Signed-off-by: Nick Piggin <npiggin@suse.de> > > +static void gather_stats(struct kmem_cache *s, struct stats_gather *stats) > > +{ > > + down_read(&slqb_lock); /* hold off hotplug */ > > + gather_stats(s, stats); > > + up_read(&slqb_lock); > > +} > > Oops, I didn't test it. That call should be to gather_stats_locked of > course. Yeah it fixed the lockdep warning :) Thanks, Fengguang ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2009-03-02 3:37 UTC | newest] Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2009-02-26 5:34 recursive slqb_lock Wu Fengguang 2009-03-01 15:07 ` Nick Piggin 2009-03-01 17:00 ` Nick Piggin 2009-03-02 3:36 ` Wu Fengguang
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).