Re: [PATCH v2 2/4] mm/slub: preserve one previous object lifetime
Hao Li <[email protected]>
| Newsgroups | org.kernel.vger.linux-doc,org.kernel.vger.linux-kernel,org.kvack.linux-mm |
|---|---|
| Message-ID | <aoLcw91-afP2Mjtb@fedora> |
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 <[email protected]> > --- > 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