All of lore.kernel.org
 help / color / mirror / Atom feed
From: Harry Yoo <harry.yoo@oracle.com>
To: Hyesoo Yu <hyesoo.yu@samsung.com>
Cc: janghyuck.kim@samsung.com, vbabka@suse.cz,
	Christoph Lameter <cl@linux.com>,
	Pekka Enberg <penberg@kernel.org>,
	David Rientjes <rientjes@google.com>,
	Joonsoo Kim <iamjoonsoo.kim@lge.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	Roman Gushchin <roman.gushchin@linux.dev>,
	Hyeonggon Yoo <42.hyeyoo@gmail.com>,
	linux-mm@kvack.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v3 1/2] mm: slub: Print the broken data before restoring slub.
Date: Fri, 21 Feb 2025 17:16:01 +0900	[thread overview]
Message-ID: <Z7g2QeQp6HkyqCol@harry> (raw)
In-Reply-To: <20250220033953.1606820-2-hyesoo.yu@samsung.com>

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


  parent reply	other threads:[~2025-02-21  8:16 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <CGME20250220034151epcas2p3e23d8496e872b49da28ced7a87edd4ad@epcas2p3.samsung.com>
2025-02-20  3:39 ` [PATCH v3 0/2] mm: slub: Enhanced debugging in slub error Hyesoo Yu
2025-02-20  3:39   ` [PATCH v3 1/2] mm: slub: Print the broken data before restoring slub Hyesoo Yu
2025-02-20 11:01     ` Harry Yoo
2025-02-20 21:49       ` Hyesoo Yu
2025-02-21  8:16     ` Harry Yoo [this message]
2025-02-24  2:43       ` Hyesoo Yu
2025-02-24 14:08         ` Vlastimil Babka
2025-02-25  1:09           ` Hyesoo Yu
2025-02-24 17:24     ` Christoph Lameter (Ampere)
2025-02-25  1:11       ` Hyesoo Yu
2025-02-20  3:39   ` [PATCH v3 2/2] mm: slub: call WARN() when the slab detect an error Hyesoo Yu
2025-02-21  8:30     ` Harry Yoo
2025-02-24  2:45       ` Hyesoo Yu

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=Z7g2QeQp6HkyqCol@harry \
    --to=harry.yoo@oracle.com \
    --cc=42.hyeyoo@gmail.com \
    --cc=akpm@linux-foundation.org \
    --cc=cl@linux.com \
    --cc=hyesoo.yu@samsung.com \
    --cc=iamjoonsoo.kim@lge.com \
    --cc=janghyuck.kim@samsung.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=penberg@kernel.org \
    --cc=rientjes@google.com \
    --cc=roman.gushchin@linux.dev \
    --cc=vbabka@suse.cz \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.