All of lore.kernel.org
 help / color / mirror / Atom feed
From: Harry Yoo <harry@kernel.org>
To: Hao Li <hao.li@linux.dev>
Cc: Pengpeng Hou <pengpeng@iscas.ac.cn>,
	 Vlastimil Babka <vbabka@kernel.org>,
	Andrew Morton <akpm@linux-foundation.org>,
	linux-mm@kvack.org,  Christoph Lameter <cl@gentwo.org>,
	David Rientjes <rientjes@google.com>,
	 Roman Gushchin <roman.gushchin@linux.dev>,
	David Hildenbrand <david@kernel.org>,
	 Lorenzo Stoakes <ljs@kernel.org>,
	"Liam R. Howlett" <liam@infradead.org>,
	 Mike Rapoport <rppt@kernel.org>,
	Suren Baghdasaryan <surenb@google.com>,
	 Michal Hocko <mhocko@suse.com>, Jonathan Corbet <corbet@lwn.net>,
	 Shuah Khan <skhan@linuxfoundation.org>,
	linux-doc@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2 2/4] mm/slub: preserve one previous object lifetime
Date: Fri, 11 Sep 2026 12:10:40 +0100	[thread overview]
Message-ID: <aqPDpORERiVkBmBf@thinkstation> (raw)
In-Reply-To: <aoLcw91-afP2Mjtb@fedora>

On Mon, Aug 17, 2026 at 06:30:51PM +0800, Hao Li wrote:
> On Fri, Aug 14, 2026 at 12:12:44AM +0800, Pengpeng Hou wrote:
> > SLAB_STORE_USER replaces the allocation track when an object is reused.  A
> > later stale free can then replace the free track as well, leaving the
> > report without the completed lifetime that created the stale reference.

Can we explicitly mention "double free" to make it more straightforward?
It confused me twice already ;-)

> > Store one additional alloc/free pair.  Before recording a new allocation,
> > copy the current pair to the previous slots only when both records exist.

Hmm we should probably instead:

1. Copy TRACK_ALLOC to TRACK_PREV_ALLOC before updating TRACK_ALLOC
2. Copy TRACK_FREE to TRACK_PREV_FREE before updating TRACK_FREE

I feel thinking of one pair as one 'lifetime' adds quite some confusion
because TRACK_ALLOC/TRACK_FREE sometimes form a 'lifetime', but
sometimes span more than one lifetime.

> > Keep the current free track intact to preserve existing SLAB_STORE_USER
> > behavior during the reuse window.
> >
> > Print the previous pair when available.  These records are diagnostic
> > history and do not infer semantic ownership.
> > 
> > Assisted-by: Codex:gpt-5
> > Signed-off-by: Pengpeng Hou <pengpeng@iscas.ac.cn>
> > ---
> >  mm/slub.c | 45 ++++++++++++++++++++++++++++++++++++---------
> >  1 file changed, 36 insertions(+), 9 deletions(-)
> > 
> > diff --git a/mm/slub.c b/mm/slub.c
> > index 0653def0fe36..355fbffb981f 100644
> > --- a/mm/slub.c
> > +++ b/mm/slub.c
> > @@ -1113,12 +1130,22 @@ static void print_track(const char *s, struct track *t, unsigned long pr_time)
> >  
> >  void print_tracking(struct kmem_cache *s, void *object)
> >  {
> > +	struct track *prev_alloc;
> >  	unsigned long pr_time = jiffies;
> > +
> >  	if (!(s->flags & SLAB_STORE_USER))
> >  		return;
> >  
> >  	print_track("Allocated", get_track(s, object, TRACK_ALLOC), pr_time);
> >  	print_track("Freed", get_track(s, object, TRACK_FREE), pr_time);
> 
> When object is in allocated state, under normal case, this "Freed" line
> duplicates with the "Freed" line under "Previous object lifetime:"

Good catch!

> Would it make sense to add a check here? something like:
> 
> if ("free track" isn't the same as "prev_free track")

Perhaps it's better to never make this condition true to avoid
confusion?

> 	print_track("Freed", get_track(s, object, TRACK_FREE), pr_time);

Pengpeng, do you plan to respin this?

-- 
Cheers,
Harry / Hyeonggon


  reply	other threads:[~2026-09-11 11:10 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-13 16:08 [PATCH v2 0/4] mm/slub: preserve previous object lifetime Pengpeng Hou
2026-08-13 16:10 ` [PATCH v2 1/4] mm/slub: use a track count for user metadata sizing Pengpeng Hou
2026-08-17 10:04   ` Hao Li
2026-09-10 12:36   ` Harry Yoo
2026-08-13 16:12 ` [PATCH v2 2/4] mm/slub: preserve one previous object lifetime Pengpeng Hou
2026-08-17 10:30   ` Hao Li
2026-09-11 11:10     ` Harry Yoo [this message]
2026-08-13 16:14 ` [PATCH v2 3/4] mm/slub: test previous lifetime tracking Pengpeng Hou
2026-08-13 16:17 ` [PATCH v2 4/4] Documentation/mm: describe SLUB " Pengpeng Hou

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=aqPDpORERiVkBmBf@thinkstation \
    --to=harry@kernel.org \
    --cc=akpm@linux-foundation.org \
    --cc=cl@gentwo.org \
    --cc=corbet@lwn.net \
    --cc=david@kernel.org \
    --cc=hao.li@linux.dev \
    --cc=liam@infradead.org \
    --cc=linux-doc@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=ljs@kernel.org \
    --cc=mhocko@suse.com \
    --cc=pengpeng@iscas.ac.cn \
    --cc=rientjes@google.com \
    --cc=roman.gushchin@linux.dev \
    --cc=rppt@kernel.org \
    --cc=skhan@linuxfoundation.org \
    --cc=surenb@google.com \
    --cc=vbabka@kernel.org \
    /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.