linux-kernel - Re: recursive slqb

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Sun, 1 Mar 2009 16:07:25 +0100
From:	Nick Piggin <npiggin@...e.de>
To:	Wu Fengguang <fengguang.wu@...el.com>
Cc:	linux-kernel@...r.kernel.org, Pekka Enberg <penberg@...helsinki.fi>
Subject: Re: recursive slqb_lock

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@...e.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));
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/