linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] slab_common: fix the check for duplicate slab names
@ 2014-03-04 22:13 Mikulas Patocka
  2014-03-04 22:48 ` David Rientjes
  2014-03-25 17:03 ` Mike Snitzer
  0 siblings, 2 replies; 10+ messages in thread
From: Mikulas Patocka @ 2014-03-04 22:13 UTC (permalink / raw)
  To: Christoph Lameter
  Cc: Jonathan Brassow, Alasdair G. Kergon, Pekka Enberg, linux-mm,
	dm-devel

The patch 3e374919b314f20e2a04f641ebc1093d758f66a4 is supposed to fix the
problem where kmem_cache_create incorrectly reports duplicate cache name
and fails. The problem is described in the header of that patch.

However, the patch doesn't really fix the problem because of these
reasons:

* the logic to test for debugging is reversed. It was intended to perform
  the check only if slub debugging is enabled (which implies that caches
  with the same parameters are not merged). Therefore, there should be
  #if !defined(CONFIG_SLUB) || defined(CONFIG_SLUB_DEBUG_ON)
  The current code has the condition reversed and performs the test if
  debugging is disabled.

* slub debugging may be enabled or disabled based on kernel command line,
  CONFIG_SLUB_DEBUG_ON is just the default settings. Therefore the test
  based on definition of CONFIG_SLUB_DEBUG_ON is unreliable.

This patch fixes the problem by removing the test
"!defined(CONFIG_SLUB_DEBUG_ON)". Therefore, duplicate names are never
checked if the SLUB allocator is used.

Note to stable kernel maintainers: when backporint this patch, please
backport also the patch 3e374919b314f20e2a04f641ebc1093d758f66a4.

Signed-off-by: Mikulas Patocka <mpatocka@redhat.com>
Cc: stable@vger.kernel.org	# 3.6+

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

Index: linux-3.14-rc5/mm/slab_common.c
===================================================================
--- linux-3.14-rc5.orig/mm/slab_common.c	2014-03-04 22:47:02.000000000 +0100
+++ linux-3.14-rc5/mm/slab_common.c	2014-03-04 22:47:08.000000000 +0100
@@ -56,7 +56,7 @@ static int kmem_cache_sanity_check(struc
 			continue;
 		}
 
-#if !defined(CONFIG_SLUB) || !defined(CONFIG_SLUB_DEBUG_ON)
+#if !defined(CONFIG_SLUB)
 		/*
 		 * For simplicity, we won't check this in the list of memcg
 		 * caches. We have control over memcg naming, and if there

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] slab_common: fix the check for duplicate slab names
  2014-03-04 22:13 [PATCH] slab_common: fix the check for duplicate slab names Mikulas Patocka
@ 2014-03-04 22:48 ` David Rientjes
  2014-03-25 17:03 ` Mike Snitzer
  1 sibling, 0 replies; 10+ messages in thread
From: David Rientjes @ 2014-03-04 22:48 UTC (permalink / raw)
  To: Mikulas Patocka
  Cc: Christoph Lameter, Jonathan Brassow, Alasdair G. Kergon,
	Pekka Enberg, linux-mm, dm-devel

On Tue, 4 Mar 2014, Mikulas Patocka wrote:

> The patch 3e374919b314f20e2a04f641ebc1093d758f66a4 is supposed to fix the
> problem where kmem_cache_create incorrectly reports duplicate cache name
> and fails. The problem is described in the header of that patch.
> 
> However, the patch doesn't really fix the problem because of these
> reasons:
> 
> * the logic to test for debugging is reversed. It was intended to perform
>   the check only if slub debugging is enabled (which implies that caches
>   with the same parameters are not merged). Therefore, there should be
>   #if !defined(CONFIG_SLUB) || defined(CONFIG_SLUB_DEBUG_ON)
>   The current code has the condition reversed and performs the test if
>   debugging is disabled.
> 
> * slub debugging may be enabled or disabled based on kernel command line,
>   CONFIG_SLUB_DEBUG_ON is just the default settings. Therefore the test
>   based on definition of CONFIG_SLUB_DEBUG_ON is unreliable.
> 
> This patch fixes the problem by removing the test
> "!defined(CONFIG_SLUB_DEBUG_ON)". Therefore, duplicate names are never
> checked if the SLUB allocator is used.
> 
> Note to stable kernel maintainers: when backporint this patch, please
> backport also the patch 3e374919b314f20e2a04f641ebc1093d758f66a4.
> 
> Signed-off-by: Mikulas Patocka <mpatocka@redhat.com>

Acked-by: David Rientjes <rientjes@google.com>

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: slab_common: fix the check for duplicate slab names
  2014-03-04 22:13 [PATCH] slab_common: fix the check for duplicate slab names Mikulas Patocka
  2014-03-04 22:48 ` David Rientjes
@ 2014-03-25 17:03 ` Mike Snitzer
  2014-03-25 18:07   ` Christoph Lameter
  1 sibling, 1 reply; 10+ messages in thread
From: Mike Snitzer @ 2014-03-25 17:03 UTC (permalink / raw)
  To: Christoph Lameter
  Cc: Pekka Enberg, linux-mm, dm-devel, linux-kernel,
	Alasdair G. Kergon, Mikulas Patocka

[Sorry for top-post but...]

This patch still isn't upstream.  Who should be shepherding it to Linus?

Without it we're seeing crashes on Fedora when running regression tests
that use dm-raid (dm target that wraps MD raid), e.g.:

6,484484,682379136295,-;md: mdX: recovery done.
6,484485,682379145308,-;bio: create slab <bio-0> at 0
6,484486,682379147581,-;md/raid:mdX: device dm-17 operational as raid disk 4
6,484487,682379149216,-;md/raid:mdX: device dm-11 operational as raid disk 2
6,484488,682379150829,-;md/raid:mdX: device dm-20 operational as raid disk 1
6,484489,682379152369,-;md/raid:mdX: device dm-9 operational as raid disk 0
3,484490,682379153954,-;kmem_cache_sanity_check (raid6-ffff880014e8b010): Cache name already exists.
4,484491,682379155824,-;CPU: 0 PID: 11228 Comm: lvm Not tainted 3.14.0-0.rc6.git0.1.fc21.x86_64 #1
4,484492,682379157704,-;Hardware name: Red Hat KVM, BIOS 0.5.1 01/01/2007
4,484493,682379159298,-; 0000000000000000 00000000a0bb80c5 ffff88003428d9d8 ffffffff816edd6b
4,484494,682379161238,-; ffff880076b51700 ffff88003428da50 ffffffff811982c3 0000000000000000
4,484495,682379163199,-; 0000000000000000 0000000000000790 0000000000000000 0000000000000000
4,484496,682379165206,-;Call Trace:
4,484497,682379166523,-; [<ffffffff816edd6b>] dump_stack+0x45/0x56
4,484498,682379168098,-; [<ffffffff811982c3>] kmem_cache_create_memcg+0x143/0x3e0
4,484499,682379169854,-; [<ffffffff8119858b>] kmem_cache_create+0x2b/0x30
4,484500,682379171541,-; [<ffffffffa020fc6c>] setup_conf+0x5cc/0x810 [raid456]
4,484501,682379173264,-; [<ffffffff811771ad>] ? mempool_create_node+0xdd/0x140
4,484502,682379174988,-; [<ffffffff81176dd0>] ? mempool_alloc_slab+0x20/0x20
4,484503,682379176728,-; [<ffffffffa0210a38>] run+0x868/0xa60 [raid456]
4,484504,682379178380,-; [<ffffffff81220a3e>] ? bioset_create+0x21e/0x2e0
4,484505,682379180038,-; [<ffffffff81563d3a>] md_run+0x3fa/0x980
4,484506,682379181631,-; [<ffffffff81221778>] ? bio_put+0x78/0x90
4,484507,682379183339,-; [<ffffffff8155badd>] ? sync_page_io+0x8d/0x110
4,484508,682379185000,-; [<ffffffffa0227570>] raid_ctr+0xf30/0x1389 [dm_raid]
4,484509,682379186771,-; [<ffffffff8156f857>] dm_table_add_target+0x177/0x460
4,484510,682379188538,-; [<ffffffff81572d57>] table_load+0x157/0x380
4,484511,682379190198,-; [<ffffffff81572c00>] ? retrieve_status+0x1c0/0x1c0
4,484512,682379191925,-; [<ffffffff815739c5>] ctl_ioctl+0x255/0x500
4,484513,682379193589,-; [<ffffffff811e8b00>] ? do_sync_write+0x50/0xa0
4,484514,682379195256,-; [<ffffffff81573c83>] dm_ctl_ioctl+0x13/0x20
4,484515,682379196900,-; [<ffffffff811fc790>] do_vfs_ioctl+0x2e0/0x4a0
4,484516,682379198600,-; [<ffffffff811eb931>] ? __sb_end_write+0x31/0x60
4,484517,682379200286,-; [<ffffffff811e9392>] ? vfs_write+0x172/0x1e0
4,484518,682379201957,-; [<ffffffff811fc9f1>] SyS_ioctl+0xa1/0xc0
4,484519,682379203651,-; [<ffffffff816fe129>] system_call_fastpath+0x16/0x1b
3,484520,682379205496,-;md/raid:mdX: couldn't allocate 0kB for buffers

On Tue, Mar 04 2014 at  5:13pm -0500,
Mikulas Patocka <mpatocka@redhat.com> wrote:

> The patch 3e374919b314f20e2a04f641ebc1093d758f66a4 is supposed to fix the
> problem where kmem_cache_create incorrectly reports duplicate cache name
> and fails. The problem is described in the header of that patch.
> 
> However, the patch doesn't really fix the problem because of these
> reasons:
> 
> * the logic to test for debugging is reversed. It was intended to perform
>   the check only if slub debugging is enabled (which implies that caches
>   with the same parameters are not merged). Therefore, there should be
>   #if !defined(CONFIG_SLUB) || defined(CONFIG_SLUB_DEBUG_ON)
>   The current code has the condition reversed and performs the test if
>   debugging is disabled.
> 
> * slub debugging may be enabled or disabled based on kernel command line,
>   CONFIG_SLUB_DEBUG_ON is just the default settings. Therefore the test
>   based on definition of CONFIG_SLUB_DEBUG_ON is unreliable.
> 
> This patch fixes the problem by removing the test
> "!defined(CONFIG_SLUB_DEBUG_ON)". Therefore, duplicate names are never
> checked if the SLUB allocator is used.
> 
> Note to stable kernel maintainers: when backporint this patch, please
> backport also the patch 3e374919b314f20e2a04f641ebc1093d758f66a4.
> 
> Signed-off-by: Mikulas Patocka <mpatocka@redhat.com>
> Cc: stable@vger.kernel.org	# 3.6+
> 
> ---
>  mm/slab_common.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> Index: linux-3.14-rc5/mm/slab_common.c
> ===================================================================
> --- linux-3.14-rc5.orig/mm/slab_common.c	2014-03-04 22:47:02.000000000 +0100
> +++ linux-3.14-rc5/mm/slab_common.c	2014-03-04 22:47:08.000000000 +0100
> @@ -56,7 +56,7 @@ static int kmem_cache_sanity_check(struc
>  			continue;
>  		}
>  
> -#if !defined(CONFIG_SLUB) || !defined(CONFIG_SLUB_DEBUG_ON)
> +#if !defined(CONFIG_SLUB)
>  		/*
>  		 * For simplicity, we won't check this in the list of memcg
>  		 * caches. We have control over memcg naming, and if there
> 
> --
> dm-devel mailing list
> dm-devel@redhat.com
> https://www.redhat.com/mailman/listinfo/dm-devel

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: slab_common: fix the check for duplicate slab names
  2014-03-25 17:03 ` Mike Snitzer
@ 2014-03-25 18:07   ` Christoph Lameter
  2014-05-23 20:16     ` Mike Snitzer
  0 siblings, 1 reply; 10+ messages in thread
From: Christoph Lameter @ 2014-03-25 18:07 UTC (permalink / raw)
  To: Mike Snitzer
  Cc: Pekka Enberg, linux-mm, dm-devel, linux-kernel,
	Alasdair G. Kergon, Mikulas Patocka

On Tue, 25 Mar 2014, Mike Snitzer wrote:

> This patch still isn't upstream.  Who should be shepherding it to Linus?

Pekka usually does that.

Acked-by: Christoph Lameter <cl@linux.com>

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: slab_common: fix the check for duplicate slab names
  2014-03-25 18:07   ` Christoph Lameter
@ 2014-05-23 20:16     ` Mike Snitzer
  2014-05-23 21:28       ` Pekka Enberg
  0 siblings, 1 reply; 10+ messages in thread
From: Mike Snitzer @ 2014-05-23 20:16 UTC (permalink / raw)
  To: Christoph Lameter
  Cc: Pekka Enberg, linux-mm, dm-devel, linux-kernel,
	Alasdair G. Kergon, Mikulas Patocka

On Tue, Mar 25 2014 at  2:07pm -0400,
Christoph Lameter <cl@linux.com> wrote:

> On Tue, 25 Mar 2014, Mike Snitzer wrote:
> 
> > This patch still isn't upstream.  Who should be shepherding it to Linus?
> 
> Pekka usually does that.
> 
> Acked-by: Christoph Lameter <cl@linux.com>

This still hasn't gotten upstream.

Pekka, any chance you can pick it up?  Here it is in dm-devel's
kernel.org patchwork: https://patchwork.kernel.org/patch/3768901/

(Though it looks like it needs to be rebased due to the recent commit
794b1248, should Mikulas rebase and re-send?)

Mike

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: slab_common: fix the check for duplicate slab names
  2014-05-23 20:16     ` Mike Snitzer
@ 2014-05-23 21:28       ` Pekka Enberg
  2014-05-24  0:37         ` Mike Snitzer
  2014-07-22 22:14         ` Mike Snitzer
  0 siblings, 2 replies; 10+ messages in thread
From: Pekka Enberg @ 2014-05-23 21:28 UTC (permalink / raw)
  To: Mike Snitzer, Christoph Lameter
  Cc: Pekka Enberg, linux-mm, dm-devel, linux-kernel,
	Alasdair G. Kergon, Mikulas Patocka

On 05/23/2014 11:16 PM, Mike Snitzer wrote:
> On Tue, Mar 25 2014 at  2:07pm -0400,
> Christoph Lameter <cl@linux.com> wrote:
>
>> On Tue, 25 Mar 2014, Mike Snitzer wrote:
>>
>>> This patch still isn't upstream.  Who should be shepherding it to Linus?
>> Pekka usually does that.
>>
>> Acked-by: Christoph Lameter <cl@linux.com>
> This still hasn't gotten upstream.
>
> Pekka, any chance you can pick it up?  Here it is in dm-devel's
> kernel.org patchwork: https://patchwork.kernel.org/patch/3768901/
>
> (Though it looks like it needs to be rebased due to the recent commit
> 794b1248, should Mikulas rebase and re-send?)

I applied it and fixed the conflict by hand.

Please double-check commit 694617474e33b8603fc76e090ed7d09376514b1a in 
my tree:

https://git.kernel.org/cgit/linux/kernel/git/penberg/linux.git/

- Pekka

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: slab_common: fix the check for duplicate slab names
  2014-05-23 21:28       ` Pekka Enberg
@ 2014-05-24  0:37         ` Mike Snitzer
  2014-07-22 22:14         ` Mike Snitzer
  1 sibling, 0 replies; 10+ messages in thread
From: Mike Snitzer @ 2014-05-24  0:37 UTC (permalink / raw)
  To: Pekka Enberg
  Cc: Christoph Lameter, Pekka Enberg, linux-mm, dm-devel, linux-kernel,
	Alasdair G. Kergon, Mikulas Patocka

On Fri, May 23 2014 at  5:28pm -0400,
Pekka Enberg <penberg@iki.fi> wrote:

> On 05/23/2014 11:16 PM, Mike Snitzer wrote:
> >On Tue, Mar 25 2014 at  2:07pm -0400,
> >Christoph Lameter <cl@linux.com> wrote:
> >
> >>On Tue, 25 Mar 2014, Mike Snitzer wrote:
> >>
> >>>This patch still isn't upstream.  Who should be shepherding it to Linus?
> >>Pekka usually does that.
> >>
> >>Acked-by: Christoph Lameter <cl@linux.com>
> >This still hasn't gotten upstream.
> >
> >Pekka, any chance you can pick it up?  Here it is in dm-devel's
> >kernel.org patchwork: https://patchwork.kernel.org/patch/3768901/
> >
> >(Though it looks like it needs to be rebased due to the recent commit
> >794b1248, should Mikulas rebase and re-send?)
> 
> I applied it and fixed the conflict by hand.
> 
> Please double-check commit 694617474e33b8603fc76e090ed7d09376514b1a
> in my tree:
> 
> https://git.kernel.org/cgit/linux/kernel/git/penberg/linux.git/

Looks good, thanks.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: slab_common: fix the check for duplicate slab names
  2014-05-23 21:28       ` Pekka Enberg
  2014-05-24  0:37         ` Mike Snitzer
@ 2014-07-22 22:14         ` Mike Snitzer
  2014-07-22 22:41           ` [patch] " David Rientjes
  1 sibling, 1 reply; 10+ messages in thread
From: Mike Snitzer @ 2014-07-22 22:14 UTC (permalink / raw)
  To: Pekka Enberg
  Cc: Christoph Lameter, Pekka Enberg, linux-mm, dm-devel, linux-kernel,
	Alasdair G. Kergon, Mikulas Patocka, Zdenek Kabelac

On Fri, May 23 2014 at  5:28pm -0400,
Pekka Enberg <penberg@iki.fi> wrote:

> On 05/23/2014 11:16 PM, Mike Snitzer wrote:
> >On Tue, Mar 25 2014 at  2:07pm -0400,
> >Christoph Lameter <cl@linux.com> wrote:
> >
> >>On Tue, 25 Mar 2014, Mike Snitzer wrote:
> >>
> >>>This patch still isn't upstream.  Who should be shepherding it to Linus?
> >>Pekka usually does that.
> >>
> >>Acked-by: Christoph Lameter <cl@linux.com>
> >This still hasn't gotten upstream.
> >
> >Pekka, any chance you can pick it up?  Here it is in dm-devel's
> >kernel.org patchwork: https://patchwork.kernel.org/patch/3768901/
> >
> >(Though it looks like it needs to be rebased due to the recent commit
> >794b1248, should Mikulas rebase and re-send?)
> 
> I applied it and fixed the conflict by hand.
> 
> Please double-check commit 694617474e33b8603fc76e090ed7d09376514b1a
> in my tree:
> 
> https://git.kernel.org/cgit/linux/kernel/git/penberg/linux.git/

Pekka, this clearly still hasn't landed for 3.16.  Can you please get
it upstream ASAP?  It is a lingering issue that keeps rearing its ugly
head, latest report on Fedora rawhide:

3,2059,887335968,-;kmem_cache_sanity_check (raid5-ffff880074a56010): Cache name already exists.
4,2060,887339337,-;CPU: 1 PID: 12874 Comm: lvm Not tainted 3.16.0-0.rc4.git1.1.fc21.x86_64 #1
4,2061,887342267,-;Hardware name: Red Hat KVM, BIOS 0.5.1 01/01/2007
4,2062,887344959,-; 0000000000000000 00000000808b098a ffff880074d23a00 ffffffff81807d5c
4,2063,887347591,-; ffff88007557ab48 ffff880074d23a78 ffffffff811e9451 ffff88007557a800
4,2064,887350230,-; ffff880074d23a78 0000000000000000 0000000000000000 0000000000000690
4,2065,887352882,-;Call Trace:
4,2066,887355210,-; [<ffffffff81807d5c>] dump_stack+0x4d/0x66
4,2067,887357157,-; [<ffffffff811e9451>] kmem_cache_create+0x2c1/0x330
4,2068,887360103,-; [<ffffffffa0272527>] setup_conf+0x677/0x8c0 [raid456]
4,2069,887362901,-; [<ffffffff810e1d78>] ? sched_clock_cpu+0x98/0xc0
4,2070,887364749,-; [<ffffffffa0273350>] run+0x8c0/0xab0 [raid456]
4,2071,887366545,-; [<ffffffff8162f562>] md_run+0x562/0x980
4,2072,887368240,-; [<ffffffffa028756c>] ? raid_ctr+0xf3c/0x13f8 [dm_raid]
4,2073,887370163,-; [<ffffffff810fbc44>] ? static_obj+0x34/0x50
4,2074,887371973,-; [<ffffffff810fc4ac>] ? lockdep_init_map+0x6c/0x570
4,2075,887373889,-; [<ffffffffa0287578>] raid_ctr+0xf48/0x13f8 [dm_raid]
4,2076,887375849,-; [<ffffffff8163c5b0>] dm_table_add_target+0x160/0x3b0
4,2077,887377816,-; [<ffffffff8163fb14>] table_load+0x144/0x360
4,2078,887379645,-; [<ffffffff8163f9d0>] ? retrieve_status+0x1c0/0x1c0
4,2079,887381536,-; [<ffffffff816407db>] ctl_ioctl+0x25b/0x550
4,2080,887383318,-; [<ffffffff81640ae3>] dm_ctl_ioctl+0x13/0x20
4,2081,887385130,-; [<ffffffff812628e0>] do_vfs_ioctl+0x2f0/0x520
4,2082,887387025,-; [<ffffffff8126f2bd>] ? __fget_light+0x13d/0x160
4,2083,887388895,-; [<ffffffff81262b91>] SyS_ioctl+0x81/0xa0
4,2084,887390694,-; [<ffffffff8115fbac>] ? __audit_syscall_entry+0x9c/0xf0
4,2085,887392656,-; [<ffffffff81811969>] system_call_fastpath+0x16/0x1b
4,2086,887394585,-;kmem_cache_create(raid5-ffff880074a56010) failed with error -22
4,2087,887395592,-;CPU: 1 PID: 12874 Comm: lvm Not tainted 3.16.0-0.rc4.git1.1.fc21.x86_64 #1

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [patch] slab_common: fix the check for duplicate slab names
  2014-07-22 22:14         ` Mike Snitzer
@ 2014-07-22 22:41           ` David Rientjes
  2014-07-22 22:58             ` [git pull] stable mm/slab_common.c fix for 3.16-rc7 Mike Snitzer
  0 siblings, 1 reply; 10+ messages in thread
From: David Rientjes @ 2014-07-22 22:41 UTC (permalink / raw)
  To: Andrew Morton, Mike Snitzer
  Cc: Pekka Enberg, Christoph Lameter, Pekka Enberg, linux-mm, dm-devel,
	linux-kernel, Alasdair G. Kergon, Mikulas Patocka, Zdenek Kabelac

From: Mikulas Patocka <mpatocka@redhat.com>

The patch 3e374919b314f20e2a04f641ebc1093d758f66a4 is supposed to fix the
problem where kmem_cache_create incorrectly reports duplicate cache name
and fails. The problem is described in the header of that patch.

However, the patch doesn't really fix the problem because of these
reasons:

* the logic to test for debugging is reversed. It was intended to perform
  the check only if slub debugging is enabled (which implies that caches
  with the same parameters are not merged). Therefore, there should be
  #if !defined(CONFIG_SLUB) || defined(CONFIG_SLUB_DEBUG_ON)
  The current code has the condition reversed and performs the test if
  debugging is disabled.

* slub debugging may be enabled or disabled based on kernel command line,
  CONFIG_SLUB_DEBUG_ON is just the default settings. Therefore the test
  based on definition of CONFIG_SLUB_DEBUG_ON is unreliable.

This patch fixes the problem by removing the test
"!defined(CONFIG_SLUB_DEBUG_ON)". Therefore, duplicate names are never
checked if the SLUB allocator is used.

Note to stable kernel maintainers: when backporint this patch, please
backport also the patch 3e374919b314f20e2a04f641ebc1093d758f66a4.

Cc: stable@vger.kernel.org	# 3.6+
Acked-by: Christoph Lameter <cl@linux.com>
Signed-off-by: Mikulas Patocka <mpatocka@redhat.com>
Signed-off-by: Pekka Enberg <penberg@kernel.org>
Signed-off-by: David Rientjes <rientjes@google.com>
---
 Andrew, this is in Pekka's tree but has not managed to be pushed for 
 3.16, mind picking it up?

 I'll check to see if there's anything else in Pekka's tree that you need.

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

diff --git a/mm/slab_common.c b/mm/slab_common.c
--- a/mm/slab_common.c
+++ b/mm/slab_common.c
@@ -55,7 +55,7 @@ static int kmem_cache_sanity_check(const char *name, size_t size)
 			continue;
 		}
 
-#if !defined(CONFIG_SLUB) || !defined(CONFIG_SLUB_DEBUG_ON)
+#if !defined(CONFIG_SLUB)
 		if (!strcmp(s->name, name)) {
 			pr_err("%s (%s): Cache name already exists.\n",
 			       __func__, name);

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [git pull] stable mm/slab_common.c fix for 3.16-rc7
  2014-07-22 22:41           ` [patch] " David Rientjes
@ 2014-07-22 22:58             ` Mike Snitzer
  0 siblings, 0 replies; 10+ messages in thread
From: Mike Snitzer @ 2014-07-22 22:58 UTC (permalink / raw)
  To: Linus Torvalds
  Cc: Andrew Morton, David Rientjes, Pekka Enberg, Christoph Lameter,
	Pekka Enberg, linux-mm, dm-devel, linux-kernel,
	Alasdair G. Kergon, Mikulas Patocka, Zdenek Kabelac

Hi Linus,

Not sure you'll be OK with what I've done or not but I pulled in a
1-liner "slab/urgent" fix that Pekka staged a couple months ago.  I've
made it available through a signed tag in the linux-dm.git tree.  My
reasoning on why this is OK is it regularly impacts DM and Pekka already
Signed-off on it (as did other mm developers).  I'm pretty sure Pekka
just forgot to follow through preparing a pull request for 3.15.

If you'd rather take this direct from Pekka (or wait for Andrew to pick
up the same patch which David just sent him) that is fine by me, I just
want the issue fixed.

The following changes since commit 048e5a07f282c57815b3901d4a68a77fa131ce0a:

  dm cache metadata: do not allow the data block size to change (2014-07-15 14:07:50 -0400)

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/device-mapper/linux-dm.git tags/urgent-slab-fix

for you to fetch changes up to 45ccaf4764278f6544db412d38a1bae056ee3acc:

  Merge branch 'slab/urgent' of git://git.kernel.org/pub/scm/linux/kernel/git/penberg/linux into for-3.16-rcX (2014-07-22 18:38:27 -0400)

Please pull, thanks.
Mike

----------------------------------------------------------------

This fixes the broken duplicate slab name check in
kmem_cache_sanity_check() that has been repeatedly reported (as recently
as today against Fedora rawhide).  Pekka seemed to have it staged for a
late 3.15-rc in his 'slab/urgent' branch but never sent a pull request,
see: https://lkml.org/lkml/2014/5/23/648

----------------------------------------------------------------
Mike Snitzer (1):
      Merge branch 'slab/urgent' of git://git.kernel.org/.../penberg/linux into for-3.16-rcX

Mikulas Patocka (1):
      slab_common: fix the check for duplicate slab names

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

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2014-07-22 22:58 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-03-04 22:13 [PATCH] slab_common: fix the check for duplicate slab names Mikulas Patocka
2014-03-04 22:48 ` David Rientjes
2014-03-25 17:03 ` Mike Snitzer
2014-03-25 18:07   ` Christoph Lameter
2014-05-23 20:16     ` Mike Snitzer
2014-05-23 21:28       ` Pekka Enberg
2014-05-24  0:37         ` Mike Snitzer
2014-07-22 22:14         ` Mike Snitzer
2014-07-22 22:41           ` [patch] " David Rientjes
2014-07-22 22:58             ` [git pull] stable mm/slab_common.c fix for 3.16-rc7 Mike Snitzer

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