[PATCH v3 1/2] mm: slub: Print the broken data before restoring slub.

Hyesoo Yu posted 2 patches 10 months ago
There is a newer version of this series
[PATCH v3 1/2] mm: slub: Print the broken data before restoring slub.
Posted by Hyesoo Yu 10 months ago
Previously, the restore occured after printing the object in slub.
After commit 47d911b02cbe ("slab: make check_object() more consistent"),
the bytes are printed after the restore. This information about the bytes
before the restore is highly valuable for debugging purpose.
For instance, in a event of cache issue, it displays byte patterns
by breaking them down into 64-bytes units. Without this information,
we can only speculate on how it was broken. Hence the corrupted regions
should be printed prior to the restoration process. However if an object
breaks in multiple places, the same log may be output multiple times.
Therefore the slub log is reported only once to prevent redundant printing,
by sending a parameter indicating whether an error has occurred previously.

Changes in v3:
- Change the parameter type of check_bytes_and_report.

Changes in v2:
- Instead of using print_section every time on check_bytes_and_report,
just print it once for the entire slub object before the restore.

Signed-off-by: Hyesoo Yu <hyesoo.yu@samsung.com>
Change-Id: I73cf76c110eed62506643913517c957c05a29520
---
 mm/slub.c | 29 ++++++++++++++---------------
 1 file changed, 14 insertions(+), 15 deletions(-)

diff --git a/mm/slub.c b/mm/slub.c
index b3969d63cc04..de62fed12236 100644
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -1192,12 +1192,13 @@ static void restore_bytes(struct kmem_cache *s, char *message, u8 data,
 
 static pad_check_attributes int
 check_bytes_and_report(struct kmem_cache *s, struct slab *slab,
-		       u8 *object, char *what,
-		       u8 *start, unsigned int value, unsigned int bytes)
+		       u8 *object, char *what, u8 *start, unsigned int value,
+		       unsigned int bytes, bool slab_obj_print)
 {
 	u8 *fault;
 	u8 *end;
 	u8 *addr = slab_address(slab);
+	char buf[100];
 
 	metadata_access_enable();
 	fault = memchr_inv(kasan_reset_tag(start), value, bytes);
@@ -1212,11 +1213,14 @@ check_bytes_and_report(struct kmem_cache *s, struct slab *slab,
 	if (slab_add_kunit_errors())
 		goto skip_bug_print;
 
-	slab_bug(s, "%s overwritten", what);
 	pr_err("0x%p-0x%p @offset=%tu. First byte 0x%x instead of 0x%x\n",
 					fault, end - 1, fault - addr,
 					fault[0], value);
 
+	scnprintf(buf, 100, "%s overwritten", what);
+	if (slab_obj_print)
+		object_err(s, slab, object, buf);
+
 skip_bug_print:
 	restore_bytes(s, what, value, fault, end);
 	return 0;
@@ -1279,7 +1283,7 @@ static int check_pad_bytes(struct kmem_cache *s, struct slab *slab, u8 *p)
 		return 1;
 
 	return check_bytes_and_report(s, slab, p, "Object padding",
-			p + off, POISON_INUSE, size_from_object(s) - off);
+			p + off, POISON_INUSE, size_from_object(s) - off, true);
 }
 
 /* Check the pad bytes at the end of a slab page */
@@ -1329,11 +1333,11 @@ static int check_object(struct kmem_cache *s, struct slab *slab,
 
 	if (s->flags & SLAB_RED_ZONE) {
 		if (!check_bytes_and_report(s, slab, object, "Left Redzone",
-			object - s->red_left_pad, val, s->red_left_pad))
+			object - s->red_left_pad, val, s->red_left_pad, !!ret))
 			ret = 0;
 
 		if (!check_bytes_and_report(s, slab, object, "Right Redzone",
-			endobject, val, s->inuse - s->object_size))
+			endobject, val, s->inuse - s->object_size, !!ret))
 			ret = 0;
 
 		if (slub_debug_orig_size(s) && val == SLUB_RED_ACTIVE) {
@@ -1342,7 +1346,7 @@ static int check_object(struct kmem_cache *s, struct slab *slab,
 			if (s->object_size > orig_size  &&
 				!check_bytes_and_report(s, slab, object,
 					"kmalloc Redzone", p + orig_size,
-					val, s->object_size - orig_size)) {
+					val, s->object_size - orig_size, !!ret)) {
 				ret = 0;
 			}
 		}
@@ -1350,7 +1354,7 @@ static int check_object(struct kmem_cache *s, struct slab *slab,
 		if ((s->flags & SLAB_POISON) && s->object_size < s->inuse) {
 			if (!check_bytes_and_report(s, slab, p, "Alignment padding",
 				endobject, POISON_INUSE,
-				s->inuse - s->object_size))
+				s->inuse - s->object_size, !!ret))
 				ret = 0;
 		}
 	}
@@ -1366,11 +1370,11 @@ static int check_object(struct kmem_cache *s, struct slab *slab,
 			if (kasan_meta_size < s->object_size - 1 &&
 			    !check_bytes_and_report(s, slab, p, "Poison",
 					p + kasan_meta_size, POISON_FREE,
-					s->object_size - kasan_meta_size - 1))
+					s->object_size - kasan_meta_size - 1, !!ret))
 				ret = 0;
 			if (kasan_meta_size < s->object_size &&
 			    !check_bytes_and_report(s, slab, p, "End Poison",
-					p + s->object_size - 1, POISON_END, 1))
+					p + s->object_size - 1, POISON_END, 1, !!ret))
 				ret = 0;
 		}
 		/*
@@ -1396,11 +1400,6 @@ static int check_object(struct kmem_cache *s, struct slab *slab,
 		ret = 0;
 	}
 
-	if (!ret && !slab_in_kunit_test()) {
-		print_trailer(s, slab, object);
-		add_taint(TAINT_BAD_PAGE, LOCKDEP_NOW_UNRELIABLE);
-	}
-
 	return ret;
 }
 
-- 
2.28.0
Re: [PATCH v3 1/2] mm: slub: Print the broken data before restoring slub.
Posted by Christoph Lameter (Ampere) 9 months, 3 weeks ago
On Thu, 20 Feb 2025, Hyesoo Yu wrote:

> @@ -1396,11 +1400,6 @@ static int check_object(struct kmem_cache *s, struct slab *slab,
>  		ret = 0;
>  	}
>
> -	if (!ret && !slab_in_kunit_test()) {


> -		print_trailer(s, slab, object);
> -		add_taint(TAINT_BAD_PAGE, LOCKDEP_NOW_UNRELIABLE);

No tainting and printing of the trailer anymore?
Re: [PATCH v3 1/2] mm: slub: Print the broken data before restoring slub.
Posted by Hyesoo Yu 9 months, 3 weeks ago
On Mon, Feb 24, 2025 at 09:24:42AM -0800, Christoph Lameter (Ampere) wrote:
> On Thu, 20 Feb 2025, Hyesoo Yu wrote:
> 
> > @@ -1396,11 +1400,6 @@ static int check_object(struct kmem_cache *s, struct slab *slab,
> >  		ret = 0;
> >  	}
> >
> > -	if (!ret && !slab_in_kunit_test()) {
> 
> 
> > -		print_trailer(s, slab, object);
> > -		add_taint(TAINT_BAD_PAGE, LOCKDEP_NOW_UNRELIABLE);
> 
> No tainting and printing of the trailer anymore?
> 

If there is an error before on check_object, the check_bytes_and_report
calls object_err that calls print_trailer and add_traint.

Thanks,
Regards.
Re: [PATCH v3 1/2] mm: slub: Print the broken data before restoring slub.
Posted by Harry Yoo 10 months ago
On Thu, Feb 20, 2025 at 12:39:43PM +0900, Hyesoo Yu wrote:
> Previously, the restore occured after printing the object in slub.
> After commit 47d911b02cbe ("slab: make check_object() more consistent"),
> the bytes are printed after the restore. This information about the bytes
> before the restore is highly valuable for debugging purpose.
> For instance, in a event of cache issue, it displays byte patterns
> by breaking them down into 64-bytes units. Without this information,
> we can only speculate on how it was broken. Hence the corrupted regions
> should be printed prior to the restoration process. However if an object
> breaks in multiple places, the same log may be output multiple times.
> Therefore the slub log is reported only once to prevent redundant printing,
> by sending a parameter indicating whether an error has occurred previously.
> 
> Changes in v3:
> - Change the parameter type of check_bytes_and_report.
> 
> Changes in v2:
> - Instead of using print_section every time on check_bytes_and_report,
> just print it once for the entire slub object before the restore.
> 
> Signed-off-by: Hyesoo Yu <hyesoo.yu@samsung.com>
> Change-Id: I73cf76c110eed62506643913517c957c05a29520
> ---
>  mm/slub.c | 29 ++++++++++++++---------------
>  1 file changed, 14 insertions(+), 15 deletions(-)
> 

> @@ -1212,11 +1213,14 @@ check_bytes_and_report(struct kmem_cache *s, struct slab *slab,
>  	if (slab_add_kunit_errors())
>  		goto skip_bug_print;
>  
> -	slab_bug(s, "%s overwritten", what);
>  	pr_err("0x%p-0x%p @offset=%tu. First byte 0x%x instead of 0x%x\n",
>  					fault, end - 1, fault - addr,
>  					fault[0], value);
>  
> +	scnprintf(buf, 100, "%s overwritten", what);
> +	if (slab_obj_print)
> +		object_err(s, slab, object, buf);


Wait, I think it's better to keep printing "%s overwritten" regardless
of slab_obj_print and only call __slab_err() if slab_obj_print == true
as discussed here [1]? Becuase in case there are multiple errors,
users should know.

[1] https://lore.kernel.org/all/2ff52c5e-4b6b-4b3d-9047-f00967315d3e@suse.cz

-- 
Cheers,
Harry
Re: [PATCH v3 1/2] mm: slub: Print the broken data before restoring slub.
Posted by Hyesoo Yu 9 months, 3 weeks ago
On Fri, Feb 21, 2025 at 05:16:01PM +0900, Harry Yoo wrote:
> On Thu, Feb 20, 2025 at 12:39:43PM +0900, Hyesoo Yu wrote:
> > Previously, the restore occured after printing the object in slub.
> > After commit 47d911b02cbe ("slab: make check_object() more consistent"),
> > the bytes are printed after the restore. This information about the bytes
> > before the restore is highly valuable for debugging purpose.
> > For instance, in a event of cache issue, it displays byte patterns
> > by breaking them down into 64-bytes units. Without this information,
> > we can only speculate on how it was broken. Hence the corrupted regions
> > should be printed prior to the restoration process. However if an object
> > breaks in multiple places, the same log may be output multiple times.
> > Therefore the slub log is reported only once to prevent redundant printing,
> > by sending a parameter indicating whether an error has occurred previously.
> > 
> > Changes in v3:
> > - Change the parameter type of check_bytes_and_report.
> > 
> > Changes in v2:
> > - Instead of using print_section every time on check_bytes_and_report,
> > just print it once for the entire slub object before the restore.
> > 
> > Signed-off-by: Hyesoo Yu <hyesoo.yu@samsung.com>
> > Change-Id: I73cf76c110eed62506643913517c957c05a29520
> > ---
> >  mm/slub.c | 29 ++++++++++++++---------------
> >  1 file changed, 14 insertions(+), 15 deletions(-)
> > 
> 
> > @@ -1212,11 +1213,14 @@ check_bytes_and_report(struct kmem_cache *s, struct slab *slab,
> >  	if (slab_add_kunit_errors())
> >  		goto skip_bug_print;
> >  
> > -	slab_bug(s, "%s overwritten", what);
> >  	pr_err("0x%p-0x%p @offset=%tu. First byte 0x%x instead of 0x%x\n",
> >  					fault, end - 1, fault - addr,
> >  					fault[0], value);
> >  
> > +	scnprintf(buf, 100, "%s overwritten", what);
> > +	if (slab_obj_print)
> > +		object_err(s, slab, object, buf);
> 
> 
> Wait, I think it's better to keep printing "%s overwritten" regardless
> of slab_obj_print and only call __slab_err() if slab_obj_print == true
> as discussed here [1]? Becuase in case there are multiple errors,
> users should know.
> 
> [1] https://lore.kernel.org/all/2ff52c5e-4b6b-4b3d-9047-f00967315d3e@suse.cz
> 

Hi,

__slab_err() doesn't include print_trainer(). It needs object_err().

How about including the specific error name 'what' to pr_err ?
And then object_err would print "Object corrupt" at the beginning once
without buf like below.

	if (slab_obj_print)
		object_err(s, slab, object, "Object corrupt");

	pr_err("[%s] 0x%p-0x%p @offset=%tu. First byte 0x%x instead of 0x%x\n",
 	       what, fault, end - 1, fault - addr, fault[0], value);

Thanks,
Regards.
> -- 
> Cheers,
> Harry
> 
Re: [PATCH v3 1/2] mm: slub: Print the broken data before restoring slub.
Posted by Vlastimil Babka 9 months, 3 weeks ago
On 2/24/25 03:43, Hyesoo Yu wrote:
> On Fri, Feb 21, 2025 at 05:16:01PM +0900, Harry Yoo wrote:
>> On Thu, Feb 20, 2025 at 12:39:43PM +0900, Hyesoo Yu wrote:
>> > Previously, the restore occured after printing the object in slub.
>> > After commit 47d911b02cbe ("slab: make check_object() more consistent"),
>> > the bytes are printed after the restore. This information about the bytes
>> > before the restore is highly valuable for debugging purpose.
>> > For instance, in a event of cache issue, it displays byte patterns
>> > by breaking them down into 64-bytes units. Without this information,
>> > we can only speculate on how it was broken. Hence the corrupted regions
>> > should be printed prior to the restoration process. However if an object
>> > breaks in multiple places, the same log may be output multiple times.
>> > Therefore the slub log is reported only once to prevent redundant printing,
>> > by sending a parameter indicating whether an error has occurred previously.
>> > 
>> > Changes in v3:
>> > - Change the parameter type of check_bytes_and_report.
>> > 
>> > Changes in v2:
>> > - Instead of using print_section every time on check_bytes_and_report,
>> > just print it once for the entire slub object before the restore.
>> > 
>> > Signed-off-by: Hyesoo Yu <hyesoo.yu@samsung.com>
>> > Change-Id: I73cf76c110eed62506643913517c957c05a29520
>> > ---
>> >  mm/slub.c | 29 ++++++++++++++---------------
>> >  1 file changed, 14 insertions(+), 15 deletions(-)
>> > 
>> 
>> > @@ -1212,11 +1213,14 @@ check_bytes_and_report(struct kmem_cache *s, struct slab *slab,
>> >  	if (slab_add_kunit_errors())
>> >  		goto skip_bug_print;
>> >  
>> > -	slab_bug(s, "%s overwritten", what);
>> >  	pr_err("0x%p-0x%p @offset=%tu. First byte 0x%x instead of 0x%x\n",
>> >  					fault, end - 1, fault - addr,
>> >  					fault[0], value);
>> >  
>> > +	scnprintf(buf, 100, "%s overwritten", what);
>> > +	if (slab_obj_print)
>> > +		object_err(s, slab, object, buf);
>> 
>> 
>> Wait, I think it's better to keep printing "%s overwritten" regardless
>> of slab_obj_print and only call __slab_err() if slab_obj_print == true
>> as discussed here [1]? Becuase in case there are multiple errors,
>> users should know.
>> 
>> [1] https://lore.kernel.org/all/2ff52c5e-4b6b-4b3d-9047-f00967315d3e@suse.cz
>> 
> 
> Hi,
> 
> __slab_err() doesn't include print_trainer(). It needs object_err().

print_trailer() could be used directly?

> How about including the specific error name 'what' to pr_err ?
> And then object_err would print "Object corrupt" at the beginning once
> without buf like below.

Could also work.

> 	if (slab_obj_print)
> 		object_err(s, slab, object, "Object corrupt");
> 
> 	pr_err("[%s] 0x%p-0x%p @offset=%tu. First byte 0x%x instead of 0x%x\n",
>  	       what, fault, end - 1, fault - addr, fault[0], value);

Probably in opposite order so object_err doesn't panic_on_warn before the
pr_err?

> Thanks,
> Regards.
>> -- 
>> Cheers,
>> Harry
>> 
> 
>
Re: [PATCH v3 1/2] mm: slub: Print the broken data before restoring slub.
Posted by Hyesoo Yu 9 months, 3 weeks ago
On Mon, Feb 24, 2025 at 03:08:55PM +0100, Vlastimil Babka wrote:
> On 2/24/25 03:43, Hyesoo Yu wrote:
> > On Fri, Feb 21, 2025 at 05:16:01PM +0900, Harry Yoo wrote:
> >> On Thu, Feb 20, 2025 at 12:39:43PM +0900, Hyesoo Yu wrote:
> >> > Previously, the restore occured after printing the object in slub.
> >> > After commit 47d911b02cbe ("slab: make check_object() more consistent"),
> >> > the bytes are printed after the restore. This information about the bytes
> >> > before the restore is highly valuable for debugging purpose.
> >> > For instance, in a event of cache issue, it displays byte patterns
> >> > by breaking them down into 64-bytes units. Without this information,
> >> > we can only speculate on how it was broken. Hence the corrupted regions
> >> > should be printed prior to the restoration process. However if an object
> >> > breaks in multiple places, the same log may be output multiple times.
> >> > Therefore the slub log is reported only once to prevent redundant printing,
> >> > by sending a parameter indicating whether an error has occurred previously.
> >> > 
> >> > Changes in v3:
> >> > - Change the parameter type of check_bytes_and_report.
> >> > 
> >> > Changes in v2:
> >> > - Instead of using print_section every time on check_bytes_and_report,
> >> > just print it once for the entire slub object before the restore.
> >> > 
> >> > Signed-off-by: Hyesoo Yu <hyesoo.yu@samsung.com>
> >> > Change-Id: I73cf76c110eed62506643913517c957c05a29520
> >> > ---
> >> >  mm/slub.c | 29 ++++++++++++++---------------
> >> >  1 file changed, 14 insertions(+), 15 deletions(-)
> >> > 
> >> 
> >> > @@ -1212,11 +1213,14 @@ check_bytes_and_report(struct kmem_cache *s, struct slab *slab,
> >> >  	if (slab_add_kunit_errors())
> >> >  		goto skip_bug_print;
> >> >  
> >> > -	slab_bug(s, "%s overwritten", what);
> >> >  	pr_err("0x%p-0x%p @offset=%tu. First byte 0x%x instead of 0x%x\n",
> >> >  					fault, end - 1, fault - addr,
> >> >  					fault[0], value);
> >> >  
> >> > +	scnprintf(buf, 100, "%s overwritten", what);
> >> > +	if (slab_obj_print)
> >> > +		object_err(s, slab, object, buf);
> >> 
> >> 
> >> Wait, I think it's better to keep printing "%s overwritten" regardless
> >> of slab_obj_print and only call __slab_err() if slab_obj_print == true
> >> as discussed here [1]? Becuase in case there are multiple errors,
> >> users should know.
> >> 
> >> [1] https://lore.kernel.org/all/2ff52c5e-4b6b-4b3d-9047-f00967315d3e@suse.cz
> >> 
> > 
> > Hi,
> > 
> > __slab_err() doesn't include print_trainer(). It needs object_err().
> 
> print_trailer() could be used directly?
>

object_err calls print_trailer, add_taint and WARN_ON that we need to call here.
I think direct calling is just redundant.

> > How about including the specific error name 'what' to pr_err ?
> > And then object_err would print "Object corrupt" at the beginning once
> > without buf like below.
> 
> Could also work.
> 
> > 	if (slab_obj_print)
> > 		object_err(s, slab, object, "Object corrupt");
> > 
> > 	pr_err("[%s] 0x%p-0x%p @offset=%tu. First byte 0x%x instead of 0x%x\n",
> >  	       what, fault, end - 1, fault - addr, fault[0], value);
> 
> Probably in opposite order so object_err doesn't panic_on_warn before the
> pr_err?
> 

Yes, I tested and found that logs are not printed when panic_on_warn is enabled.
we first call pr_err and then call object_err.

> > Thanks,
> > Regards.
> >> -- 
> >> Cheers,
> >> Harry
> >> 
> > 
> > 
> 
> 
Re: [PATCH v3 1/2] mm: slub: Print the broken data before restoring slub.
Posted by Harry Yoo 10 months ago
On Thu, Feb 20, 2025 at 12:39:43PM +0900, Hyesoo Yu wrote:
> Previously, the restore occured after printing the object in slub.
> After commit 47d911b02cbe ("slab: make check_object() more consistent"),
> the bytes are printed after the restore. This information about the bytes
> before the restore is highly valuable for debugging purpose.
> For instance, in a event of cache issue, it displays byte patterns
> by breaking them down into 64-bytes units. Without this information,
> we can only speculate on how it was broken. Hence the corrupted regions
> should be printed prior to the restoration process. However if an object
> breaks in multiple places, the same log may be output multiple times.
> Therefore the slub log is reported only once to prevent redundant printing,
> by sending a parameter indicating whether an error has occurred previously.
> 
> Changes in v3:
> - Change the parameter type of check_bytes_and_report.
> 
> Changes in v2:
> - Instead of using print_section every time on check_bytes_and_report,
> just print it once for the entire slub object before the restore.
>
> Signed-off-by: Hyesoo Yu <hyesoo.yu@samsung.com>
> Change-Id: I73cf76c110eed62506643913517c957c05a29520

As previously mentioned by others, Change-Id is not used in Linux
kernel development.

> ---
>  mm/slub.c | 29 ++++++++++++++---------------
>  1 file changed, 14 insertions(+), 15 deletions(-)
> 
> diff --git a/mm/slub.c b/mm/slub.c
> index b3969d63cc04..de62fed12236 100644
> --- a/mm/slub.c
> +++ b/mm/slub.c
> @@ -1192,12 +1192,13 @@ static void restore_bytes(struct kmem_cache *s, char *message, u8 data,
>  
>  static pad_check_attributes int
>  check_bytes_and_report(struct kmem_cache *s, struct slab *slab,
> -		       u8 *object, char *what,
> -		       u8 *start, unsigned int value, unsigned int bytes)
> +		       u8 *object, char *what, u8 *start, unsigned int value,
> +		       unsigned int bytes, bool slab_obj_print)
>  {
>  	u8 *fault;
>  	u8 *end;
>  	u8 *addr = slab_address(slab);
> +	char buf[100];
>  
>  	metadata_access_enable();
>  	fault = memchr_inv(kasan_reset_tag(start), value, bytes);
> @@ -1212,11 +1213,14 @@ check_bytes_and_report(struct kmem_cache *s, struct slab *slab,
>  	if (slab_add_kunit_errors())
>  		goto skip_bug_print;
>  
> -	slab_bug(s, "%s overwritten", what);
>  	pr_err("0x%p-0x%p @offset=%tu. First byte 0x%x instead of 0x%x\n",
>  					fault, end - 1, fault - addr,
>  					fault[0], value);
>  
> +	scnprintf(buf, 100, "%s overwritten", what);

How about moving this into the if block and changing 100 to sizeof(buf)?

> +	if (slab_obj_print)
> +		object_err(s, slab, object, buf);
> +
>  skip_bug_print:
>  	restore_bytes(s, what, value, fault, end);
>  	return 0;
> @@ -1279,7 +1283,7 @@ static int check_pad_bytes(struct kmem_cache *s, struct slab *slab, u8 *p)
>  		return 1;
>  
>  	return check_bytes_and_report(s, slab, p, "Object padding",
> -			p + off, POISON_INUSE, size_from_object(s) - off);
> +			p + off, POISON_INUSE, size_from_object(s) - off, true);
>  }
>  
>  /* Check the pad bytes at the end of a slab page */
> @@ -1329,11 +1333,11 @@ static int check_object(struct kmem_cache *s, struct slab *slab,
>  
>  	if (s->flags & SLAB_RED_ZONE) {
>  		if (!check_bytes_and_report(s, slab, object, "Left Redzone",
> -			object - s->red_left_pad, val, s->red_left_pad))
> +			object - s->red_left_pad, val, s->red_left_pad, !!ret))

I think you don't have to add !! to ret.
Converting from int to _Bool is legal in C99 and it will work as intended. 

>  			ret = 0;

-- 
Cheers,
Harry
Re: [PATCH v3 1/2] mm: slub: Print the broken data before restoring slub.
Posted by Hyesoo Yu 10 months ago
On Thu, Feb 20, 2025 at 08:01:57PM +0900, Harry Yoo wrote:
> On Thu, Feb 20, 2025 at 12:39:43PM +0900, Hyesoo Yu wrote:
> > Previously, the restore occured after printing the object in slub.
> > After commit 47d911b02cbe ("slab: make check_object() more consistent"),
> > the bytes are printed after the restore. This information about the bytes
> > before the restore is highly valuable for debugging purpose.
> > For instance, in a event of cache issue, it displays byte patterns
> > by breaking them down into 64-bytes units. Without this information,
> > we can only speculate on how it was broken. Hence the corrupted regions
> > should be printed prior to the restoration process. However if an object
> > breaks in multiple places, the same log may be output multiple times.
> > Therefore the slub log is reported only once to prevent redundant printing,
> > by sending a parameter indicating whether an error has occurred previously.
> > 
> > Changes in v3:
> > - Change the parameter type of check_bytes_and_report.
> > 
> > Changes in v2:
> > - Instead of using print_section every time on check_bytes_and_report,
> > just print it once for the entire slub object before the restore.
> >
> > Signed-off-by: Hyesoo Yu <hyesoo.yu@samsung.com>
> > Change-Id: I73cf76c110eed62506643913517c957c05a29520
> 
> As previously mentioned by others, Change-Id is not used in Linux
> kernel development.
>

Oops, It is my mistake. I will remove it.

> > ---
> >  mm/slub.c | 29 ++++++++++++++---------------
> >  1 file changed, 14 insertions(+), 15 deletions(-)
> > 
> > diff --git a/mm/slub.c b/mm/slub.c
> > index b3969d63cc04..de62fed12236 100644
> > --- a/mm/slub.c
> > +++ b/mm/slub.c
> > @@ -1192,12 +1192,13 @@ static void restore_bytes(struct kmem_cache *s, char *message, u8 data,
> >  
> >  static pad_check_attributes int
> >  check_bytes_and_report(struct kmem_cache *s, struct slab *slab,
> > -		       u8 *object, char *what,
> > -		       u8 *start, unsigned int value, unsigned int bytes)
> > +		       u8 *object, char *what, u8 *start, unsigned int value,
> > +		       unsigned int bytes, bool slab_obj_print)
> >  {
> >  	u8 *fault;
> >  	u8 *end;
> >  	u8 *addr = slab_address(slab);
> > +	char buf[100];
> >  
> >  	metadata_access_enable();
> >  	fault = memchr_inv(kasan_reset_tag(start), value, bytes);
> > @@ -1212,11 +1213,14 @@ check_bytes_and_report(struct kmem_cache *s, struct slab *slab,
> >  	if (slab_add_kunit_errors())
> >  		goto skip_bug_print;
> >  
> > -	slab_bug(s, "%s overwritten", what);
> >  	pr_err("0x%p-0x%p @offset=%tu. First byte 0x%x instead of 0x%x\n",
> >  					fault, end - 1, fault - addr,
> >  					fault[0], value);
> >  
> > +	scnprintf(buf, 100, "%s overwritten", what);
> 
> How about moving this into the if block and changing 100 to sizeof(buf)?
>

That sounds good. I will change it.

> > +	if (slab_obj_print)
> > +		object_err(s, slab, object, buf);
> > +
> >  skip_bug_print:
> >  	restore_bytes(s, what, value, fault, end);
> >  	return 0;
> > @@ -1279,7 +1283,7 @@ static int check_pad_bytes(struct kmem_cache *s, struct slab *slab, u8 *p)
> >  		return 1;
> >  
> >  	return check_bytes_and_report(s, slab, p, "Object padding",
> > -			p + off, POISON_INUSE, size_from_object(s) - off);
> > +			p + off, POISON_INUSE, size_from_object(s) - off, true);
> >  }
> >  
> >  /* Check the pad bytes at the end of a slab page */
> > @@ -1329,11 +1333,11 @@ static int check_object(struct kmem_cache *s, struct slab *slab,
> >  
> >  	if (s->flags & SLAB_RED_ZONE) {
> >  		if (!check_bytes_and_report(s, slab, object, "Left Redzone",
> > -			object - s->red_left_pad, val, s->red_left_pad))
> > +			object - s->red_left_pad, val, s->red_left_pad, !!ret))
> 
> I think you don't have to add !! to ret.
> Converting from int to _Bool is legal in C99 and it will work as intended. 
> 

Thank you for informing me.
I remove !! to next version.

Thanks,
Regards.

> > 			ret = 0;
> 
> -- 
> Cheers,
> Harry
>