kernel-testers.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* SLUB/debugobjects locking (Re: 2.6.27-rc4-git1: Reported regressions from 2.6.26)
@ 2008-08-25 19:44 Vegard Nossum
       [not found] ` <19f34abd0808251244v439e78b1hbb24f77c637559c3-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
  0 siblings, 1 reply; 4+ messages in thread
From: Vegard Nossum @ 2008-08-25 19:44 UTC (permalink / raw)
  To: Daniel J Blueman
  Cc: Linus Torvalds, Rafael J. Wysocki, Thomas Gleixner, Ingo Molnar,
	Linux Kernel Mailing List, Adrian Bunk, Andrew Morton,
	Natalie Protasevich, Kernel Testers List

[-- Attachment #1: Type: text/plain, Size: 1500 bytes --]

On Mon, Aug 25, 2008 at 3:03 PM, Daniel J Blueman
<daniel.blueman-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> Hi Linus, Vegard,
>
> On Sun, Aug 24, 2008 at 7:58 PM, Linus Torvalds
> <torvalds-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b@public.gmane.org> wrote:
>> On Sun, 24 Aug 2008, Vegard Nossum wrote:
> [snip]
>> Anyway, I think your patch is likely fine, I just thought it looked a bit
>> odd to have a loop to move a list from one head pointer to another.
>>
>> But regardless, it would need some testing. Daniel?
>
> This opens another lockdep report at boot-time [1] - promoting
> pool_lock may not be the best fix?
>
> We then see a new deadlock condition (on the pool_lock spinlock) [2],
> which seemingly was avoided by taking the debug-bucket lock first.
>
> We reproduce this by booting with debug_objects=1 and causing a lot of activity.

Thanks. I get the same thing here as well.

I tried your suggestion of promoting the lock to irq-safe, and it
fixed the warning for me (didn't get or look for deadlocks yet, but it
seems likely that it is caused by the same thing?), the patch is
attached for reference.

I also don't know if this is the best fix, but I also don't have any
other (better) suggestions.

Others are welcome to pick it up from here...


Vegard

-- 
"The animistic metaphor of the bug that maliciously sneaked in while
the programmer was not looking is intellectually dishonest as it
disguises that the error is the programmer's own creation."
	-- E. W. Dijkstra, EWD1036

[-- Attachment #2: promoted.patch --]
[-- Type: application/octet-stream, Size: 1577 bytes --]

diff --git a/lib/debugobjects.c b/lib/debugobjects.c
index 9d8161f..ac46405 100644
--- a/lib/debugobjects.c
+++ b/lib/debugobjects.c
@@ -116,9 +116,10 @@ static struct debug_obj *lookup_object(void *addr, struct debug_bucket *b)
 static struct debug_obj *
 alloc_object(void *addr, struct debug_bucket *b, struct debug_obj_descr *descr)
 {
+	unsigned long flags;
 	struct debug_obj *obj = NULL;
 
-	spin_lock(&pool_lock);
+	spin_lock_irqsave(&pool_lock, flags);
 	if (obj_pool.first) {
 		obj	    = hlist_entry(obj_pool.first, typeof(*obj), node);
 
@@ -137,7 +138,7 @@ alloc_object(void *addr, struct debug_bucket *b, struct debug_obj_descr *descr)
 		if (obj_pool_free < obj_pool_min_free)
 			obj_pool_min_free = obj_pool_free;
 	}
-	spin_unlock(&pool_lock);
+	spin_unlock_irqrestore(&pool_lock, flags);
 
 	return obj;
 }
@@ -147,18 +148,19 @@ alloc_object(void *addr, struct debug_bucket *b, struct debug_obj_descr *descr)
  */
 static void free_object(struct debug_obj *obj)
 {
+	unsigned long flags;
 	unsigned long idx = (unsigned long)(obj - obj_static_pool);
 
 	if (obj_pool_free < ODEBUG_POOL_SIZE || idx < ODEBUG_POOL_SIZE) {
-		spin_lock(&pool_lock);
+		spin_lock_irqsave(&pool_lock, flags);
 		hlist_add_head(&obj->node, &obj_pool);
 		obj_pool_free++;
 		obj_pool_used--;
-		spin_unlock(&pool_lock);
+		spin_unlock_irqrestore(&pool_lock, flags);
 	} else {
-		spin_lock(&pool_lock);
+		spin_lock_irqsave(&pool_lock, flags);
 		obj_pool_used--;
-		spin_unlock(&pool_lock);
+		spin_unlock_irqrestore(&pool_lock, flags);
 		kmem_cache_free(obj_cache, obj);
 	}
 }

^ permalink raw reply related	[flat|nested] 4+ messages in thread
end of thread, other threads:[~2008-08-28 13:56 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2008-08-25 19:44 SLUB/debugobjects locking (Re: 2.6.27-rc4-git1: Reported regressions from 2.6.26) Vegard Nossum
     [not found] ` <19f34abd0808251244v439e78b1hbb24f77c637559c3-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2008-08-26 22:14   ` Daniel J Blueman
     [not found]     ` <6278d2220808261514p2661251aw914215652c547125-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2008-08-28 13:42       ` Vegard Nossum
2008-08-28 13:56         ` Ingo Molnar

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).