[PATCH 1/3] Documentation: minor spelling and grammar fixes

[PATCH 1/3] Documentation: minor spelling and grammar fixes

[Chris J Arges]

Signed-off-by: Chris J Arges <chris.j.arges@canonical.com>
---
 Documentation/vm/slub.txt | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/Documentation/vm/slub.txt b/Documentation/vm/slub.txt
index b0c6d1b..e159c04 100644
--- a/Documentation/vm/slub.txt
+++ b/Documentation/vm/slub.txt
@@ -64,7 +64,7 @@ to the dentry cache with
 
 Debugging options may require the minimum possible slab order to increase as
 a result of storing the metadata (for example, caches with PAGE_SIZE object
-sizes).  This has a higher liklihood of resulting in slab allocation errors
+sizes).  This has a higher likelihood of resulting in slab allocation errors
 in low memory situations or if there's high fragmentation of memory.  To
 switch off debugging for such caches by default, use
 
@@ -95,7 +95,7 @@ slabinfo -a displays which slabs were merged together.
 Slab validation
 ---------------
 
-SLUB can validate all object if the kernel was booted with slub_debug. In
+SLUB can validate all objects if the kernel was booted with slub_debug. In
 order to do so you must have the slabinfo tool. Then you can do
 
 slabinfo -v
@@ -125,7 +125,7 @@ In general slub will be able to perform this number of allocations
 on a slab without consulting centralized resources (list_lock) where
 contention may occur.
 
-slub_min_order specifies a minim order of slabs. A similar effect like
+slub_min_order specifies a minimum order of slabs. A similar effect like
 slub_min_objects.
 
 slub_max_order specified the order at which slub_min_objects should no
@@ -133,7 +133,7 @@ longer be checked. This is useful to avoid SLUB trying to generate
 super large order pages to fit slub_min_objects of a slab cache with
 large object sizes into one high order page. Setting command line
 parameter debug_guardpage_minorder=N (N > 0), forces setting
-slub_max_order to 0, what cause minimum possible order of slabs
+slub_max_order to 0, which causes the minimum possible order of slab
 allocation.
 
 SLUB Debug output
@@ -234,7 +234,7 @@ Padding <address> : <bytes>
 3. A stackdump
 
 The stackdump describes the location where the error was detected. The cause
-of the corruption is may be more likely found by looking at the function that
+of the corruption may be more likely found by looking at the function that
 allocated or freed the object.
 
 4. Report on how the problem was dealt with in order to ensure the continued
@@ -246,7 +246,7 @@ FIX <slab cache affected>: <corrective action taken>
 
 In the above sample SLUB found that the Redzone of an active object has
 been overwritten. Here a string of 8 characters was written into a slab that
-has the length of 8 characters. However, a 8 character string needs a
+has the length of 8 characters. However, an 8 character string needs a
 terminating 0. That zero has overwritten the first byte of the Redzone field.
 After reporting the details of the issue encountered the FIX SLUB message
 tells us that SLUB has restored the Redzone to its proper value and then
-- 
1.9.1

[PATCH 2/3] mm: slub: parse slub_debug O option in switch statement

[Chris J Arges]

By moving the O option detection into the switch statement, we allow this
parameter to be combined with other options correctly. Previously options like
slub_debug=OFZ would only detect the 'o' and use DEBUG_DEFAULT_FLAGS to fill
in the rest of the flags.

Signed-off-by: Chris J Arges <chris.j.arges@canonical.com>
---
 mm/slub.c | 16 +++++++---------
 1 file changed, 7 insertions(+), 9 deletions(-)

diff --git a/mm/slub.c b/mm/slub.c
index 06cdb18..88482f8 100644
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -1112,15 +1112,6 @@ static int __init setup_slub_debug(char *str)
 		 */
 		goto check_slabs;
 
-	if (tolower(*str) == 'o') {
-		/*
-		 * Avoid enabling debugging on caches if its minimum order
-		 * would increase as a result.
-		 */
-		disable_higher_order_debug = 1;
-		goto out;
-	}
-
 	slub_debug = 0;
 	if (*str == '-')
 		/*
@@ -1151,6 +1142,13 @@ static int __init setup_slub_debug(char *str)
 		case 'a':
 			slub_debug |= SLAB_FAILSLAB;
 			break;
+		case 'o':
+			/*
+			 * Avoid enabling debugging on caches if its minimum
+			 * order would increase as a result.
+			 */
+			disable_higher_order_debug = 1;
+			break;
 		default:
 			pr_err("slub_debug option '%c' unknown. skipped\n",
 			       *str);
-- 
1.9.1

Re: [PATCH 2/3] mm: slub: parse slub_debug O option in switch statement

[David Rientjes]

On Fri, 13 Feb 2015, Chris J Arges wrote:

> By moving the O option detection into the switch statement, we allow this
> parameter to be combined with other options correctly. Previously options like
> slub_debug=OFZ would only detect the 'o' and use DEBUG_DEFAULT_FLAGS to fill
> in the rest of the flags.
> 
> Signed-off-by: Chris J Arges <chris.j.arges@canonical.com>

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

[PATCH 3/3] mm: slub: Add SLAB_DEBUG_CRASH option

[Chris J Arges]

This option crashes the kernel whenever corruption is initially detected. This
is useful when trying to use crash dump analysis to determine where memory was
corrupted.

To enable this option use slub_debug=C.

Signed-off-by: Chris J Arges <chris.j.arges@canonical.com>
---
 Documentation/vm/slub.txt |  2 ++
 include/linux/slab.h      |  1 +
 mm/slub.c                 | 10 ++++++++++
 3 files changed, 13 insertions(+)

diff --git a/Documentation/vm/slub.txt b/Documentation/vm/slub.txt
index e159c04..78fbe44 100644
--- a/Documentation/vm/slub.txt
+++ b/Documentation/vm/slub.txt
@@ -44,6 +44,8 @@ Possible debug options are
 	A		Toggle failslab filter mark for the cache
 	O		Switch debugging off for caches that would have
 			caused higher minimum slab orders
+	C		Crash kernel on corruption detection. (Useful for
+			debugging with crash dumps)
 	-		Switch all debugging off (useful if the kernel is
 			configured with CONFIG_SLUB_DEBUG_ON)
 
diff --git a/include/linux/slab.h b/include/linux/slab.h
index ed2ffaa..6c8eda9 100644
--- a/include/linux/slab.h
+++ b/include/linux/slab.h
@@ -23,6 +23,7 @@
 #define SLAB_DEBUG_FREE		0x00000100UL	/* DEBUG: Perform (expensive) checks on free */
 #define SLAB_RED_ZONE		0x00000400UL	/* DEBUG: Red zone objs in a cache */
 #define SLAB_POISON		0x00000800UL	/* DEBUG: Poison objects */
+#define SLAB_DEBUG_CRASH	0x00001000UL	/* DEBUG: Crash on any errors detected */
 #define SLAB_HWCACHE_ALIGN	0x00002000UL	/* Align objs on cache lines */
 #define SLAB_CACHE_DMA		0x00004000UL	/* Use GFP_DMA memory */
 #define SLAB_STORE_USER		0x00010000UL	/* DEBUG: Store the last owner for bug hunting */
diff --git a/mm/slub.c b/mm/slub.c
index 88482f8..1eb0031 100644
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -1025,6 +1025,9 @@ static noinline int alloc_debug_processing(struct kmem_cache *s,
 	return 1;
 
 bad:
+	/* BUG_ON to trace initial corruption */
+	BUG_ON(s->flags & SLAB_DEBUG_CRASH);
+
 	if (PageSlab(page)) {
 		/*
 		 * If this is a slab page then lets do the best we can
@@ -1092,6 +1095,10 @@ out:
 fail:
 	slab_unlock(page);
 	spin_unlock_irqrestore(&n->list_lock, *flags);
+
+	/* BUG_ON to trace initial corruption */
+	BUG_ON(s->flags & SLAB_DEBUG_CRASH);
+
 	slab_fix(s, "Object at 0x%p not freed", object);
 	return NULL;
 }
@@ -1149,6 +1156,9 @@ static int __init setup_slub_debug(char *str)
 			 */
 			disable_higher_order_debug = 1;
 			break;
+		case 'c':
+			slub_debug |= SLAB_DEBUG_CRASH;
+			break;
 		default:
 			pr_err("slub_debug option '%c' unknown. skipped\n",
 			       *str);
-- 
1.9.1

Re: [PATCH 3/3] mm: slub: Add SLAB_DEBUG_CRASH option

[David Rientjes]

On Fri, 13 Feb 2015, Chris J Arges wrote:

> This option crashes the kernel whenever corruption is initially detected. This
> is useful when trying to use crash dump analysis to determine where memory was
> corrupted.
> 
> To enable this option use slub_debug=C.
> 

Why isn't this done in other debugging functions such as 
free_debug_processing()?

Re: [PATCH 3/3] mm: slub: Add SLAB_DEBUG_CRASH option

[Chris J Arges]

On 02/13/2015 05:38 PM, David Rientjes wrote:
> On Fri, 13 Feb 2015, Chris J Arges wrote:
> 
>> This option crashes the kernel whenever corruption is initially detected. This
>> is useful when trying to use crash dump analysis to determine where memory was
>> corrupted.
>>
>> To enable this option use slub_debug=C.
>>
> 
> Why isn't this done in other debugging functions such as 
> free_debug_processing()?
> 

The diff doesn't show this clearly, but the BUG_ON was added to both
free_debug_processing and alloc_debug_processing.

--chris

Re: [PATCH 3/3] mm: slub: Add SLAB_DEBUG_CRASH option

[Christoph Lameter]

On Fri, 13 Feb 2015, Chris J Arges wrote:

> The diff doesn't show this clearly, but the BUG_ON was added to both
> free_debug_processing and alloc_debug_processing.

This is not good. There should be no BUG_ON. The problem report by the
allocator already includes a backtrace. You need to abort with the crash
dump. Maybe print a message describing what is going on before.

Crash dump and abort would make most sense in slab_err() and object_err().

[PATCH v2] mm: slub: Add SLAB_DEBUG_CRASH option

[Chris J Arges]

This option crashes the kernel whenever corruption is initially detected. This
is useful when trying to use crash dump analysis to determine where memory was
initially corrupted.

To enable this option use slub_debug=C.

[v2]
Panic in slab_err and object_err instead of BUG_ON.

Signed-off-by: Chris J Arges <chris.j.arges@canonical.com>
---
 Documentation/vm/slub.txt | 2 ++
 include/linux/slab.h      | 1 +
 mm/slub.c                 | 9 +++++++++
 3 files changed, 12 insertions(+)

diff --git a/Documentation/vm/slub.txt b/Documentation/vm/slub.txt
index e159c04..78fbe44 100644
--- a/Documentation/vm/slub.txt
+++ b/Documentation/vm/slub.txt
@@ -44,6 +44,8 @@ Possible debug options are
 	A		Toggle failslab filter mark for the cache
 	O		Switch debugging off for caches that would have
 			caused higher minimum slab orders
+	C		Crash kernel on corruption detection. (Useful for
+			debugging with crash dumps)
 	-		Switch all debugging off (useful if the kernel is
 			configured with CONFIG_SLUB_DEBUG_ON)
 
diff --git a/include/linux/slab.h b/include/linux/slab.h
index ed2ffaa..6c8eda9 100644
--- a/include/linux/slab.h
+++ b/include/linux/slab.h
@@ -23,6 +23,7 @@
 #define SLAB_DEBUG_FREE		0x00000100UL	/* DEBUG: Perform (expensive) checks on free */
 #define SLAB_RED_ZONE		0x00000400UL	/* DEBUG: Red zone objs in a cache */
 #define SLAB_POISON		0x00000800UL	/* DEBUG: Poison objects */
+#define SLAB_DEBUG_CRASH	0x00001000UL	/* DEBUG: Crash on any errors detected */
 #define SLAB_HWCACHE_ALIGN	0x00002000UL	/* Align objs on cache lines */
 #define SLAB_CACHE_DMA		0x00004000UL	/* Use GFP_DMA memory */
 #define SLAB_STORE_USER		0x00010000UL	/* DEBUG: Store the last owner for bug hunting */
diff --git a/mm/slub.c b/mm/slub.c
index 88482f8..89a8631 100644
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -634,6 +634,9 @@ static void object_err(struct kmem_cache *s, struct page *page,
 {
 	slab_bug(s, "%s", reason);
 	print_trailer(s, page, object);
+
+	if (unlikely(s->flags & SLAB_DEBUG_CRASH))
+		panic("Panic on object error\n");
 }
 
 static void slab_err(struct kmem_cache *s, struct page *page,
@@ -648,6 +651,9 @@ static void slab_err(struct kmem_cache *s, struct page *page,
 	slab_bug(s, "%s", buf);
 	print_page_info(page);
 	dump_stack();
+
+	if (unlikely(s->flags & SLAB_DEBUG_CRASH))
+		panic("Panic on slab error\n");
 }
 
 static void init_object(struct kmem_cache *s, void *object, u8 val)
@@ -1149,6 +1155,9 @@ static int __init setup_slub_debug(char *str)
 			 */
 			disable_higher_order_debug = 1;
 			break;
+		case 'c':
+			slub_debug |= SLAB_DEBUG_CRASH;
+			break;
 		default:
 			pr_err("slub_debug option '%c' unknown. skipped\n",
 			       *str);
-- 
1.9.1

Re: [PATCH 3/3] mm: slub: Add SLAB_DEBUG_CRASH option

[Christoph Lameter]

On Fri, 13 Feb 2015, David Rientjes wrote:

> Why isn't this done in other debugging functions such as
> free_debug_processing()?

I think this belongs into the functions that report the bug. They should
report the issue and at the end of the report crash.

Re: [PATCH 1/3] Documentation: minor spelling and grammar fixes

[David Rientjes]

On Fri, 13 Feb 2015, Chris J Arges wrote:

> Signed-off-by: Chris J Arges <chris.j.arges@canonical.com>

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

Re: [PATCH 1/3] Documentation: minor spelling and grammar fixes

[Randy Dunlap]

On 02/13/15 14:19, Chris J Arges wrote:
> Signed-off-by: Chris J Arges <chris.j.arges@canonical.com>
> ---
>  Documentation/vm/slub.txt | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
> 

Acked-by: Randy Dunlap <rdunlap@infradead.org>

Thanks.

-- 
~Randy

Re: [PATCH 1/3] Documentation: minor spelling and grammar fixes

[Jonathan Corbet]

On Fri, 13 Feb 2015 16:19:35 -0600
Chris J Arges <chris.j.arges@canonical.com> wrote:

>  Documentation/vm/slub.txt | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)

This one has been applied to the docs tree, thanks.

jon