All of lore.kernel.org
 help / color / mirror / Atom feed
From: Hao Li <hao.li@linux.dev>
To: Pengpeng Hou <pengpeng@iscas.ac.cn>
Cc: Vlastimil Babka <vbabka@kernel.org>,
	 Andrew Morton <akpm@linux-foundation.org>,
	linux-mm@kvack.org, Harry Yoo <harry@kernel.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: Mon, 17 Aug 2026 18:30:51 +0800	[thread overview]
Message-ID: <aoLcw91-afP2Mjtb@fedora> (raw)
In-Reply-To: <20260813161244.74476-1-pengpeng@iscas.ac.cn>

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.
> 
> Store one additional alloc/free pair.  Before recording a new allocation,
> copy the current pair to the previous slots only when both records exist.
> 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
> @@ -329,7 +329,13 @@ struct track {
>  	unsigned long when;	/* When did the operation occur */
>  };
>  
> -enum track_item { TRACK_ALLOC, TRACK_FREE, TRACK_NR };
> +enum track_item {
> +	TRACK_ALLOC,
> +	TRACK_FREE,
> +	TRACK_PREV_ALLOC,
> +	TRACK_PREV_FREE,
> +	TRACK_NR,
> +};
>  
>  #ifdef SLAB_SUPPORTS_SYSFS
>  static int sysfs_slab_add(struct kmem_cache *);
> @@ -1074,12 +1080,23 @@ static void set_track_update(struct kmem_cache *s, void *object,
>  	p->when = jiffies;
>  }
>  
> -static __always_inline void set_track(struct kmem_cache *s, void *object,
> -				      enum track_item alloc, unsigned long addr, gfp_t gfp_flags)
> +static __always_inline void set_alloc_track(struct kmem_cache *s, void *object,
> +					    unsigned long addr, gfp_t gfp_flags)
>  {
>  	depot_stack_handle_t handle = set_track_prepare(gfp_flags);
> +	struct track *alloc = get_track(s, object, TRACK_ALLOC);
> +	struct track *free = get_track(s, object, TRACK_FREE);
> +	struct track *prev_alloc;
> +	struct track *prev_free;
> +
> +	if (alloc->addr && free->addr) {
> +		prev_alloc = get_track(s, object, TRACK_PREV_ALLOC);
> +		prev_free = get_track(s, object, TRACK_PREV_FREE);
> +		*prev_alloc = *alloc;
> +		*prev_free = *free;
> +	}
>  
> -	set_track_update(s, object, alloc, addr, handle);
> +	set_track_update(s, object, TRACK_ALLOC, addr, handle);
>  }
>  
>  static void init_tracking(struct kmem_cache *s, void *object)
> @@ -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:"

Would it make sense to add a check here? something like:

if ("free track" isn't the same as "prev_free track")
	print_track("Freed", get_track(s, object, TRACK_FREE), pr_time);

> +
> +	prev_alloc = get_track(s, object, TRACK_PREV_ALLOC);
> +	if (!prev_alloc->addr)
> +		return;
> +
> +	pr_err("Previous object lifetime:\n");
> +	print_track("Allocated", prev_alloc, pr_time);
> +	print_track("Freed", get_track(s, object, TRACK_PREV_FREE), pr_time);
>  }
>  
>  static void print_slab_info(const struct slab *slab)
> @@ -1366,8 +1393,8 @@ check_bytes_and_report(struct kmem_cache *s, struct slab *slab,
>   *
>   * [Metadata starts at object + s->inuse]
>   *   - A. freelist pointer (if freeptr_outside_object)
> - *   - B. alloc tracking (SLAB_STORE_USER)
> - *   - C. free tracking (SLAB_STORE_USER)
> + *   - B. current alloc/free tracking (SLAB_STORE_USER)
> + *   - C. previous alloc/free tracking (SLAB_STORE_USER)
>   *   - D. original request size (SLAB_KMALLOC && SLAB_STORE_USER)
>   *   - E. KASAN metadata (if enabled)
>   *
> @@ -2024,8 +2051,8 @@ static inline void slab_pad_check(struct kmem_cache *s, struct slab *slab) {}
>  static inline int check_object(struct kmem_cache *s, struct slab *slab,
>  			void *object, u8 val) { return 1; }
>  static inline depot_stack_handle_t set_track_prepare(gfp_t gfp_flags) { return 0; }
> -static inline void set_track(struct kmem_cache *s, void *object,
> -			     enum track_item alloc, unsigned long addr, gfp_t gfp_flags) {}
> +static inline void set_alloc_track(struct kmem_cache *s, void *object,
> +				   unsigned long addr, gfp_t gfp_flags) {}
>  static inline void add_full(struct kmem_cache *s, struct kmem_cache_node *n,
>  					struct slab *slab) {}
>  static inline void remove_full(struct kmem_cache *s, struct kmem_cache_node *n,
> @@ -4493,7 +4520,7 @@ static void *___slab_alloc(struct kmem_cache *s, gfp_t gfpflags, int node,
>  
>  success:
>  	if (kmem_cache_debug_flags(s, SLAB_STORE_USER))
> -		set_track(s, object, TRACK_ALLOC, ac->caller_addr, gfpflags);
> +		set_alloc_track(s, object, ac->caller_addr, gfpflags);
>  
>  	return object;
>  }
> -- 
> 2.50.1 (Apple Git-155)
> 

-- 
Thanks,
Hao

  reply	other threads:[~2026-08-17 10:31 UTC|newest]

Thread overview: 7+ 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-08-13 16:12 ` [PATCH v2 2/4] mm/slub: preserve one previous object lifetime Pengpeng Hou
2026-08-17 10:30   ` Hao Li [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=aoLcw91-afP2Mjtb@fedora \
    --to=hao.li@linux.dev \
    --cc=akpm@linux-foundation.org \
    --cc=cl@gentwo.org \
    --cc=corbet@lwn.net \
    --cc=david@kernel.org \
    --cc=harry@kernel.org \
    --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.