2.6.23-rc1: known regressions with patches

PATCH New Michal Piotrowski 8 participants 19 messages Jul 23, 2007 – Jul 26, 2007

Subsystems (3)

Memory management VFS Slab allocator

lore

Share & tools

lore

Michal Piotrowski

author

2007-07-23 09:49 UTC #0 lore raw

Hi all,

Here is a list of some known regressions in 2.6.23-rc1 with patches available.

Feel free to add new regressions/remove fixed etc.
http://kernelnewbies.org/known_regressions

List of Aces

Name Regressions fixed since 21-Jun-2007

Andi Kleen                             4
Linus Torvalds                         4
Adrian Bunk                            3
Andrew Morton                          3
Jens Axboe                             3
Al Viro                                2
David Woodhouse                        2
Hugh Dickins                           2
Tejun Heo                              2

Unclassified

Subject : Regression with sys_time() speedup patch
References : http://lkml.org/lkml/2007/7/20/205
Last known good : ?
Submitter : Takashi Iwai <[email protected]>

Caused-By       : ?
Handled-By      : Ingo Molnar <[email protected]>
Patch           : http://lkml.org/lkml/2007/7/20/219
Status          : patch available

Subject : kobject link failure
References : http://lkml.org/lkml/2007/7/19/495
Last known good : ?
Submitter : Jan Engelhardt <[email protected]>

Caused-By ?

Handled-By Cornelia Huck <[email protected]>

Greg Kroah-Hartman <[email protected]>
Patch : http://lkml.org/lkml/2007/7/20/143
Status : patch available

Subject : Kconfig prompts without help text
References : http://lkml.org/lkml/2007/7/16/326
Last known good : ?
Submitter : Stefan Richter <[email protected]>

Caused-By       : ?
Handled-By      : Jan Engelhardt <[email protected]>
Patch           : http://lkml.org/lkml/2007/7/18/236
Status          : patch available

Subject : Oops while modprobing phy fixed module
References : http://lkml.org/lkml/2007/7/14/63
Last known good : ?
Submitter : Gabriel C <[email protected]>

Caused-By Tejun Heo <[email protected]>

                  commit 3007e997de91ec59af39a3f9c91595b31ae6e08b

Handled-By Satyam Sharma <[email protected]>

Tejun Heo <[email protected]> Vitaly Bordug <[email protected]>
Patch1 : http://lkml.org/lkml/2007/7/18/506
Status : patch available

Subject : ia64 build failure from recent diskquota patch
References : http://lkml.org/lkml/2007/7/18/407
Last known good : ?
Submitter : Doug Chapman <[email protected]>

Caused-By Vasily Tarasov <[email protected]>

                  commit b716395e2b8e450e294537de0c91476ded2f0395
Handled-By      : Luck, Tony <[email protected]>
Patch1          : http://lkml.org/lkml/2007/7/20/255
Patch2          : http://lkml.org/lkml/2007/7/20/272
Status          : patch available

Media

Subject : usbvision: don't return an uninitialized value
References : http://lkml.org/lkml/2007/7/23/65
Last known good : ?
Submitter : Adrian Bunk <[email protected]>

Caused-By       : ?
Handled-By      : Adrian Bunk <[email protected]>
Patch           : http://lkml.org/lkml/2007/7/23/65
Status          : patch available

Regards,
Michal

--
LOG
http://www.stardust.webpages.pl/log/

Ingo Molnar

2007-07-23 10:25 UTC #1 lore raw

Quoted 8 lines · Michal Piotrowski

Subject : Regression with sys_time() speedup patch
References : http://lkml.org/lkml/2007/7/20/205
Last known good : ?
Submitter : Takashi Iwai <[email protected]>
Caused-By       : ?
Handled-By      : Ingo Molnar <[email protected]>
Patch           : http://lkml.org/lkml/2007/7/20/219
Status          : patch available

this has been resolved in -rc1 already, by:

commit 2008220879af095d00ca27eb168a55c8595fbc0b

Ingo

Greg KH

2007-07-23 23:13 UTC #2 lore raw

In reply to Michal Piotrowski

Quoted 5 lines · Michal Piotrowski

Unclassified
Subject : kobject link failure
References : http://lkml.org/lkml/2007/7/19/495
Last known good : ?

This is caused by a patch that happened after 2.6.22 was released, so it is a regression.

Quoted 6 lines

Submitter       : Jan Engelhardt <[email protected]>
Caused-By       : ?
Handled-By      : Cornelia Huck <[email protected]>
Greg Kroah-Hartman <[email protected]>
Patch : http://lkml.org/lkml/2007/7/20/143
Status : patch available

I'll be sending Cornelia's patch to Linus within the week to fix this.

thanks,

greg k-h

Tejun Heo

2007-07-24 06:21 UTC #4 lore raw

In reply to Michal Piotrowski

Quoted 11 lines · Michal Piotrowski

Subject : Oops while modprobing phy fixed module
References : http://lkml.org/lkml/2007/7/14/63
Last known good : ?
Submitter : Gabriel C <[email protected]>
Caused-By Tejun Heo <[email protected]>
                  commit 3007e997de91ec59af39a3f9c91595b31ae6e08b
Handled-By Satyam Sharma <[email protected]>
Tejun Heo <[email protected]> Vitaly Bordug <[email protected]>
Patch1 : http://lkml.org/lkml/2007/7/18/506
Status : patch available

Patch is in mainline. Commit a1da4dfe35bc36c3bc9716d995c85b7983c38a76.

Thanks.

Signature · 1 line

tejun

Michal Piotrowski

author

2007-07-24 21:57 UTC #3 lore raw

In reply to Greg KH

Quoted 9 lines · Greg KH

Quoted 5 lines · Michal Piotrowski
Unclassified
Subject : kobject link failure
References : http://lkml.org/lkml/2007/7/19/495
Last known good : ?
This is caused by a patch that happened after 2.6.22 was released, so it is a regression.

Yes, I know. "?" is a default value. When someone says that bug appeared after 2.6.22-git7 I'm adding this information to "Last known good".

Quoted 8 lines

Quoted 7 lines
Submitter       : Jan Engelhardt <[email protected]>
Caused-By       : ?
Handled-By      : Cornelia Huck <[email protected]>
Greg Kroah-Hartman <[email protected]>
Patch : http://lkml.org/lkml/2007/7/20/143
Status : patch available

Quoted 6 lines

I'll be sending Cornelia's patch to Linus within the week to fix this.
thanks,
greg k-h

Regards,
Michal

Signature · 2 lines

LOG
http://www.stardust.webpages.pl/log/

Ingo Molnar

2007-07-26 09:11 UTC #5 lore raw

[bug] SLUB & freeing locks [was: Re: 2.6.23-rc1: known regressions with patches]

In reply to Michal Piotrowski

there's a new regression:

SLUB does not seem to call debug_check_no_locks_freed() - hence on SLUB we will not notice certain types of freeing races. (such as freeing an object still locked and used on another CPU.) SLAB does this corretly.

Ingo

Peter Zijlstra

2007-07-26 09:26 UTC #6 lore raw

Re: [bug] SLUB & freeing locks [was: Re: 2.6.23-rc1: known regressions with patches]

Quoted 5 lines · Ingo Molnar

there's a new regression:
SLUB does not seem to call debug_check_no_locks_freed() - hence on SLUB we will not notice certain types of freeing races. (such as freeing an object still locked and used on another CPU.) SLAB does this corretly.

Does this help?

---
 mm/slub.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

mm/slub.c +1 -1

Index: linux-2.6/mm/slub.c
===================================================================
@@ -1131,6 +1131,7 @@ static void __free_slab(struct kmem_cach
 		slab_pad_check(s, page);
 		for_each_object(p, s, page_address(page))
 			check_object(s, page, p, 0);
+		ClearSlabDebug(page);
 	}
 
 	mod_zone_page_state(page_zone(page),

@@ -1169,7 +1170,6 @@ static void discard_slab(struct kmem_cac
 
 	atomic_long_dec(&n->nr_slabs);
 	reset_page_mapcount(page);
-	ClearSlabDebug(page);
 	__ClearPageSlab(page);
 	free_slab(s, page);
 }

Peter Zijlstra

2007-07-26 09:32 UTC #7 lore raw

Re: [bug] SLUB & freeing locks [was: Re: 2.6.23-rc1: known regressions with patches]

Quoted 6 lines · Peter Zijlstra

Quoted 5 lines · Ingo Molnar
there's a new regression:
SLUB does not seem to call debug_check_no_locks_freed() - hence on SLUB we will not notice certain types of freeing races. (such as freeing an object still locked and used on another CPU.) SLAB does this corretly.

Oh, nm, it doesn't call debug_check_no_locks_freed() at all.

This should do I guess.

Signed-off-by Peter Zijlstra <[email protected]>

---
 mm/slub.c |    1 +
 1 file changed, 1 insertion(+)

mm/slub.c +1 -0

Index: linux-2.6/mm/slub.c
===================================================================
@@ -1656,6 +1656,7 @@ static void __always_inline slab_free(st
 	unsigned long flags;
 
 	local_irq_save(flags);
+	debug_check_no_locks_freed(object, s->size);
 	if (likely(page == s->cpu_slab[smp_processor_id()] &&
 						!SlabDebug(page))) {
 		object[page->offset] = page->lockless_freelist;

Ingo Molnar

Acked-by

2007-07-26 10:50 UTC #8 lore raw

Re: [bug] SLUB & freeing locks [was: Re: 2.6.23-rc1: known regressions with patches]

Quoted 8 lines · Peter Zijlstra

Quoted 6 lines · Peter Zijlstra
Quoted 5 lines · Ingo Molnar
there's a new regression:
SLUB does not seem to call debug_check_no_locks_freed() - hence on SLUB we will not notice certain types of freeing races. (such as freeing an object still locked and used on another CPU.) SLAB does this corretly.

Quoted 5 lines

Oh, nm, it doesn't call debug_check_no_locks_freed() at all.
This should do I guess.
Signed-off-by Peter Zijlstra <[email protected]>

yeah.

Acked-by Ingo Molnar <[email protected]>

Ingo

Pekka Enberg

2007-07-26 11:08 UTC #10 lore raw

Re: [bug] SLUB & freeing locks [was: Re: 2.6.23-rc1: known regressions with patches]

In reply to Peter Zijlstra

mm/slub.c static void __always_inline slab_free(st · line 1659

Quoted 7 lines · Peter Zijlstra

mm/slub.c +1 -0

@@ -1656,6 +1656,7 @@ static void __always_inline slab_free(st
        unsigned long flags;
 
        local_irq_save(flags);
+       debug_check_no_locks_freed(object, s->size);

Although it doesn't matter much, I think this should be s->objsize. Christoph?

Pekka

Peter Zijlstra

2007-07-26 11:11 UTC #11 lore raw

Re: [bug] SLUB & freeing locks [was: Re: 2.6.23-rc1: known regressions with patches]

Quoted 10 lines · Pekka Enberg

mm/slub.c static void __always_inline slab_free(st · line 1659
Quoted 7 lines · Peter Zijlstra
mm/slub.c +1 -0
@@ -1656,6 +1656,7 @@ static void __always_inline slab_free(st
        unsigned long flags;
 
        local_irq_save(flags);
+       debug_check_no_locks_freed(object, s->size);
Although it doesn't matter much, I think this should be s->objsize. Christoph?

Ah, I think you are right, s->size includes all the padding and redzones.

Christoph Lameter

2007-07-26 17:47 UTC #14 lore raw

Re: [bug] SLUB & freeing locks [was: Re: 2.6.23-rc1: known regressions with patches]

In reply to Pekka Enberg

Quoted 10 lines · Pekka Enberg

mm/slub.c static void __always_inline slab_free(st · line 1659
Quoted 7 lines · Peter Zijlstra
mm/slub.c +1 -0
@@ -1656,6 +1656,7 @@ static void __always_inline slab_free(st
        unsigned long flags;
 
        local_irq_save(flags);
+       debug_check_no_locks_freed(object, s->size);
Although it doesn't matter much, I think this should be s->objsize. Christoph?

Correct. Locks can only be in the first objsize range of bytes.

Christoph Lameter

2007-07-26 17:48 UTC #9 lore raw

Re: [bug] SLUB & freeing locks [was: Re: 2.6.23-rc1: known regressions with patches]

In reply to Ingo Molnar

Christoph Lameter

Acked-by

2007-07-26 17:51 UTC #12 lore raw

Re: [bug] SLUB & freeing locks [was: Re: 2.6.23-rc1: known regressions with patches]

In reply to Peter Zijlstra

Note that debug_check... adds more code to the slab_free hotpath which will have a significant performance impact if lockdep is enabled.

Acked-by Christoph Lameter <[email protected]>

Peter Zijlstra

2007-07-26 18:02 UTC #13 lore raw

PATCH 1/1 slub: add lock debugging check

Ingo noticed that the SLUB code does include the lock debugging free check.

Signed-off-by Peter Zijlstra <[email protected]>

Acked-by Ingo Molnar <[email protected]>

Acked-by Pekka Enberg <[email protected]>

Acked-by Christoph Lameter <[email protected]>

---
 mm/slub.c |    1 +
 1 file changed, 1 insertion(+)

mm/slub.c +1 -0

Index: linux-2.6/mm/slub.c
===================================================================
@@ -1656,6 +1656,7 @@ static void __always_inline slab_free(st
 	unsigned long flags;
 
 	local_irq_save(flags);
+	debug_check_no_locks_freed(object, s->objsize);
 	if (likely(page == s->cpu_slab[smp_processor_id()] &&
 						!SlabDebug(page))) {
 		object[page->offset] = page->lockless_freelist;

Peter Zijlstra

2007-07-26 18:05 UTC #15 lore raw

Re: [bug] SLUB & freeing locks [was: Re: 2.6.23-rc1: known regressions with patches]

In reply to Peter Zijlstra

Quoted 18 lines · Peter Zijlstra

---
 mm/slub.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Index: linux-2.6/mm/slub.c
===================================================================

mm/slub.c +1 -0

@@ -1131,6 +1131,7 @@ static void __free_slab(struct kmem_cach
 		slab_pad_check(s, page);
 		for_each_object(p, s, page_address(page))
 			check_object(s, page, p, 0);
+		ClearSlabDebug(page);
 	}
 
 	mod_zone_page_state(page_zone(page),

mm/slub.c static void discard_slab(struct kmem_cac · line 1175

Quoted 7 lines

mm/slub.c +0 -1

@@ -1169,7 +1170,6 @@ static void discard_slab(struct kmem_cac
 
 	atomic_long_dec(&n->nr_slabs);
 	reset_page_mapcount(page);
-	ClearSlabDebug(page);
 	__ClearPageSlab(page);
 	free_slab(s, page);
 }

Christoph, how about this bit, it looks to me like we clear SlabDebug() before we hit the last check in __free_slab().

Christoph Lameter

Acked-by

2007-07-26 18:10 UTC #16 lore raw

Re: [bug] SLUB & freeing locks [was: Re: 2.6.23-rc1: known regressions with patches]

Quoted 14 lines · Peter Zijlstra

Quoted 14 lines

Index: linux-2.6/mm/slub.c
===================================================================

mm/slub.c +1 -0

@@ -1131,6 +1131,7 @@ static void __free_slab(struct kmem_cach
 		slab_pad_check(s, page);
 		for_each_object(p, s, page_address(page))
 			check_object(s, page, p, 0);
+		ClearSlabDebug(page);
 	}
 
 	mod_zone_page_state(page_zone(page),

@@ -1169,7 +1170,6 @@ static void discard_slab(struct kmem_cac

Quoted 10 lines

mm/slub.c static void discard_slab(struct kmem_cac · line 1175
Quoted 7 lines
mm/slub.c +0 -1
 	atomic_long_dec(&n->nr_slabs);
 	reset_page_mapcount(page);
-	ClearSlabDebug(page);
 	__ClearPageSlab(page);
 	free_slab(s, page);
 }
Christoph, how about this bit, it looks to me like we clear SlabDebug() before we hit the last check in __free_slab().

Looks right. Have you tried to run a kernel with this change and "slub_debug"?

Acked-by Christoph Lameter <[email protected]>

Peter Zijlstra

2007-07-26 18:18 UTC #17 lore raw

Re: [bug] SLUB & freeing locks [was: Re: 2.6.23-rc1: known regressions with patches]

Quoted 27 lines · Christoph Lameter

Quoted 14 lines · Peter Zijlstra
Quoted 14 lines
Index: linux-2.6/mm/slub.c
===================================================================
mm/slub.c +1 -0
@@ -1131,6 +1131,7 @@ static void __free_slab(struct kmem_cach
 		slab_pad_check(s, page);
 		for_each_object(p, s, page_address(page))
 			check_object(s, page, p, 0);
+		ClearSlabDebug(page);
 	}
 
 	mod_zone_page_state(page_zone(page),
@@ -1169,7 +1170,6 @@ static void discard_slab(struct kmem_cac
 
Quoted 10 lines
mm/slub.c static void discard_slab(struct kmem_cac · line 1175
Quoted 7 lines
mm/slub.c +0 -1
 	atomic_long_dec(&n->nr_slabs);
 	reset_page_mapcount(page);
-	ClearSlabDebug(page);
 	__ClearPageSlab(page);
 	free_slab(s, page);
 }
Christoph, how about this bit, it looks to me like we clear SlabDebug() before we hit the last check in __free_slab().

Quoted 4 lines

Looks right. Have you tried to run a kernel with this change and "slub_debug"?
Acked-by Christoph Lameter <[email protected]>

Not something this clean, only my WIP slub-rt. I'll run this and do a proper post of this fix.

Peter Zijlstra

2007-07-26 18:55 UTC #18 lore raw

PATCH 1/1 slub: fix bug in slub debug support

Seems to properly boot

---
We ClearSlabDebug() before the last SlabDebug() check. Clear it later.

Signed-off-by Peter Zijlstra <[email protected]>

Acked-by Christoph Lameter <[email protected]>

---
 mm/slub.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

mm/slub.c +1 -1

Index: linux-2.6/mm/slub.c
===================================================================
@@ -1131,6 +1131,7 @@ static void __free_slab(struct kmem_cach
 		slab_pad_check(s, page);
 		for_each_object(p, s, page_address(page))
 			check_object(s, page, p, 0);
+		ClearSlabDebug(page);
 	}
 
 	mod_zone_page_state(page_zone(page),

@@ -1169,7 +1170,6 @@ static void discard_slab(struct kmem_cac
 
 	atomic_long_dec(&n->nr_slabs);
 	reset_page_mapcount(page);
-	ClearSlabDebug(page);
 	__ClearPageSlab(page);
 	free_slab(s, page);
 }