[PATCH 1/1] alloc_tag: mark inaccurate allocation counters in /proc/allocinfo output

[PATCH 1/1] alloc_tag: mark inaccurate allocation counters in /proc/allocinfo output

[Suren Baghdasaryan]

While rare, memory allocation profiling can contain inaccurate counters
if slab object extension vector allocation fails. That allocation might
succeed later but prior to that, slab allocations that would have used
that object extension vector will not be accounted for. To indicate
incorrect counters, mark them with an asterisk in the /proc/allocinfo
output.
Bump up /proc/allocinfo version to reflect change in the file format.

Example output with invalid counters:
allocinfo - version: 2.0
           0        0 arch/x86/kernel/kdebugfs.c:105 func:create_setup_data_nodes
           0        0 arch/x86/kernel/alternative.c:2090 func:alternatives_smp_module_add
          0*       0* arch/x86/kernel/alternative.c:127 func:__its_alloc
           0        0 arch/x86/kernel/fpu/regset.c:160 func:xstateregs_set
           0        0 arch/x86/kernel/fpu/xstate.c:1590 func:fpstate_realloc
           0        0 arch/x86/kernel/cpu/aperfmperf.c:379 func:arch_enable_hybrid_capacity_scale
           0        0 arch/x86/kernel/cpu/amd_cache_disable.c:258 func:init_amd_l3_attrs
      49152*      48* arch/x86/kernel/cpu/mce/core.c:2709 func:mce_device_create
       32768        1 arch/x86/kernel/cpu/mce/genpool.c:132 func:mce_gen_pool_create
           0        0 arch/x86/kernel/cpu/mce/amd.c:1341 func:mce_threshold_create_device

Suggested-by: Johannes Weiner <hannes@cmpxchg.org>
Signed-off-by: Suren Baghdasaryan <surenb@google.com>
---
Patch is based on mm-new.

 include/linux/alloc_tag.h | 12 ++++++++++++
 include/linux/codetag.h   |  5 ++++-
 lib/alloc_tag.c           |  7 +++++--
 mm/slub.c                 |  2 ++
 4 files changed, 23 insertions(+), 3 deletions(-)

diff --git a/include/linux/alloc_tag.h b/include/linux/alloc_tag.h
index 9ef2633e2c08..d40ac39bfbe8 100644
--- a/include/linux/alloc_tag.h
+++ b/include/linux/alloc_tag.h
@@ -221,6 +221,16 @@ static inline void alloc_tag_sub(union codetag_ref *ref, size_t bytes)
 	ref->ct = NULL;
 }
 
+static inline void alloc_tag_set_inaccurate(struct alloc_tag *tag)
+{
+	tag->ct.flags |= CODETAG_FLAG_INACCURATE;
+}
+
+static inline bool alloc_tag_is_inaccurate(struct alloc_tag *tag)
+{
+	return !!(tag->ct.flags & CODETAG_FLAG_INACCURATE);
+}
+
 #define alloc_tag_record(p)	((p) = current->alloc_tag)
 
 #else /* CONFIG_MEM_ALLOC_PROFILING */
@@ -230,6 +240,8 @@ static inline bool mem_alloc_profiling_enabled(void) { return false; }
 static inline void alloc_tag_add(union codetag_ref *ref, struct alloc_tag *tag,
 				 size_t bytes) {}
 static inline void alloc_tag_sub(union codetag_ref *ref, size_t bytes) {}
+static inline void alloc_tag_set_inaccurate(struct alloc_tag *tag) {}
+static inline bool alloc_tag_is_inaccurate(struct alloc_tag *tag) { return false; }
 #define alloc_tag_record(p)	do {} while (0)
 
 #endif /* CONFIG_MEM_ALLOC_PROFILING */
diff --git a/include/linux/codetag.h b/include/linux/codetag.h
index 457ed8fd3214..8ea2a5f7c98a 100644
--- a/include/linux/codetag.h
+++ b/include/linux/codetag.h
@@ -16,13 +16,16 @@ struct module;
 #define CODETAG_SECTION_START_PREFIX	"__start_"
 #define CODETAG_SECTION_STOP_PREFIX	"__stop_"
 
+/* codetag flags */
+#define CODETAG_FLAG_INACCURATE	(1 << 0)
+
 /*
  * An instance of this structure is created in a special ELF section at every
  * code location being tagged.  At runtime, the special section is treated as
  * an array of these.
  */
 struct codetag {
-	unsigned int flags; /* used in later patches */
+	unsigned int flags;
 	unsigned int lineno;
 	const char *modname;
 	const char *function;
diff --git a/lib/alloc_tag.c b/lib/alloc_tag.c
index e9b33848700a..a7f15117c759 100644
--- a/lib/alloc_tag.c
+++ b/lib/alloc_tag.c
@@ -80,7 +80,7 @@ static void allocinfo_stop(struct seq_file *m, void *arg)
 static void print_allocinfo_header(struct seq_buf *buf)
 {
 	/* Output format version, so we can change it. */
-	seq_buf_printf(buf, "allocinfo - version: 1.0\n");
+	seq_buf_printf(buf, "allocinfo - version: 2.0\n");
 	seq_buf_printf(buf, "#     <size>  <calls> <tag info>\n");
 }
 
@@ -90,7 +90,10 @@ static void alloc_tag_to_text(struct seq_buf *out, struct codetag *ct)
 	struct alloc_tag_counters counter = alloc_tag_read(tag);
 	s64 bytes = counter.bytes;
 
-	seq_buf_printf(out, "%12lli %8llu ", bytes, counter.calls);
+	if (unlikely(alloc_tag_is_inaccurate(tag)))
+		seq_buf_printf(out, "%11lli* %7llu* ", bytes, counter.calls);
+	else
+		seq_buf_printf(out, "%12lli %8llu ", bytes, counter.calls);
 	codetag_to_text(out, ct);
 	seq_buf_putc(out, ' ');
 	seq_buf_putc(out, '\n');
diff --git a/mm/slub.c b/mm/slub.c
index af343ca570b5..9c04f29ee8de 100644
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -2143,6 +2143,8 @@ __alloc_tagging_slab_alloc_hook(struct kmem_cache *s, void *object, gfp_t flags)
 	 */
 	if (likely(obj_exts))
 		alloc_tag_add(&obj_exts->ref, current->alloc_tag, s->size);
+	else
+		alloc_tag_set_inaccurate(current->alloc_tag);
 }
 
 static inline void

base-commit: f4e8f46973fe0c0f579944a37e96ba9efbe00cca
-- 
2.51.0.384.g4c02a37b29-goog

Re: [PATCH 1/1] alloc_tag: mark inaccurate allocation counters in /proc/allocinfo output

[Shakeel Butt]

On Tue, Sep 09, 2025 at 04:49:42PM -0700, Suren Baghdasaryan wrote:
> While rare, memory allocation profiling can contain inaccurate counters
> if slab object extension vector allocation fails. That allocation might
> succeed later but prior to that, slab allocations that would have used
> that object extension vector will not be accounted for. To indicate
> incorrect counters, mark them with an asterisk in the /proc/allocinfo
> output.
> Bump up /proc/allocinfo version to reflect change in the file format.
> 
> Example output with invalid counters:
> allocinfo - version: 2.0
>            0        0 arch/x86/kernel/kdebugfs.c:105 func:create_setup_data_nodes
>            0        0 arch/x86/kernel/alternative.c:2090 func:alternatives_smp_module_add
>           0*       0* arch/x86/kernel/alternative.c:127 func:__its_alloc
>            0        0 arch/x86/kernel/fpu/regset.c:160 func:xstateregs_set
>            0        0 arch/x86/kernel/fpu/xstate.c:1590 func:fpstate_realloc
>            0        0 arch/x86/kernel/cpu/aperfmperf.c:379 func:arch_enable_hybrid_capacity_scale
>            0        0 arch/x86/kernel/cpu/amd_cache_disable.c:258 func:init_amd_l3_attrs
>       49152*      48* arch/x86/kernel/cpu/mce/core.c:2709 func:mce_device_create
>        32768        1 arch/x86/kernel/cpu/mce/genpool.c:132 func:mce_gen_pool_create
>            0        0 arch/x86/kernel/cpu/mce/amd.c:1341 func:mce_threshold_create_device
> 
> Suggested-by: Johannes Weiner <hannes@cmpxchg.org>
> Signed-off-by: Suren Baghdasaryan <surenb@google.com>

Acked-by: Shakeel Butt <shakeel.butt@linux.dev>

Re: [PATCH 1/1] alloc_tag: mark inaccurate allocation counters in /proc/allocinfo output

[Vlastimil Babka]

On 9/10/25 01:49, Suren Baghdasaryan wrote:
> While rare, memory allocation profiling can contain inaccurate counters
> if slab object extension vector allocation fails. That allocation might
> succeed later but prior to that, slab allocations that would have used
> that object extension vector will not be accounted for. To indicate
> incorrect counters, mark them with an asterisk in the /proc/allocinfo
> output.
> Bump up /proc/allocinfo version to reflect change in the file format.

Since it's rare, is it worth the trouble?

> Example output with invalid counters:
> allocinfo - version: 2.0
>            0        0 arch/x86/kernel/kdebugfs.c:105 func:create_setup_data_nodes
>            0        0 arch/x86/kernel/alternative.c:2090 func:alternatives_smp_module_add
>           0*       0* arch/x86/kernel/alternative.c:127 func:__its_alloc
>            0        0 arch/x86/kernel/fpu/regset.c:160 func:xstateregs_set
>            0        0 arch/x86/kernel/fpu/xstate.c:1590 func:fpstate_realloc
>            0        0 arch/x86/kernel/cpu/aperfmperf.c:379 func:arch_enable_hybrid_capacity_scale
>            0        0 arch/x86/kernel/cpu/amd_cache_disable.c:258 func:init_amd_l3_attrs
>       49152*      48* arch/x86/kernel/cpu/mce/core.c:2709 func:mce_device_create
>        32768        1 arch/x86/kernel/cpu/mce/genpool.c:132 func:mce_gen_pool_create
>            0        0 arch/x86/kernel/cpu/mce/amd.c:1341 func:mce_threshold_create_device
> 
> Suggested-by: Johannes Weiner <hannes@cmpxchg.org>

Here a link might have been helpful :)

> Signed-off-by: Suren Baghdasaryan <surenb@google.com>
> ---
> Patch is based on mm-new.
> 
>  include/linux/alloc_tag.h | 12 ++++++++++++
>  include/linux/codetag.h   |  5 ++++-
>  lib/alloc_tag.c           |  7 +++++--
>  mm/slub.c                 |  2 ++
>  4 files changed, 23 insertions(+), 3 deletions(-)
> 
> diff --git a/include/linux/alloc_tag.h b/include/linux/alloc_tag.h
> index 9ef2633e2c08..d40ac39bfbe8 100644
> --- a/include/linux/alloc_tag.h
> +++ b/include/linux/alloc_tag.h
> @@ -221,6 +221,16 @@ static inline void alloc_tag_sub(union codetag_ref *ref, size_t bytes)
>  	ref->ct = NULL;
>  }
>  
> +static inline void alloc_tag_set_inaccurate(struct alloc_tag *tag)
> +{
> +	tag->ct.flags |= CODETAG_FLAG_INACCURATE;
> +}
> +
> +static inline bool alloc_tag_is_inaccurate(struct alloc_tag *tag)
> +{
> +	return !!(tag->ct.flags & CODETAG_FLAG_INACCURATE);
> +}
> +
>  #define alloc_tag_record(p)	((p) = current->alloc_tag)
>  
>  #else /* CONFIG_MEM_ALLOC_PROFILING */
> @@ -230,6 +240,8 @@ static inline bool mem_alloc_profiling_enabled(void) { return false; }
>  static inline void alloc_tag_add(union codetag_ref *ref, struct alloc_tag *tag,
>  				 size_t bytes) {}
>  static inline void alloc_tag_sub(union codetag_ref *ref, size_t bytes) {}
> +static inline void alloc_tag_set_inaccurate(struct alloc_tag *tag) {}
> +static inline bool alloc_tag_is_inaccurate(struct alloc_tag *tag) { return false; }
>  #define alloc_tag_record(p)	do {} while (0)
>  
>  #endif /* CONFIG_MEM_ALLOC_PROFILING */
> diff --git a/include/linux/codetag.h b/include/linux/codetag.h
> index 457ed8fd3214..8ea2a5f7c98a 100644
> --- a/include/linux/codetag.h
> +++ b/include/linux/codetag.h
> @@ -16,13 +16,16 @@ struct module;
>  #define CODETAG_SECTION_START_PREFIX	"__start_"
>  #define CODETAG_SECTION_STOP_PREFIX	"__stop_"
>  
> +/* codetag flags */
> +#define CODETAG_FLAG_INACCURATE	(1 << 0)
> +
>  /*
>   * An instance of this structure is created in a special ELF section at every
>   * code location being tagged.  At runtime, the special section is treated as
>   * an array of these.
>   */
>  struct codetag {
> -	unsigned int flags; /* used in later patches */
> +	unsigned int flags;
>  	unsigned int lineno;
>  	const char *modname;
>  	const char *function;
> diff --git a/lib/alloc_tag.c b/lib/alloc_tag.c
> index e9b33848700a..a7f15117c759 100644
> --- a/lib/alloc_tag.c
> +++ b/lib/alloc_tag.c
> @@ -80,7 +80,7 @@ static void allocinfo_stop(struct seq_file *m, void *arg)
>  static void print_allocinfo_header(struct seq_buf *buf)
>  {
>  	/* Output format version, so we can change it. */
> -	seq_buf_printf(buf, "allocinfo - version: 1.0\n");
> +	seq_buf_printf(buf, "allocinfo - version: 2.0\n");
>  	seq_buf_printf(buf, "#     <size>  <calls> <tag info>\n");
>  }
>  
> @@ -90,7 +90,10 @@ static void alloc_tag_to_text(struct seq_buf *out, struct codetag *ct)
>  	struct alloc_tag_counters counter = alloc_tag_read(tag);
>  	s64 bytes = counter.bytes;
>  
> -	seq_buf_printf(out, "%12lli %8llu ", bytes, counter.calls);
> +	if (unlikely(alloc_tag_is_inaccurate(tag)))
> +		seq_buf_printf(out, "%11lli* %7llu* ", bytes, counter.calls);
> +	else
> +		seq_buf_printf(out, "%12lli %8llu ", bytes, counter.calls);
>  	codetag_to_text(out, ct);
>  	seq_buf_putc(out, ' ');
>  	seq_buf_putc(out, '\n');
> diff --git a/mm/slub.c b/mm/slub.c
> index af343ca570b5..9c04f29ee8de 100644
> --- a/mm/slub.c
> +++ b/mm/slub.c
> @@ -2143,6 +2143,8 @@ __alloc_tagging_slab_alloc_hook(struct kmem_cache *s, void *object, gfp_t flags)
>  	 */
>  	if (likely(obj_exts))
>  		alloc_tag_add(&obj_exts->ref, current->alloc_tag, s->size);
> +	else
> +		alloc_tag_set_inaccurate(current->alloc_tag);
>  }
>  
>  static inline void
> 
> base-commit: f4e8f46973fe0c0f579944a37e96ba9efbe00cca

Re: [PATCH 1/1] alloc_tag: mark inaccurate allocation counters in /proc/allocinfo output

[Suren Baghdasaryan]

On Tue, Sep 9, 2025 at 11:25 PM Vlastimil Babka <vbabka@suse.cz> wrote:
>
> On 9/10/25 01:49, Suren Baghdasaryan wrote:
> > While rare, memory allocation profiling can contain inaccurate counters
> > if slab object extension vector allocation fails. That allocation might
> > succeed later but prior to that, slab allocations that would have used
> > that object extension vector will not be accounted for. To indicate
> > incorrect counters, mark them with an asterisk in the /proc/allocinfo
> > output.
> > Bump up /proc/allocinfo version to reflect change in the file format.
>
> Since it's rare, is it worth the trouble?

Apparently they are seen in Meta's fleet which instigated this thread:
https://lore.kernel.org/all/17fab2d6-5a74-4573-bcc3-b75951508f0a@gmail.com/

>
> > Example output with invalid counters:
> > allocinfo - version: 2.0
> >            0        0 arch/x86/kernel/kdebugfs.c:105 func:create_setup_data_nodes
> >            0        0 arch/x86/kernel/alternative.c:2090 func:alternatives_smp_module_add
> >           0*       0* arch/x86/kernel/alternative.c:127 func:__its_alloc
> >            0        0 arch/x86/kernel/fpu/regset.c:160 func:xstateregs_set
> >            0        0 arch/x86/kernel/fpu/xstate.c:1590 func:fpstate_realloc
> >            0        0 arch/x86/kernel/cpu/aperfmperf.c:379 func:arch_enable_hybrid_capacity_scale
> >            0        0 arch/x86/kernel/cpu/amd_cache_disable.c:258 func:init_amd_l3_attrs
> >       49152*      48* arch/x86/kernel/cpu/mce/core.c:2709 func:mce_device_create
> >        32768        1 arch/x86/kernel/cpu/mce/genpool.c:132 func:mce_gen_pool_create
> >            0        0 arch/x86/kernel/cpu/mce/amd.c:1341 func:mce_threshold_create_device
> >
> > Suggested-by: Johannes Weiner <hannes@cmpxchg.org>
>
> Here a link might have been helpful :)

Right, here it is and suggestion is in the last paragraph:
https://lore.kernel.org/all/20250519160846.GA773385@cmpxchg.org/

>
> > Signed-off-by: Suren Baghdasaryan <surenb@google.com>
> > ---
> > Patch is based on mm-new.
> >
> >  include/linux/alloc_tag.h | 12 ++++++++++++
> >  include/linux/codetag.h   |  5 ++++-
> >  lib/alloc_tag.c           |  7 +++++--
> >  mm/slub.c                 |  2 ++
> >  4 files changed, 23 insertions(+), 3 deletions(-)
> >
> > diff --git a/include/linux/alloc_tag.h b/include/linux/alloc_tag.h
> > index 9ef2633e2c08..d40ac39bfbe8 100644
> > --- a/include/linux/alloc_tag.h
> > +++ b/include/linux/alloc_tag.h
> > @@ -221,6 +221,16 @@ static inline void alloc_tag_sub(union codetag_ref *ref, size_t bytes)
> >       ref->ct = NULL;
> >  }
> >
> > +static inline void alloc_tag_set_inaccurate(struct alloc_tag *tag)
> > +{
> > +     tag->ct.flags |= CODETAG_FLAG_INACCURATE;
> > +}
> > +
> > +static inline bool alloc_tag_is_inaccurate(struct alloc_tag *tag)
> > +{
> > +     return !!(tag->ct.flags & CODETAG_FLAG_INACCURATE);
> > +}
> > +
> >  #define alloc_tag_record(p)  ((p) = current->alloc_tag)
> >
> >  #else /* CONFIG_MEM_ALLOC_PROFILING */
> > @@ -230,6 +240,8 @@ static inline bool mem_alloc_profiling_enabled(void) { return false; }
> >  static inline void alloc_tag_add(union codetag_ref *ref, struct alloc_tag *tag,
> >                                size_t bytes) {}
> >  static inline void alloc_tag_sub(union codetag_ref *ref, size_t bytes) {}
> > +static inline void alloc_tag_set_inaccurate(struct alloc_tag *tag) {}
> > +static inline bool alloc_tag_is_inaccurate(struct alloc_tag *tag) { return false; }
> >  #define alloc_tag_record(p)  do {} while (0)
> >
> >  #endif /* CONFIG_MEM_ALLOC_PROFILING */
> > diff --git a/include/linux/codetag.h b/include/linux/codetag.h
> > index 457ed8fd3214..8ea2a5f7c98a 100644
> > --- a/include/linux/codetag.h
> > +++ b/include/linux/codetag.h
> > @@ -16,13 +16,16 @@ struct module;
> >  #define CODETAG_SECTION_START_PREFIX "__start_"
> >  #define CODETAG_SECTION_STOP_PREFIX  "__stop_"
> >
> > +/* codetag flags */
> > +#define CODETAG_FLAG_INACCURATE      (1 << 0)
> > +
> >  /*
> >   * An instance of this structure is created in a special ELF section at every
> >   * code location being tagged.  At runtime, the special section is treated as
> >   * an array of these.
> >   */
> >  struct codetag {
> > -     unsigned int flags; /* used in later patches */
> > +     unsigned int flags;
> >       unsigned int lineno;
> >       const char *modname;
> >       const char *function;
> > diff --git a/lib/alloc_tag.c b/lib/alloc_tag.c
> > index e9b33848700a..a7f15117c759 100644
> > --- a/lib/alloc_tag.c
> > +++ b/lib/alloc_tag.c
> > @@ -80,7 +80,7 @@ static void allocinfo_stop(struct seq_file *m, void *arg)
> >  static void print_allocinfo_header(struct seq_buf *buf)
> >  {
> >       /* Output format version, so we can change it. */
> > -     seq_buf_printf(buf, "allocinfo - version: 1.0\n");
> > +     seq_buf_printf(buf, "allocinfo - version: 2.0\n");
> >       seq_buf_printf(buf, "#     <size>  <calls> <tag info>\n");
> >  }
> >
> > @@ -90,7 +90,10 @@ static void alloc_tag_to_text(struct seq_buf *out, struct codetag *ct)
> >       struct alloc_tag_counters counter = alloc_tag_read(tag);
> >       s64 bytes = counter.bytes;
> >
> > -     seq_buf_printf(out, "%12lli %8llu ", bytes, counter.calls);
> > +     if (unlikely(alloc_tag_is_inaccurate(tag)))
> > +             seq_buf_printf(out, "%11lli* %7llu* ", bytes, counter.calls);
> > +     else
> > +             seq_buf_printf(out, "%12lli %8llu ", bytes, counter.calls);
> >       codetag_to_text(out, ct);
> >       seq_buf_putc(out, ' ');
> >       seq_buf_putc(out, '\n');
> > diff --git a/mm/slub.c b/mm/slub.c
> > index af343ca570b5..9c04f29ee8de 100644
> > --- a/mm/slub.c
> > +++ b/mm/slub.c
> > @@ -2143,6 +2143,8 @@ __alloc_tagging_slab_alloc_hook(struct kmem_cache *s, void *object, gfp_t flags)
> >        */
> >       if (likely(obj_exts))
> >               alloc_tag_add(&obj_exts->ref, current->alloc_tag, s->size);
> > +     else
> > +             alloc_tag_set_inaccurate(current->alloc_tag);
> >  }
> >
> >  static inline void
> >
> > base-commit: f4e8f46973fe0c0f579944a37e96ba9efbe00cca
>

Re: [PATCH 1/1] alloc_tag: mark inaccurate allocation counters in /proc/allocinfo output

[Usama Arif]

On 10/09/2025 15:50, Suren Baghdasaryan wrote:
> On Tue, Sep 9, 2025 at 11:25 PM Vlastimil Babka <vbabka@suse.cz> wrote:
>>
>> On 9/10/25 01:49, Suren Baghdasaryan wrote:
>>> While rare, memory allocation profiling can contain inaccurate counters
>>> if slab object extension vector allocation fails. That allocation might
>>> succeed later but prior to that, slab allocations that would have used
>>> that object extension vector will not be accounted for. To indicate
>>> incorrect counters, mark them with an asterisk in the /proc/allocinfo
>>> output.
>>> Bump up /proc/allocinfo version to reflect change in the file format.
>>
>> Since it's rare, is it worth the trouble?
> 
> Apparently they are seen in Meta's fleet which instigated this thread:
> https://lore.kernel.org/all/17fab2d6-5a74-4573-bcc3-b75951508f0a@gmail.com/
> 

Yes it happens on memory bound services!

>>
>>> Example output with invalid counters:
>>> allocinfo - version: 2.0
>>>            0        0 arch/x86/kernel/kdebugfs.c:105 func:create_setup_data_nodes
>>>            0        0 arch/x86/kernel/alternative.c:2090 func:alternatives_smp_module_add
>>>           0*       0* arch/x86/kernel/alternative.c:127 func:__its_alloc
>>>            0        0 arch/x86/kernel/fpu/regset.c:160 func:xstateregs_set
>>>            0        0 arch/x86/kernel/fpu/xstate.c:1590 func:fpstate_realloc
>>>            0        0 arch/x86/kernel/cpu/aperfmperf.c:379 func:arch_enable_hybrid_capacity_scale
>>>            0        0 arch/x86/kernel/cpu/amd_cache_disable.c:258 func:init_amd_l3_attrs
>>>       49152*      48* arch/x86/kernel/cpu/mce/core.c:2709 func:mce_device_create
>>>        32768        1 arch/x86/kernel/cpu/mce/genpool.c:132 func:mce_gen_pool_create
>>>            0        0 arch/x86/kernel/cpu/mce/amd.c:1341 func:mce_threshold_create_device
>>>
>>> Suggested-by: Johannes Weiner <hannes@cmpxchg.org>
>>
>> Here a link might have been helpful :)
> 
> Right, here it is and suggestion is in the last paragraph:
> https://lore.kernel.org/all/20250519160846.GA773385@cmpxchg.org/
> 
>>
>>> Signed-off-by: Suren Baghdasaryan <surenb@google.com>
>>> ---

Acked-by: Usama Arif <usamaarif642@gmail.com>

>>> Patch is based on mm-new.
>>>
>>>  include/linux/alloc_tag.h | 12 ++++++++++++
>>>  include/linux/codetag.h   |  5 ++++-
>>>  lib/alloc_tag.c           |  7 +++++--
>>>  mm/slub.c                 |  2 ++
>>>  4 files changed, 23 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/include/linux/alloc_tag.h b/include/linux/alloc_tag.h
>>> index 9ef2633e2c08..d40ac39bfbe8 100644
>>> --- a/include/linux/alloc_tag.h
>>> +++ b/include/linux/alloc_tag.h
>>> @@ -221,6 +221,16 @@ static inline void alloc_tag_sub(union codetag_ref *ref, size_t bytes)
>>>       ref->ct = NULL;
>>>  }
>>>
>>> +static inline void alloc_tag_set_inaccurate(struct alloc_tag *tag)
>>> +{
>>> +     tag->ct.flags |= CODETAG_FLAG_INACCURATE;
>>> +}
>>> +
>>> +static inline bool alloc_tag_is_inaccurate(struct alloc_tag *tag)
>>> +{
>>> +     return !!(tag->ct.flags & CODETAG_FLAG_INACCURATE);
>>> +}
>>> +
>>>  #define alloc_tag_record(p)  ((p) = current->alloc_tag)
>>>
>>>  #else /* CONFIG_MEM_ALLOC_PROFILING */
>>> @@ -230,6 +240,8 @@ static inline bool mem_alloc_profiling_enabled(void) { return false; }
>>>  static inline void alloc_tag_add(union codetag_ref *ref, struct alloc_tag *tag,
>>>                                size_t bytes) {}
>>>  static inline void alloc_tag_sub(union codetag_ref *ref, size_t bytes) {}
>>> +static inline void alloc_tag_set_inaccurate(struct alloc_tag *tag) {}
>>> +static inline bool alloc_tag_is_inaccurate(struct alloc_tag *tag) { return false; }
>>>  #define alloc_tag_record(p)  do {} while (0)
>>>
>>>  #endif /* CONFIG_MEM_ALLOC_PROFILING */
>>> diff --git a/include/linux/codetag.h b/include/linux/codetag.h
>>> index 457ed8fd3214..8ea2a5f7c98a 100644
>>> --- a/include/linux/codetag.h
>>> +++ b/include/linux/codetag.h
>>> @@ -16,13 +16,16 @@ struct module;
>>>  #define CODETAG_SECTION_START_PREFIX "__start_"
>>>  #define CODETAG_SECTION_STOP_PREFIX  "__stop_"
>>>
>>> +/* codetag flags */
>>> +#define CODETAG_FLAG_INACCURATE      (1 << 0)
>>> +
>>>  /*
>>>   * An instance of this structure is created in a special ELF section at every
>>>   * code location being tagged.  At runtime, the special section is treated as
>>>   * an array of these.
>>>   */
>>>  struct codetag {
>>> -     unsigned int flags; /* used in later patches */
>>> +     unsigned int flags;
>>>       unsigned int lineno;
>>>       const char *modname;
>>>       const char *function;
>>> diff --git a/lib/alloc_tag.c b/lib/alloc_tag.c
>>> index e9b33848700a..a7f15117c759 100644
>>> --- a/lib/alloc_tag.c
>>> +++ b/lib/alloc_tag.c
>>> @@ -80,7 +80,7 @@ static void allocinfo_stop(struct seq_file *m, void *arg)
>>>  static void print_allocinfo_header(struct seq_buf *buf)
>>>  {
>>>       /* Output format version, so we can change it. */
>>> -     seq_buf_printf(buf, "allocinfo - version: 1.0\n");
>>> +     seq_buf_printf(buf, "allocinfo - version: 2.0\n");
>>>       seq_buf_printf(buf, "#     <size>  <calls> <tag info>\n");
>>>  }
>>>
>>> @@ -90,7 +90,10 @@ static void alloc_tag_to_text(struct seq_buf *out, struct codetag *ct)
>>>       struct alloc_tag_counters counter = alloc_tag_read(tag);
>>>       s64 bytes = counter.bytes;
>>>
>>> -     seq_buf_printf(out, "%12lli %8llu ", bytes, counter.calls);
>>> +     if (unlikely(alloc_tag_is_inaccurate(tag)))
>>> +             seq_buf_printf(out, "%11lli* %7llu* ", bytes, counter.calls);
>>> +     else
>>> +             seq_buf_printf(out, "%12lli %8llu ", bytes, counter.calls);
>>>       codetag_to_text(out, ct);
>>>       seq_buf_putc(out, ' ');
>>>       seq_buf_putc(out, '\n');
>>> diff --git a/mm/slub.c b/mm/slub.c
>>> index af343ca570b5..9c04f29ee8de 100644
>>> --- a/mm/slub.c
>>> +++ b/mm/slub.c
>>> @@ -2143,6 +2143,8 @@ __alloc_tagging_slab_alloc_hook(struct kmem_cache *s, void *object, gfp_t flags)
>>>        */
>>>       if (likely(obj_exts))
>>>               alloc_tag_add(&obj_exts->ref, current->alloc_tag, s->size);
>>> +     else
>>> +             alloc_tag_set_inaccurate(current->alloc_tag);
>>>  }
>>>
>>>  static inline void
>>>
>>> base-commit: f4e8f46973fe0c0f579944a37e96ba9efbe00cca
>>

Re: [PATCH 1/1] alloc_tag: mark inaccurate allocation counters in /proc/allocinfo output

[Johannes Weiner]

On Tue, Sep 09, 2025 at 04:49:42PM -0700, Suren Baghdasaryan wrote:
> While rare, memory allocation profiling can contain inaccurate counters
> if slab object extension vector allocation fails. That allocation might
> succeed later but prior to that, slab allocations that would have used
> that object extension vector will not be accounted for. To indicate
> incorrect counters, mark them with an asterisk in the /proc/allocinfo
> output.
> Bump up /proc/allocinfo version to reflect change in the file format.
> 
> Example output with invalid counters:
> allocinfo - version: 2.0
>            0        0 arch/x86/kernel/kdebugfs.c:105 func:create_setup_data_nodes
>            0        0 arch/x86/kernel/alternative.c:2090 func:alternatives_smp_module_add
>           0*       0* arch/x86/kernel/alternative.c:127 func:__its_alloc
>            0        0 arch/x86/kernel/fpu/regset.c:160 func:xstateregs_set
>            0        0 arch/x86/kernel/fpu/xstate.c:1590 func:fpstate_realloc
>            0        0 arch/x86/kernel/cpu/aperfmperf.c:379 func:arch_enable_hybrid_capacity_scale
>            0        0 arch/x86/kernel/cpu/amd_cache_disable.c:258 func:init_amd_l3_attrs
>       49152*      48* arch/x86/kernel/cpu/mce/core.c:2709 func:mce_device_create
>        32768        1 arch/x86/kernel/cpu/mce/genpool.c:132 func:mce_gen_pool_create
>            0        0 arch/x86/kernel/cpu/mce/amd.c:1341 func:mce_threshold_create_device
> 
> Suggested-by: Johannes Weiner <hannes@cmpxchg.org>
> Signed-off-by: Suren Baghdasaryan <surenb@google.com>

Acked-by: Johannes Weiner <hannes@cmpxchg.org>

Thanks Suren!

Re:[PATCH 1/1] alloc_tag: mark inaccurate allocation counters in /proc/allocinfo output

[David Wang]

At 2025-09-10 07:49:42, "Suren Baghdasaryan" <surenb@google.com> wrote:
>While rare, memory allocation profiling can contain inaccurate counters
>if slab object extension vector allocation fails. That allocation might
>succeed later but prior to that, slab allocations that would have used
>that object extension vector will not be accounted for. To indicate
>incorrect counters, mark them with an asterisk in the /proc/allocinfo
>output.
>Bump up /proc/allocinfo version to reflect change in the file format.
>
>Example output with invalid counters:
>allocinfo - version: 2.0
>           0        0 arch/x86/kernel/kdebugfs.c:105 func:create_setup_data_nodes
>           0        0 arch/x86/kernel/alternative.c:2090 func:alternatives_smp_module_add
>          0*       0* arch/x86/kernel/alternative.c:127 func:__its_alloc
>           0        0 arch/x86/kernel/fpu/regset.c:160 func:xstateregs_set
>           0        0 arch/x86/kernel/fpu/xstate.c:1590 func:fpstate_realloc
>           0        0 arch/x86/kernel/cpu/aperfmperf.c:379 func:arch_enable_hybrid_capacity_scale
>           0        0 arch/x86/kernel/cpu/amd_cache_disable.c:258 func:init_amd_l3_attrs
>      49152*      48* arch/x86/kernel/cpu/mce/core.c:2709 func:mce_device_create
>       32768        1 arch/x86/kernel/cpu/mce/genpool.c:132 func:mce_gen_pool_create
>           0        0 arch/x86/kernel/cpu/mce/amd.c:1341 func:mce_threshold_create_device
>

Hi, 
The changes may  break some client tools, mine included.... 
I don't mind adjusting my tools, but still
Is it acceptable  to change 
      49152*      48* arch/x86/kernel/cpu/mce/core.c:2709 func:mce_device_create
to
      +49152      +48 arch/x86/kernel/cpu/mce/core.c:2709 func:mce_device_create*

The '+' sign make it still standout when view from a terminal, and client tools, not all of them though, might not need any changes. 
And when client want to filter out inaccurate data items, it could be done by checking the tailing '*" of func name.

(There would be some corner cases, for example, the '+' sign may not needed when the value reach a negative value if some underflow bug happened)


Thanks
David.


>Suggested-by: Johannes Weiner <hannes@cmpxchg.org>
>Signed-off-by: Suren Baghdasaryan <surenb@google.com>
>---

Re: [PATCH 1/1] alloc_tag: mark inaccurate allocation counters in /proc/allocinfo output

[Yueyang Pan]

On Thu, Sep 11, 2025 at 11:03:50PM +0800, David Wang wrote:
> 
> At 2025-09-10 07:49:42, "Suren Baghdasaryan" <surenb@google.com> wrote:
> >While rare, memory allocation profiling can contain inaccurate counters
> >if slab object extension vector allocation fails. That allocation might
> >succeed later but prior to that, slab allocations that would have used
> >that object extension vector will not be accounted for. To indicate
> >incorrect counters, mark them with an asterisk in the /proc/allocinfo
> >output.
> >Bump up /proc/allocinfo version to reflect change in the file format.
> >
> >Example output with invalid counters:
> >allocinfo - version: 2.0
> >           0        0 arch/x86/kernel/kdebugfs.c:105 func:create_setup_data_nodes
> >           0        0 arch/x86/kernel/alternative.c:2090 func:alternatives_smp_module_add
> >          0*       0* arch/x86/kernel/alternative.c:127 func:__its_alloc
> >           0        0 arch/x86/kernel/fpu/regset.c:160 func:xstateregs_set
> >           0        0 arch/x86/kernel/fpu/xstate.c:1590 func:fpstate_realloc
> >           0        0 arch/x86/kernel/cpu/aperfmperf.c:379 func:arch_enable_hybrid_capacity_scale
> >           0        0 arch/x86/kernel/cpu/amd_cache_disable.c:258 func:init_amd_l3_attrs
> >      49152*      48* arch/x86/kernel/cpu/mce/core.c:2709 func:mce_device_create
> >       32768        1 arch/x86/kernel/cpu/mce/genpool.c:132 func:mce_gen_pool_create
> >           0        0 arch/x86/kernel/cpu/mce/amd.c:1341 func:mce_threshold_create_device
> >
> 
> Hi, 
> The changes may  break some client tools, mine included.... 
> I don't mind adjusting my tools, but still
> Is it acceptable  to change 
>       49152*      48* arch/x86/kernel/cpu/mce/core.c:2709 func:mce_device_create
> to
>       +49152      +48 arch/x86/kernel/cpu/mce/core.c:2709 func:mce_device_create*
> 
> The '+' sign make it still standout when view from a terminal, and client tools, not all of them though, might not need any changes. 
> And when client want to filter out inaccurate data items, it could be done by checking the tailing '*" of func name.

I agree with David on this point. We already have monitoring tool built on top 
of this output across meta fleet. Ideally we would like to keep the format of 
of size and calls the same, even for future version, because adding a * will 
change the format from int to str, which leads to change over the regex parser 
many places.

I think simply adding * to the end of function name or filename is sufficient 
as they are already str.

> 
> (There would be some corner cases, for example, the '+' sign may not needed when the value reach a negative value if some underflow bug happened)
> 
> 
> Thanks
> David.
> 
> 
> >Suggested-by: Johannes Weiner <hannes@cmpxchg.org>
> >Signed-off-by: Suren Baghdasaryan <surenb@google.com>
> >---
> 

Thanks
Pan

Re: [PATCH 1/1] alloc_tag: mark inaccurate allocation counters in /proc/allocinfo output

[Usama Arif]

On 11/09/2025 16:47, Yueyang Pan wrote:
> On Thu, Sep 11, 2025 at 11:03:50PM +0800, David Wang wrote:
>>
>> At 2025-09-10 07:49:42, "Suren Baghdasaryan" <surenb@google.com> wrote:
>>> While rare, memory allocation profiling can contain inaccurate counters
>>> if slab object extension vector allocation fails. That allocation might
>>> succeed later but prior to that, slab allocations that would have used
>>> that object extension vector will not be accounted for. To indicate
>>> incorrect counters, mark them with an asterisk in the /proc/allocinfo
>>> output.
>>> Bump up /proc/allocinfo version to reflect change in the file format.
>>>
>>> Example output with invalid counters:
>>> allocinfo - version: 2.0
>>>           0        0 arch/x86/kernel/kdebugfs.c:105 func:create_setup_data_nodes
>>>           0        0 arch/x86/kernel/alternative.c:2090 func:alternatives_smp_module_add
>>>          0*       0* arch/x86/kernel/alternative.c:127 func:__its_alloc
>>>           0        0 arch/x86/kernel/fpu/regset.c:160 func:xstateregs_set
>>>           0        0 arch/x86/kernel/fpu/xstate.c:1590 func:fpstate_realloc
>>>           0        0 arch/x86/kernel/cpu/aperfmperf.c:379 func:arch_enable_hybrid_capacity_scale
>>>           0        0 arch/x86/kernel/cpu/amd_cache_disable.c:258 func:init_amd_l3_attrs
>>>      49152*      48* arch/x86/kernel/cpu/mce/core.c:2709 func:mce_device_create
>>>       32768        1 arch/x86/kernel/cpu/mce/genpool.c:132 func:mce_gen_pool_create
>>>           0        0 arch/x86/kernel/cpu/mce/amd.c:1341 func:mce_threshold_create_device
>>>
>>
>> Hi, 
>> The changes may  break some client tools, mine included.... 
>> I don't mind adjusting my tools, but still
>> Is it acceptable  to change 
>>       49152*      48* arch/x86/kernel/cpu/mce/core.c:2709 func:mce_device_create
>> to
>>       +49152      +48 arch/x86/kernel/cpu/mce/core.c:2709 func:mce_device_create*
>>
>> The '+' sign make it still standout when view from a terminal, and client tools, not all of them though, might not need any changes. 
>> And when client want to filter out inaccurate data items, it could be done by checking the tailing '*" of func name.
> 
> I agree with David on this point. We already have monitoring tool built on top 
> of this output across meta fleet. Ideally we would like to keep the format of 
> of size and calls the same, even for future version, because adding a * will 
> change the format from int to str, which leads to change over the regex parser 
> many places.
> 
> I think simply adding * to the end of function name or filename is sufficient 
> as they are already str.
> 

Instead of:

49152*      48* arch/x86/kernel/cpu/mce/core.c:2709 func:mce_device_create

Could we do something like:

49152      48 arch/x86/kernel/cpu/mce/core.c:2709 func:mce_device_create(inaccurate)

This should hopefully not require any changes to the tools that are consuming this file.
I think it might be better to use "(inaccurate)" (without any space after function name) or
some other text instead of "+" or "*" to prevent breaking such tools. I dont think we need
to even increment allocinfo version number as well then?

>>
>> (There would be some corner cases, for example, the '+' sign may not needed when the value reach a negative value if some underflow bug happened)
>>
>>
>> Thanks
>> David.
>>
>>
>>> Suggested-by: Johannes Weiner <hannes@cmpxchg.org>
>>> Signed-off-by: Suren Baghdasaryan <surenb@google.com>
>>> ---
>>
> 
> Thanks
> Pan

Re: [PATCH 1/1] alloc_tag: mark inaccurate allocation counters in /proc/allocinfo output

[Suren Baghdasaryan]

On Thu, Sep 11, 2025 at 9:00 AM Usama Arif <usamaarif642@gmail.com> wrote:
>
>
>
> On 11/09/2025 16:47, Yueyang Pan wrote:
> > On Thu, Sep 11, 2025 at 11:03:50PM +0800, David Wang wrote:
> >>
> >> At 2025-09-10 07:49:42, "Suren Baghdasaryan" <surenb@google.com> wrote:
> >>> While rare, memory allocation profiling can contain inaccurate counters
> >>> if slab object extension vector allocation fails. That allocation might
> >>> succeed later but prior to that, slab allocations that would have used
> >>> that object extension vector will not be accounted for. To indicate
> >>> incorrect counters, mark them with an asterisk in the /proc/allocinfo
> >>> output.
> >>> Bump up /proc/allocinfo version to reflect change in the file format.
> >>>
> >>> Example output with invalid counters:
> >>> allocinfo - version: 2.0
> >>>           0        0 arch/x86/kernel/kdebugfs.c:105 func:create_setup_data_nodes
> >>>           0        0 arch/x86/kernel/alternative.c:2090 func:alternatives_smp_module_add
> >>>          0*       0* arch/x86/kernel/alternative.c:127 func:__its_alloc
> >>>           0        0 arch/x86/kernel/fpu/regset.c:160 func:xstateregs_set
> >>>           0        0 arch/x86/kernel/fpu/xstate.c:1590 func:fpstate_realloc
> >>>           0        0 arch/x86/kernel/cpu/aperfmperf.c:379 func:arch_enable_hybrid_capacity_scale
> >>>           0        0 arch/x86/kernel/cpu/amd_cache_disable.c:258 func:init_amd_l3_attrs
> >>>      49152*      48* arch/x86/kernel/cpu/mce/core.c:2709 func:mce_device_create
> >>>       32768        1 arch/x86/kernel/cpu/mce/genpool.c:132 func:mce_gen_pool_create
> >>>           0        0 arch/x86/kernel/cpu/mce/amd.c:1341 func:mce_threshold_create_device
> >>>
> >>
> >> Hi,
> >> The changes may  break some client tools, mine included....
> >> I don't mind adjusting my tools, but still
> >> Is it acceptable  to change
> >>       49152*      48* arch/x86/kernel/cpu/mce/core.c:2709 func:mce_device_create
> >> to
> >>       +49152      +48 arch/x86/kernel/cpu/mce/core.c:2709 func:mce_device_create*
> >>
> >> The '+' sign make it still standout when view from a terminal, and client tools, not all of them though, might not need any changes.
> >> And when client want to filter out inaccurate data items, it could be done by checking the tailing '*" of func name.
> >
> > I agree with David on this point. We already have monitoring tool built on top
> > of this output across meta fleet. Ideally we would like to keep the format of
> > of size and calls the same, even for future version, because adding a * will
> > change the format from int to str, which leads to change over the regex parser
> > many places.
> >
> > I think simply adding * to the end of function name or filename is sufficient
> > as they are already str.
> >
>
> Instead of:
>
> 49152*      48* arch/x86/kernel/cpu/mce/core.c:2709 func:mce_device_create
>
> Could we do something like:
>
> 49152      48 arch/x86/kernel/cpu/mce/core.c:2709 func:mce_device_create(inaccurate)

If there is a postprocessing then this would break sometimes later
when the function name is parsed, right? So IMO that just postpones
the breakage.

>
> This should hopefully not require any changes to the tools that are consuming this file.
> I think it might be better to use "(inaccurate)" (without any space after function name) or
> some other text instead of "+" or "*" to prevent breaking such tools. I dont think we need
> to even increment allocinfo version number as well then?

I'm wondering if we add a new column at the end like this:

49152      48 arch/x86/kernel/cpu/mce/core.c:2709
func:mce_device_create [inaccurate]

would that break the parsing tools?
Well-designed parsers usually throw away additional fields which they
don't know how to parse. WDYT?

>
> >>
> >> (There would be some corner cases, for example, the '+' sign may not needed when the value reach a negative value if some underflow bug happened)
> >>
> >>
> >> Thanks
> >> David.
> >>
> >>
> >>> Suggested-by: Johannes Weiner <hannes@cmpxchg.org>
> >>> Signed-off-by: Suren Baghdasaryan <surenb@google.com>
> >>> ---
> >>
> >
> > Thanks
> > Pan
>

Re: [PATCH 1/1] alloc_tag: mark inaccurate allocation counters in /proc/allocinfo output

[Yueyang Pan]

On Thu, Sep 11, 2025 at 09:18:29AM -0700, Suren Baghdasaryan wrote:
> On Thu, Sep 11, 2025 at 9:00 AM Usama Arif <usamaarif642@gmail.com> wrote:
> >
> >
> >
> > On 11/09/2025 16:47, Yueyang Pan wrote:
> > > On Thu, Sep 11, 2025 at 11:03:50PM +0800, David Wang wrote:
> > >>
> > >> At 2025-09-10 07:49:42, "Suren Baghdasaryan" <surenb@google.com> wrote:
> > >>> While rare, memory allocation profiling can contain inaccurate counters
> > >>> if slab object extension vector allocation fails. That allocation might
> > >>> succeed later but prior to that, slab allocations that would have used
> > >>> that object extension vector will not be accounted for. To indicate
> > >>> incorrect counters, mark them with an asterisk in the /proc/allocinfo
> > >>> output.
> > >>> Bump up /proc/allocinfo version to reflect change in the file format.
> > >>>
> > >>> Example output with invalid counters:
> > >>> allocinfo - version: 2.0
> > >>>           0        0 arch/x86/kernel/kdebugfs.c:105 func:create_setup_data_nodes
> > >>>           0        0 arch/x86/kernel/alternative.c:2090 func:alternatives_smp_module_add
> > >>>          0*       0* arch/x86/kernel/alternative.c:127 func:__its_alloc
> > >>>           0        0 arch/x86/kernel/fpu/regset.c:160 func:xstateregs_set
> > >>>           0        0 arch/x86/kernel/fpu/xstate.c:1590 func:fpstate_realloc
> > >>>           0        0 arch/x86/kernel/cpu/aperfmperf.c:379 func:arch_enable_hybrid_capacity_scale
> > >>>           0        0 arch/x86/kernel/cpu/amd_cache_disable.c:258 func:init_amd_l3_attrs
> > >>>      49152*      48* arch/x86/kernel/cpu/mce/core.c:2709 func:mce_device_create
> > >>>       32768        1 arch/x86/kernel/cpu/mce/genpool.c:132 func:mce_gen_pool_create
> > >>>           0        0 arch/x86/kernel/cpu/mce/amd.c:1341 func:mce_threshold_create_device
> > >>>
> > >>
> > >> Hi,
> > >> The changes may  break some client tools, mine included....
> > >> I don't mind adjusting my tools, but still
> > >> Is it acceptable  to change
> > >>       49152*      48* arch/x86/kernel/cpu/mce/core.c:2709 func:mce_device_create
> > >> to
> > >>       +49152      +48 arch/x86/kernel/cpu/mce/core.c:2709 func:mce_device_create*
> > >>
> > >> The '+' sign make it still standout when view from a terminal, and client tools, not all of them though, might not need any changes.
> > >> And when client want to filter out inaccurate data items, it could be done by checking the tailing '*" of func name.
> > >
> > > I agree with David on this point. We already have monitoring tool built on top
> > > of this output across meta fleet. Ideally we would like to keep the format of
> > > of size and calls the same, even for future version, because adding a * will
> > > change the format from int to str, which leads to change over the regex parser
> > > many places.
> > >
> > > I think simply adding * to the end of function name or filename is sufficient
> > > as they are already str.
> > >
> >
> > Instead of:
> >
> > 49152*      48* arch/x86/kernel/cpu/mce/core.c:2709 func:mce_device_create
> >
> > Could we do something like:
> >
> > 49152      48 arch/x86/kernel/cpu/mce/core.c:2709 func:mce_device_create(inaccurate)
> 
> If there is a postprocessing then this would break sometimes later
> when the function name is parsed, right? So IMO that just postpones
> the breakage.
> 
> >
> > This should hopefully not require any changes to the tools that are consuming this file.
> > I think it might be better to use "(inaccurate)" (without any space after function name) or
> > some other text instead of "+" or "*" to prevent breaking such tools. I dont think we need
> > to even increment allocinfo version number as well then?
> 
> I'm wondering if we add a new column at the end like this:
> 
> 49152      48 arch/x86/kernel/cpu/mce/core.c:2709
> func:mce_device_create [inaccurate]
> 
> would that break the parsing tools?
> Well-designed parsers usually throw away additional fields which they
> don't know how to parse. WDYT?
> 

It would break the parse now as we count the number of string to decide if 
there is an optional module name or not. I don't think it is a big 
deal to fix though.

I think one more important thing is probably to reach a consensus on 
what format can be changed in the future, for example say, we can 
keep adding columns but not change the format the type of one column.
With such consensus in mind, it will be easier to design the parser. 
And I guess many companies will build parser upon this info for fleet-
wise collection.

> >
> > >>
> > >> (There would be some corner cases, for example, the '+' sign may not needed when the value reach a negative value if some underflow bug happened)
> > >>
> > >>
> > >> Thanks
> > >> David.
> > >>
> > >>
> > >>> Suggested-by: Johannes Weiner <hannes@cmpxchg.org>
> > >>> Signed-off-by: Suren Baghdasaryan <surenb@google.com>
> > >>> ---
> > >>
> > >
> > > Thanks
> > > Pan
> >

Thanks
Pan

Re: [PATCH 1/1] alloc_tag: mark inaccurate allocation counters in /proc/allocinfo output

[David Wang]

At 2025-09-12 01:25:05, "Yueyang Pan" <pyyjason@gmail.com> wrote:
>On Thu, Sep 11, 2025 at 09:18:29AM -0700, Suren Baghdasaryan wrote:
>> On Thu, Sep 11, 2025 at 9:00 AM Usama Arif <usamaarif642@gmail.com> wrote:
>> >
>> >
>> >
>> > On 11/09/2025 16:47, Yueyang Pan wrote:
>> > > On Thu, Sep 11, 2025 at 11:03:50PM +0800, David Wang wrote:
>> > >>
>> > >> At 2025-09-10 07:49:42, "Suren Baghdasaryan" <surenb@google.com> wrote:
>> > >>> While rare, memory allocation profiling can contain inaccurate counters
>> > >>> if slab object extension vector allocation fails. That allocation might
>> > >>> succeed later but prior to that, slab allocations that would have used
>> > >>> that object extension vector will not be accounted for. To indicate
>> > >>> incorrect counters, mark them with an asterisk in the /proc/allocinfo
>> > >>> output.
>> > >>> Bump up /proc/allocinfo version to reflect change in the file format.
>> > >>>
>> > >>> Example output with invalid counters:
>> > >>> allocinfo - version: 2.0
>> > >>>           0        0 arch/x86/kernel/kdebugfs.c:105 func:create_setup_data_nodes
>> > >>>           0        0 arch/x86/kernel/alternative.c:2090 func:alternatives_smp_module_add
>> > >>>          0*       0* arch/x86/kernel/alternative.c:127 func:__its_alloc
>> > >>>           0        0 arch/x86/kernel/fpu/regset.c:160 func:xstateregs_set
>> > >>>           0        0 arch/x86/kernel/fpu/xstate.c:1590 func:fpstate_realloc
>> > >>>           0        0 arch/x86/kernel/cpu/aperfmperf.c:379 func:arch_enable_hybrid_capacity_scale
>> > >>>           0        0 arch/x86/kernel/cpu/amd_cache_disable.c:258 func:init_amd_l3_attrs
>> > >>>      49152*      48* arch/x86/kernel/cpu/mce/core.c:2709 func:mce_device_create
>> > >>>       32768        1 arch/x86/kernel/cpu/mce/genpool.c:132 func:mce_gen_pool_create
>> > >>>           0        0 arch/x86/kernel/cpu/mce/amd.c:1341 func:mce_threshold_create_device
>> > >>>
>> > >>
>> > >> Hi,
>> > >> The changes may  break some client tools, mine included....
>> > >> I don't mind adjusting my tools, but still
>> > >> Is it acceptable  to change
>> > >>       49152*      48* arch/x86/kernel/cpu/mce/core.c:2709 func:mce_device_create
>> > >> to
>> > >>       +49152      +48 arch/x86/kernel/cpu/mce/core.c:2709 func:mce_device_create*
>> > >>
>> > >> The '+' sign make it still standout when view from a terminal, and client tools, not all of them though, might not need any changes.
>> > >> And when client want to filter out inaccurate data items, it could be done by checking the tailing '*" of func name.
>> > >
>> > > I agree with David on this point. We already have monitoring tool built on top
>> > > of this output across meta fleet. Ideally we would like to keep the format of
>> > > of size and calls the same, even for future version, because adding a * will
>> > > change the format from int to str, which leads to change over the regex parser
>> > > many places.
>> > >
>> > > I think simply adding * to the end of function name or filename is sufficient
>> > > as they are already str.
>> > >
>> >
>> > Instead of:
>> >
>> > 49152*      48* arch/x86/kernel/cpu/mce/core.c:2709 func:mce_device_create
>> >
>> > Could we do something like:
>> >
>> > 49152      48 arch/x86/kernel/cpu/mce/core.c:2709 func:mce_device_create(inaccurate)
>> 
>> If there is a postprocessing then this would break sometimes later
>> when the function name is parsed, right? So IMO that just postpones
>> the breakage.
>> 
>> >
>> > This should hopefully not require any changes to the tools that are consuming this file.
>> > I think it might be better to use "(inaccurate)" (without any space after function name) or
>> > some other text instead of "+" or "*" to prevent breaking such tools. I dont think we need
>> > to even increment allocinfo version number as well then?
>> 
>> I'm wondering if we add a new column at the end like this:
>> 
>> 49152      48 arch/x86/kernel/cpu/mce/core.c:2709
>> func:mce_device_create [inaccurate]
>> 
>> would that break the parsing tools?
>> Well-designed parsers usually throw away additional fields which they
>> don't know how to parse. WDYT?
>> 
>
>It would break the parse now as we count the number of string to decide if 
>there is an optional module name or not. I don't think it is a big 
>deal to fix though.

The inconsistent of module name is really inconvenient for parsing.....  
Could we make changes to make it consistent, something like:

diff --git a/lib/codetag.c b/lib/codetag.c
index 545911cebd25..b8a4595adc95 100644
--- a/lib/codetag.c
+++ b/lib/codetag.c
@@ -124,7 +124,7 @@ void codetag_to_text(struct seq_buf *out, struct codetag *ct)
                               ct->filename, ct->lineno,
                               ct->modname, ct->function);
        else
-               seq_buf_printf(out, "%s:%u func:%s",
+               seq_buf_printf(out, "%s:%u [kernel] func:%s",
                               ct->filename, ct->lineno, ct->function);
 }




>
>I think one more important thing is probably to reach a consensus on 
>what format can be changed in the future, for example say, we can 
>keep adding columns but not change the format the type of one column.
>With such consensus in mind, it will be easier to design the parser. 
>And I guess many companies will build parser upon this info for fleet-
>wise collection.
>
>> >
>> > >>
>> > >> (There would be some corner cases, for example, the '+' sign may not needed when the value reach a negative value if some underflow bug happened)
>> > >>
>> > >>
>> > >> Thanks
>> > >> David.
>> > >>
>> > >>
>> > >>> Suggested-by: Johannes Weiner <hannes@cmpxchg.org>
>> > >>> Signed-off-by: Suren Baghdasaryan <surenb@google.com>
>> > >>> ---
>> > >>
>> > >
>> > > Thanks
>> > > Pan
>> >
>
>Thanks
>Pan

Re: [PATCH 1/1] alloc_tag: mark inaccurate allocation counters in /proc/allocinfo output

[Suren Baghdasaryan]

On Thu, Sep 11, 2025 at 10:35 AM David Wang <00107082@163.com> wrote:
>
>
> At 2025-09-12 01:25:05, "Yueyang Pan" <pyyjason@gmail.com> wrote:
> >On Thu, Sep 11, 2025 at 09:18:29AM -0700, Suren Baghdasaryan wrote:
> >> On Thu, Sep 11, 2025 at 9:00 AM Usama Arif <usamaarif642@gmail.com> wrote:
> >> >
> >> >
> >> >
> >> > On 11/09/2025 16:47, Yueyang Pan wrote:
> >> > > On Thu, Sep 11, 2025 at 11:03:50PM +0800, David Wang wrote:
> >> > >>
> >> > >> At 2025-09-10 07:49:42, "Suren Baghdasaryan" <surenb@google.com> wrote:
> >> > >>> While rare, memory allocation profiling can contain inaccurate counters
> >> > >>> if slab object extension vector allocation fails. That allocation might
> >> > >>> succeed later but prior to that, slab allocations that would have used
> >> > >>> that object extension vector will not be accounted for. To indicate
> >> > >>> incorrect counters, mark them with an asterisk in the /proc/allocinfo
> >> > >>> output.
> >> > >>> Bump up /proc/allocinfo version to reflect change in the file format.
> >> > >>>
> >> > >>> Example output with invalid counters:
> >> > >>> allocinfo - version: 2.0
> >> > >>>           0        0 arch/x86/kernel/kdebugfs.c:105 func:create_setup_data_nodes
> >> > >>>           0        0 arch/x86/kernel/alternative.c:2090 func:alternatives_smp_module_add
> >> > >>>          0*       0* arch/x86/kernel/alternative.c:127 func:__its_alloc
> >> > >>>           0        0 arch/x86/kernel/fpu/regset.c:160 func:xstateregs_set
> >> > >>>           0        0 arch/x86/kernel/fpu/xstate.c:1590 func:fpstate_realloc
> >> > >>>           0        0 arch/x86/kernel/cpu/aperfmperf.c:379 func:arch_enable_hybrid_capacity_scale
> >> > >>>           0        0 arch/x86/kernel/cpu/amd_cache_disable.c:258 func:init_amd_l3_attrs
> >> > >>>      49152*      48* arch/x86/kernel/cpu/mce/core.c:2709 func:mce_device_create
> >> > >>>       32768        1 arch/x86/kernel/cpu/mce/genpool.c:132 func:mce_gen_pool_create
> >> > >>>           0        0 arch/x86/kernel/cpu/mce/amd.c:1341 func:mce_threshold_create_device
> >> > >>>
> >> > >>
> >> > >> Hi,
> >> > >> The changes may  break some client tools, mine included....
> >> > >> I don't mind adjusting my tools, but still
> >> > >> Is it acceptable  to change
> >> > >>       49152*      48* arch/x86/kernel/cpu/mce/core.c:2709 func:mce_device_create
> >> > >> to
> >> > >>       +49152      +48 arch/x86/kernel/cpu/mce/core.c:2709 func:mce_device_create*
> >> > >>
> >> > >> The '+' sign make it still standout when view from a terminal, and client tools, not all of them though, might not need any changes.
> >> > >> And when client want to filter out inaccurate data items, it could be done by checking the tailing '*" of func name.
> >> > >
> >> > > I agree with David on this point. We already have monitoring tool built on top
> >> > > of this output across meta fleet. Ideally we would like to keep the format of
> >> > > of size and calls the same, even for future version, because adding a * will
> >> > > change the format from int to str, which leads to change over the regex parser
> >> > > many places.
> >> > >
> >> > > I think simply adding * to the end of function name or filename is sufficient
> >> > > as they are already str.
> >> > >
> >> >
> >> > Instead of:
> >> >
> >> > 49152*      48* arch/x86/kernel/cpu/mce/core.c:2709 func:mce_device_create
> >> >
> >> > Could we do something like:
> >> >
> >> > 49152      48 arch/x86/kernel/cpu/mce/core.c:2709 func:mce_device_create(inaccurate)
> >>
> >> If there is a postprocessing then this would break sometimes later
> >> when the function name is parsed, right? So IMO that just postpones
> >> the breakage.
> >>
> >> >
> >> > This should hopefully not require any changes to the tools that are consuming this file.
> >> > I think it might be better to use "(inaccurate)" (without any space after function name) or
> >> > some other text instead of "+" or "*" to prevent breaking such tools. I dont think we need
> >> > to even increment allocinfo version number as well then?
> >>
> >> I'm wondering if we add a new column at the end like this:
> >>
> >> 49152      48 arch/x86/kernel/cpu/mce/core.c:2709
> >> func:mce_device_create [inaccurate]
> >>
> >> would that break the parsing tools?
> >> Well-designed parsers usually throw away additional fields which they
> >> don't know how to parse. WDYT?
> >>
> >
> >It would break the parse now as we count the number of string to decide if
> >there is an optional module name or not. I don't think it is a big
> >deal to fix though.

Uh, right. We do have module name as an optional field...

>
> The inconsistent of module name is really inconvenient for parsing.....
> Could we make changes to make it consistent, something like:
>
> diff --git a/lib/codetag.c b/lib/codetag.c
> index 545911cebd25..b8a4595adc95 100644
> --- a/lib/codetag.c
> +++ b/lib/codetag.c
> @@ -124,7 +124,7 @@ void codetag_to_text(struct seq_buf *out, struct codetag *ct)
>                                ct->filename, ct->lineno,
>                                ct->modname, ct->function);
>         else
> -               seq_buf_printf(out, "%s:%u func:%s",
> +               seq_buf_printf(out, "%s:%u [kernel] func:%s",

Yeah, until someone creates a module called "kernel" :)
We could keep the name empty like this:

+               seq_buf_printf(out, "%s:%u [] func:%s",

but I'm not sure that's the best solution.

If we are really concerned about parsers, I could add an ioctl
interface to query the counters which are inaccurate. Would that be
better?

BTW, I have other ideas for ioctls, like filtering-out 0-sized
allocations and such.

>                                ct->filename, ct->lineno, ct->function);
>  }
>
>
>
>
> >
> >I think one more important thing is probably to reach a consensus on
> >what format can be changed in the future, for example say, we can
> >keep adding columns but not change the format the type of one column.
> >With such consensus in mind, it will be easier to design the parser.
> >And I guess many companies will build parser upon this info for fleet-
> >wise collection.
> >
> >> >
> >> > >>
> >> > >> (There would be some corner cases, for example, the '+' sign may not needed when the value reach a negative value if some underflow bug happened)
> >> > >>
> >> > >>
> >> > >> Thanks
> >> > >> David.
> >> > >>
> >> > >>
> >> > >>> Suggested-by: Johannes Weiner <hannes@cmpxchg.org>
> >> > >>> Signed-off-by: Suren Baghdasaryan <surenb@google.com>
> >> > >>> ---
> >> > >>
> >> > >
> >> > > Thanks
> >> > > Pan
> >> >
> >
> >Thanks
> >Pan

Re: [PATCH 1/1] alloc_tag: mark inaccurate allocation counters in /proc/allocinfo output

[Yueyang Pan]

On Thu, Sep 11, 2025 at 11:13:58AM -0700, Suren Baghdasaryan wrote:
> On Thu, Sep 11, 2025 at 10:35 AM David Wang <00107082@163.com> wrote:
> >
> >
> > At 2025-09-12 01:25:05, "Yueyang Pan" <pyyjason@gmail.com> wrote:
> > >On Thu, Sep 11, 2025 at 09:18:29AM -0700, Suren Baghdasaryan wrote:
> > >> On Thu, Sep 11, 2025 at 9:00 AM Usama Arif <usamaarif642@gmail.com> wrote:
> > >> >
> > >> >
> > >> >
> > >> > On 11/09/2025 16:47, Yueyang Pan wrote:
> > >> > > On Thu, Sep 11, 2025 at 11:03:50PM +0800, David Wang wrote:
> > >> > >>
> > >> > >> At 2025-09-10 07:49:42, "Suren Baghdasaryan" <surenb@google.com> wrote:
> > >> > >>> While rare, memory allocation profiling can contain inaccurate counters
> > >> > >>> if slab object extension vector allocation fails. That allocation might
> > >> > >>> succeed later but prior to that, slab allocations that would have used
> > >> > >>> that object extension vector will not be accounted for. To indicate
> > >> > >>> incorrect counters, mark them with an asterisk in the /proc/allocinfo
> > >> > >>> output.
> > >> > >>> Bump up /proc/allocinfo version to reflect change in the file format.
> > >> > >>>
> > >> > >>> Example output with invalid counters:
> > >> > >>> allocinfo - version: 2.0
> > >> > >>>           0        0 arch/x86/kernel/kdebugfs.c:105 func:create_setup_data_nodes
> > >> > >>>           0        0 arch/x86/kernel/alternative.c:2090 func:alternatives_smp_module_add
> > >> > >>>          0*       0* arch/x86/kernel/alternative.c:127 func:__its_alloc
> > >> > >>>           0        0 arch/x86/kernel/fpu/regset.c:160 func:xstateregs_set
> > >> > >>>           0        0 arch/x86/kernel/fpu/xstate.c:1590 func:fpstate_realloc
> > >> > >>>           0        0 arch/x86/kernel/cpu/aperfmperf.c:379 func:arch_enable_hybrid_capacity_scale
> > >> > >>>           0        0 arch/x86/kernel/cpu/amd_cache_disable.c:258 func:init_amd_l3_attrs
> > >> > >>>      49152*      48* arch/x86/kernel/cpu/mce/core.c:2709 func:mce_device_create
> > >> > >>>       32768        1 arch/x86/kernel/cpu/mce/genpool.c:132 func:mce_gen_pool_create
> > >> > >>>           0        0 arch/x86/kernel/cpu/mce/amd.c:1341 func:mce_threshold_create_device
> > >> > >>>
> > >> > >>
> > >> > >> Hi,
> > >> > >> The changes may  break some client tools, mine included....
> > >> > >> I don't mind adjusting my tools, but still
> > >> > >> Is it acceptable  to change
> > >> > >>       49152*      48* arch/x86/kernel/cpu/mce/core.c:2709 func:mce_device_create
> > >> > >> to
> > >> > >>       +49152      +48 arch/x86/kernel/cpu/mce/core.c:2709 func:mce_device_create*
> > >> > >>
> > >> > >> The '+' sign make it still standout when view from a terminal, and client tools, not all of them though, might not need any changes.
> > >> > >> And when client want to filter out inaccurate data items, it could be done by checking the tailing '*" of func name.
> > >> > >
> > >> > > I agree with David on this point. We already have monitoring tool built on top
> > >> > > of this output across meta fleet. Ideally we would like to keep the format of
> > >> > > of size and calls the same, even for future version, because adding a * will
> > >> > > change the format from int to str, which leads to change over the regex parser
> > >> > > many places.
> > >> > >
> > >> > > I think simply adding * to the end of function name or filename is sufficient
> > >> > > as they are already str.
> > >> > >
> > >> >
> > >> > Instead of:
> > >> >
> > >> > 49152*      48* arch/x86/kernel/cpu/mce/core.c:2709 func:mce_device_create
> > >> >
> > >> > Could we do something like:
> > >> >
> > >> > 49152      48 arch/x86/kernel/cpu/mce/core.c:2709 func:mce_device_create(inaccurate)
> > >>
> > >> If there is a postprocessing then this would break sometimes later
> > >> when the function name is parsed, right? So IMO that just postpones
> > >> the breakage.
> > >>
> > >> >
> > >> > This should hopefully not require any changes to the tools that are consuming this file.
> > >> > I think it might be better to use "(inaccurate)" (without any space after function name) or
> > >> > some other text instead of "+" or "*" to prevent breaking such tools. I dont think we need
> > >> > to even increment allocinfo version number as well then?
> > >>
> > >> I'm wondering if we add a new column at the end like this:
> > >>
> > >> 49152      48 arch/x86/kernel/cpu/mce/core.c:2709
> > >> func:mce_device_create [inaccurate]
> > >>
> > >> would that break the parsing tools?
> > >> Well-designed parsers usually throw away additional fields which they
> > >> don't know how to parse. WDYT?
> > >>
> > >
> > >It would break the parse now as we count the number of string to decide if
> > >there is an optional module name or not. I don't think it is a big
> > >deal to fix though.
> 
> Uh, right. We do have module name as an optional field...
> 
> >
> > The inconsistent of module name is really inconvenient for parsing.....
> > Could we make changes to make it consistent, something like:
> >
> > diff --git a/lib/codetag.c b/lib/codetag.c
> > index 545911cebd25..b8a4595adc95 100644
> > --- a/lib/codetag.c
> > +++ b/lib/codetag.c
> > @@ -124,7 +124,7 @@ void codetag_to_text(struct seq_buf *out, struct codetag *ct)
> >                                ct->filename, ct->lineno,
> >                                ct->modname, ct->function);
> >         else
> > -               seq_buf_printf(out, "%s:%u func:%s",
> > +               seq_buf_printf(out, "%s:%u [kernel] func:%s",
> 
> Yeah, until someone creates a module called "kernel" :)
> We could keep the name empty like this:
> 
> +               seq_buf_printf(out, "%s:%u [] func:%s",
> 
> but I'm not sure that's the best solution.
> 

I guess the best option would be to decide how the format can evolve 
in the future with some rules in comment or doc. But I am sure who 
will decide the rules...

> If we are really concerned about parsers, I could add an ioctl
> interface to query the counters which are inaccurate. Would that be
> better?

I think this would be nice as we just need to add functionality on 
top of the parser when we do need it. I guess most of the time we don't 
care about that temporary imprecision.

> 
> BTW, I have other ideas for ioctls, like filtering-out 0-sized
> allocations and such.

You mean using ioctl to control if we only print out the non zero 
ones?

> 
> >                                ct->filename, ct->lineno, ct->function);
> >  }
> >
> >
> >
> >
> > >
> > >I think one more important thing is probably to reach a consensus on
> > >what format can be changed in the future, for example say, we can
> > >keep adding columns but not change the format the type of one column.
> > >With such consensus in mind, it will be easier to design the parser.
> > >And I guess many companies will build parser upon this info for fleet-
> > >wise collection.
> > >
> > >> >
> > >> > >>
> > >> > >> (There would be some corner cases, for example, the '+' sign may not needed when the value reach a negative value if some underflow bug happened)
> > >> > >>
> > >> > >>
> > >> > >> Thanks
> > >> > >> David.
> > >> > >>
> > >> > >>
> > >> > >>> Suggested-by: Johannes Weiner <hannes@cmpxchg.org>
> > >> > >>> Signed-off-by: Suren Baghdasaryan <surenb@google.com>
> > >> > >>> ---
> > >> > >>
> > >> > >
> > >> > > Thanks
> > >> > > Pan
> > >> >
> > >
> > >Thanks
> > >Pan

Thanks
Pan

Re: [PATCH 1/1] alloc_tag: mark inaccurate allocation counters in /proc/allocinfo output

[Suren Baghdasaryan]

On Thu, Sep 11, 2025 at 11:51 AM Yueyang Pan <pyyjason@gmail.com> wrote:
>
> On Thu, Sep 11, 2025 at 11:13:58AM -0700, Suren Baghdasaryan wrote:
> > On Thu, Sep 11, 2025 at 10:35 AM David Wang <00107082@163.com> wrote:
> > >
> > >
> > > At 2025-09-12 01:25:05, "Yueyang Pan" <pyyjason@gmail.com> wrote:
> > > >On Thu, Sep 11, 2025 at 09:18:29AM -0700, Suren Baghdasaryan wrote:
> > > >> On Thu, Sep 11, 2025 at 9:00 AM Usama Arif <usamaarif642@gmail.com> wrote:
> > > >> >
> > > >> >
> > > >> >
> > > >> > On 11/09/2025 16:47, Yueyang Pan wrote:
> > > >> > > On Thu, Sep 11, 2025 at 11:03:50PM +0800, David Wang wrote:
> > > >> > >>
> > > >> > >> At 2025-09-10 07:49:42, "Suren Baghdasaryan" <surenb@google.com> wrote:
> > > >> > >>> While rare, memory allocation profiling can contain inaccurate counters
> > > >> > >>> if slab object extension vector allocation fails. That allocation might
> > > >> > >>> succeed later but prior to that, slab allocations that would have used
> > > >> > >>> that object extension vector will not be accounted for. To indicate
> > > >> > >>> incorrect counters, mark them with an asterisk in the /proc/allocinfo
> > > >> > >>> output.
> > > >> > >>> Bump up /proc/allocinfo version to reflect change in the file format.
> > > >> > >>>
> > > >> > >>> Example output with invalid counters:
> > > >> > >>> allocinfo - version: 2.0
> > > >> > >>>           0        0 arch/x86/kernel/kdebugfs.c:105 func:create_setup_data_nodes
> > > >> > >>>           0        0 arch/x86/kernel/alternative.c:2090 func:alternatives_smp_module_add
> > > >> > >>>          0*       0* arch/x86/kernel/alternative.c:127 func:__its_alloc
> > > >> > >>>           0        0 arch/x86/kernel/fpu/regset.c:160 func:xstateregs_set
> > > >> > >>>           0        0 arch/x86/kernel/fpu/xstate.c:1590 func:fpstate_realloc
> > > >> > >>>           0        0 arch/x86/kernel/cpu/aperfmperf.c:379 func:arch_enable_hybrid_capacity_scale
> > > >> > >>>           0        0 arch/x86/kernel/cpu/amd_cache_disable.c:258 func:init_amd_l3_attrs
> > > >> > >>>      49152*      48* arch/x86/kernel/cpu/mce/core.c:2709 func:mce_device_create
> > > >> > >>>       32768        1 arch/x86/kernel/cpu/mce/genpool.c:132 func:mce_gen_pool_create
> > > >> > >>>           0        0 arch/x86/kernel/cpu/mce/amd.c:1341 func:mce_threshold_create_device
> > > >> > >>>
> > > >> > >>
> > > >> > >> Hi,
> > > >> > >> The changes may  break some client tools, mine included....
> > > >> > >> I don't mind adjusting my tools, but still
> > > >> > >> Is it acceptable  to change
> > > >> > >>       49152*      48* arch/x86/kernel/cpu/mce/core.c:2709 func:mce_device_create
> > > >> > >> to
> > > >> > >>       +49152      +48 arch/x86/kernel/cpu/mce/core.c:2709 func:mce_device_create*
> > > >> > >>
> > > >> > >> The '+' sign make it still standout when view from a terminal, and client tools, not all of them though, might not need any changes.
> > > >> > >> And when client want to filter out inaccurate data items, it could be done by checking the tailing '*" of func name.
> > > >> > >
> > > >> > > I agree with David on this point. We already have monitoring tool built on top
> > > >> > > of this output across meta fleet. Ideally we would like to keep the format of
> > > >> > > of size and calls the same, even for future version, because adding a * will
> > > >> > > change the format from int to str, which leads to change over the regex parser
> > > >> > > many places.
> > > >> > >
> > > >> > > I think simply adding * to the end of function name or filename is sufficient
> > > >> > > as they are already str.
> > > >> > >
> > > >> >
> > > >> > Instead of:
> > > >> >
> > > >> > 49152*      48* arch/x86/kernel/cpu/mce/core.c:2709 func:mce_device_create
> > > >> >
> > > >> > Could we do something like:
> > > >> >
> > > >> > 49152      48 arch/x86/kernel/cpu/mce/core.c:2709 func:mce_device_create(inaccurate)
> > > >>
> > > >> If there is a postprocessing then this would break sometimes later
> > > >> when the function name is parsed, right? So IMO that just postpones
> > > >> the breakage.
> > > >>
> > > >> >
> > > >> > This should hopefully not require any changes to the tools that are consuming this file.
> > > >> > I think it might be better to use "(inaccurate)" (without any space after function name) or
> > > >> > some other text instead of "+" or "*" to prevent breaking such tools. I dont think we need
> > > >> > to even increment allocinfo version number as well then?
> > > >>
> > > >> I'm wondering if we add a new column at the end like this:
> > > >>
> > > >> 49152      48 arch/x86/kernel/cpu/mce/core.c:2709
> > > >> func:mce_device_create [inaccurate]
> > > >>
> > > >> would that break the parsing tools?
> > > >> Well-designed parsers usually throw away additional fields which they
> > > >> don't know how to parse. WDYT?
> > > >>
> > > >
> > > >It would break the parse now as we count the number of string to decide if
> > > >there is an optional module name or not. I don't think it is a big
> > > >deal to fix though.
> >
> > Uh, right. We do have module name as an optional field...
> >
> > >
> > > The inconsistent of module name is really inconvenient for parsing.....
> > > Could we make changes to make it consistent, something like:
> > >
> > > diff --git a/lib/codetag.c b/lib/codetag.c
> > > index 545911cebd25..b8a4595adc95 100644
> > > --- a/lib/codetag.c
> > > +++ b/lib/codetag.c
> > > @@ -124,7 +124,7 @@ void codetag_to_text(struct seq_buf *out, struct codetag *ct)
> > >                                ct->filename, ct->lineno,
> > >                                ct->modname, ct->function);
> > >         else
> > > -               seq_buf_printf(out, "%s:%u func:%s",
> > > +               seq_buf_printf(out, "%s:%u [kernel] func:%s",
> >
> > Yeah, until someone creates a module called "kernel" :)
> > We could keep the name empty like this:
> >
> > +               seq_buf_printf(out, "%s:%u [] func:%s",
> >
> > but I'm not sure that's the best solution.
> >
>
> I guess the best option would be to decide how the format can evolve
> in the future with some rules in comment or doc. But I am sure who
> will decide the rules...
>
> > If we are really concerned about parsers, I could add an ioctl
> > interface to query the counters which are inaccurate. Would that be
> > better?
>
> I think this would be nice as we just need to add functionality on
> top of the parser when we do need it. I guess most of the time we don't
> care about that temporary imprecision.

We don't care about it until it happens :) I think it would be nice to
have a very visible indication that some values are inaccurate. ioclt
would not do that unfortunately...
Another option is to use the "[]" where we currently encode only
module name to express extra info:

No module, accurate counters:
0        0 arch/x86/kernel/kdebugfs.c:105 func:create_setup_data_nodes

Module, accurate counters:
0        0 arch/x86/kernel/kdebugfs.c:105 [my_module]
func:create_setup_data_nodes

No module, inaccurate counters:
0        0 arch/x86/kernel/kdebugfs.c:105 [,inaccurate]
func:create_setup_data_nodes

Module, inaccurate counters:
0        0 arch/x86/kernel/kdebugfs.c:105 [my_module,inaccurate]
func:create_setup_data_nodes

Then the rule for parsers would be that whatever is in [] can be
extended with additional values. If they do not recognize the value,
just ignore it.

>
> >
> > BTW, I have other ideas for ioctls, like filtering-out 0-sized
> > allocations and such.
>
> You mean using ioctl to control if we only print out the non zero
> ones?

Correct.

>
> >
> > >                                ct->filename, ct->lineno, ct->function);
> > >  }
> > >
> > >
> > >
> > >
> > > >
> > > >I think one more important thing is probably to reach a consensus on
> > > >what format can be changed in the future, for example say, we can
> > > >keep adding columns but not change the format the type of one column.
> > > >With such consensus in mind, it will be easier to design the parser.
> > > >And I guess many companies will build parser upon this info for fleet-
> > > >wise collection.
> > > >
> > > >> >
> > > >> > >>
> > > >> > >> (There would be some corner cases, for example, the '+' sign may not needed when the value reach a negative value if some underflow bug happened)
> > > >> > >>
> > > >> > >>
> > > >> > >> Thanks
> > > >> > >> David.
> > > >> > >>
> > > >> > >>
> > > >> > >>> Suggested-by: Johannes Weiner <hannes@cmpxchg.org>
> > > >> > >>> Signed-off-by: Suren Baghdasaryan <surenb@google.com>
> > > >> > >>> ---
> > > >> > >>
> > > >> > >
> > > >> > > Thanks
> > > >> > > Pan
> > > >> >
> > > >
> > > >Thanks
> > > >Pan
>
> Thanks
> Pan

Re: [PATCH 1/1] alloc_tag: mark inaccurate allocation counters in /proc/allocinfo output

[Andrew Morton]

On Thu, 11 Sep 2025 12:00:23 -0400 Usama Arif <usamaarif642@gmail.com> wrote:

> > I think simply adding * to the end of function name or filename is sufficient 
> > as they are already str.
> > 
> 
> Instead of:
> 
> 49152*      48* arch/x86/kernel/cpu/mce/core.c:2709 func:mce_device_create
> 
> Could we do something like:
> 
> 49152      48 arch/x86/kernel/cpu/mce/core.c:2709 func:mce_device_create(inaccurate)

Can we add another row, saying "the previous row was inaccurate"?  I
guess that would break parsers also.



I don't know if this was by design, but the present format does provide
extensibility.  It is basically

	NNNN NNN name:value name:value

one could arguably append a third name:value and hope that authors of
existing parsers figured this out.


Whatev.  I'll drop this version from mm.git.

Re: [PATCH 1/1] alloc_tag: mark inaccurate allocation counters in /proc/allocinfo output

[Suren Baghdasaryan]

On Thu, Sep 11, 2025 at 2:31 PM Andrew Morton <akpm@linux-foundation.org> wrote:
>
> On Thu, 11 Sep 2025 12:00:23 -0400 Usama Arif <usamaarif642@gmail.com> wrote:
>
> > > I think simply adding * to the end of function name or filename is sufficient
> > > as they are already str.
> > >
> >
> > Instead of:
> >
> > 49152*      48* arch/x86/kernel/cpu/mce/core.c:2709 func:mce_device_create
> >
> > Could we do something like:
> >
> > 49152      48 arch/x86/kernel/cpu/mce/core.c:2709 func:mce_device_create(inaccurate)
>
> Can we add another row, saying "the previous row was inaccurate"?  I
> guess that would break parsers also.
>
>
>
> I don't know if this was by design, but the present format does provide
> extensibility.  It is basically
>
>         NNNN NNN name:value name:value
>
> one could arguably append a third name:value and hope that authors of
> existing parsers figured this out.

Actually that sounds like the best idea so far. Currently the format is:

<bytes> <count> <file>:<line> [<module>] func:<function>

We can adopt a rule that after this, the line can contain additional
key:value pairs. In that case for inaccurate lines we can add:

49152      48 arch/x86/kernel/cpu/mce/core.c:2709
func:mce_device_create accurate:no

In the future we can append more key:value pairs if we need them.
Parsers which don't know how to parse a new key can simply ignore
them.

Does that sound good to everyone?

>
>
> Whatev.  I'll drop this version from mm.git.

Re: [PATCH 1/1] alloc_tag: mark inaccurate allocation counters in /proc/allocinfo output

[David Wang]

At 2025-09-12 08:25:12, "Suren Baghdasaryan" <surenb@google.com> wrote:
>On Thu, Sep 11, 2025 at 2:31 PM Andrew Morton <akpm@linux-foundation.org> wrote:
>>
>> On Thu, 11 Sep 2025 12:00:23 -0400 Usama Arif <usamaarif642@gmail.com> wrote:
>>
>> > > I think simply adding * to the end of function name or filename is sufficient
>> > > as they are already str.
>> > >
>> >
>> > Instead of:
>> >
>> > 49152*      48* arch/x86/kernel/cpu/mce/core.c:2709 func:mce_device_create
>> >
>> > Could we do something like:
>> >
>> > 49152      48 arch/x86/kernel/cpu/mce/core.c:2709 func:mce_device_create(inaccurate)
>>
>> Can we add another row, saying "the previous row was inaccurate"?  I
>> guess that would break parsers also.
>>
>>
>>
>> I don't know if this was by design, but the present format does provide
>> extensibility.  It is basically
>>
>>         NNNN NNN name:value name:value
>>
>> one could arguably append a third name:value and hope that authors of
>> existing parsers figured this out.
>
>Actually that sounds like the best idea so far. Currently the format is:
>
><bytes> <count> <file>:<line> [<module>] func:<function>
>
>We can adopt a rule that after this, the line can contain additional
>key:value pairs. In that case for inaccurate lines we can add:
>
>49152      48 arch/x86/kernel/cpu/mce/core.c:2709
>func:mce_device_create accurate:no
>
>In the future we can append more key:value pairs if we need them.
>Parsers which don't know how to parse a new key can simply ignore
>them.
>
>Does that sound good to everyone?

This looks good to me, at least for my tools. :)
On my system, there are 4K+ lines of items, each byte added would increase 4K+bytes for one read,
but good thing is normally "accurate:no" would not show up. 

David

>
>>
>>
>> Whatev.  I'll drop this version from mm.git.

Re: [PATCH 1/1] alloc_tag: mark inaccurate allocation counters in /proc/allocinfo output

[Yueyang Pan]

On Thu, Sep 11, 2025 at 05:25:12PM -0700, Suren Baghdasaryan wrote:
> On Thu, Sep 11, 2025 at 2:31 PM Andrew Morton <akpm@linux-foundation.org> wrote:
> >
> > On Thu, 11 Sep 2025 12:00:23 -0400 Usama Arif <usamaarif642@gmail.com> wrote:
> >
> > > > I think simply adding * to the end of function name or filename is sufficient
> > > > as they are already str.
> > > >
> > >
> > > Instead of:
> > >
> > > 49152*      48* arch/x86/kernel/cpu/mce/core.c:2709 func:mce_device_create
> > >
> > > Could we do something like:
> > >
> > > 49152      48 arch/x86/kernel/cpu/mce/core.c:2709 func:mce_device_create(inaccurate)
> >
> > Can we add another row, saying "the previous row was inaccurate"?  I
> > guess that would break parsers also.
> >
> >
> >
> > I don't know if this was by design, but the present format does provide
> > extensibility.  It is basically
> >
> >         NNNN NNN name:value name:value
> >
> > one could arguably append a third name:value and hope that authors of
> > existing parsers figured this out.
> 
> Actually that sounds like the best idea so far. Currently the format is:
> 
> <bytes> <count> <file>:<line> [<module>] func:<function>
> 
> We can adopt a rule that after this, the line can contain additional
> key:value pairs. In that case for inaccurate lines we can add:
> 
> 49152      48 arch/x86/kernel/cpu/mce/core.c:2709
> func:mce_device_create accurate:no
> 
> In the future we can append more key:value pairs if we need them.
> Parsers which don't know how to parse a new key can simply ignore
> them.
> 
> Does that sound good to everyone?

Yeah I agree on this proposal. We can keep this convention.

> 
> >
> >
> > Whatev.  I'll drop this version from mm.git.

Re: [PATCH 1/1] alloc_tag: mark inaccurate allocation counters in /proc/allocinfo output

[Suren Baghdasaryan]

On Fri, Sep 12, 2025 at 3:52 AM Yueyang Pan <pyyjason@gmail.com> wrote:
>
> On Thu, Sep 11, 2025 at 05:25:12PM -0700, Suren Baghdasaryan wrote:
> > On Thu, Sep 11, 2025 at 2:31 PM Andrew Morton <akpm@linux-foundation.org> wrote:
> > >
> > > On Thu, 11 Sep 2025 12:00:23 -0400 Usama Arif <usamaarif642@gmail.com> wrote:
> > >
> > > > > I think simply adding * to the end of function name or filename is sufficient
> > > > > as they are already str.
> > > > >
> > > >
> > > > Instead of:
> > > >
> > > > 49152*      48* arch/x86/kernel/cpu/mce/core.c:2709 func:mce_device_create
> > > >
> > > > Could we do something like:
> > > >
> > > > 49152      48 arch/x86/kernel/cpu/mce/core.c:2709 func:mce_device_create(inaccurate)
> > >
> > > Can we add another row, saying "the previous row was inaccurate"?  I
> > > guess that would break parsers also.
> > >
> > >
> > >
> > > I don't know if this was by design, but the present format does provide
> > > extensibility.  It is basically
> > >
> > >         NNNN NNN name:value name:value
> > >
> > > one could arguably append a third name:value and hope that authors of
> > > existing parsers figured this out.
> >
> > Actually that sounds like the best idea so far. Currently the format is:
> >
> > <bytes> <count> <file>:<line> [<module>] func:<function>
> >
> > We can adopt a rule that after this, the line can contain additional
> > key:value pairs. In that case for inaccurate lines we can add:
> >
> > 49152      48 arch/x86/kernel/cpu/mce/core.c:2709
> > func:mce_device_create accurate:no
> >
> > In the future we can append more key:value pairs if we need them.
> > Parsers which don't know how to parse a new key can simply ignore
> > them.
> >
> > Does that sound good to everyone?
>
> Yeah I agree on this proposal. We can keep this convention.

Ok, if no further objections I'll post the next version and will
document that v2 allows additional key:value pairs in each line.
Thanks,
Suren.

>
> >
> > >
> > >
> > > Whatev.  I'll drop this version from mm.git.

Re: [PATCH 1/1] alloc_tag: mark inaccurate allocation counters in /proc/allocinfo output

[Yueyang Pan]

On Fri, Sep 12, 2025 at 12:38:01PM -0700, Suren Baghdasaryan wrote:
> On Fri, Sep 12, 2025 at 3:52 AM Yueyang Pan <pyyjason@gmail.com> wrote:
> >
> > On Thu, Sep 11, 2025 at 05:25:12PM -0700, Suren Baghdasaryan wrote:
> > > On Thu, Sep 11, 2025 at 2:31 PM Andrew Morton <akpm@linux-foundation.org> wrote:
> > > >
> > > > On Thu, 11 Sep 2025 12:00:23 -0400 Usama Arif <usamaarif642@gmail.com> wrote:
> > > >
> > > > > > I think simply adding * to the end of function name or filename is sufficient
> > > > > > as they are already str.
> > > > > >
> > > > >
> > > > > Instead of:
> > > > >
> > > > > 49152*      48* arch/x86/kernel/cpu/mce/core.c:2709 func:mce_device_create
> > > > >
> > > > > Could we do something like:
> > > > >
> > > > > 49152      48 arch/x86/kernel/cpu/mce/core.c:2709 func:mce_device_create(inaccurate)
> > > >
> > > > Can we add another row, saying "the previous row was inaccurate"?  I
> > > > guess that would break parsers also.
> > > >
> > > >
> > > >
> > > > I don't know if this was by design, but the present format does provide
> > > > extensibility.  It is basically
> > > >
> > > >         NNNN NNN name:value name:value
> > > >
> > > > one could arguably append a third name:value and hope that authors of
> > > > existing parsers figured this out.
> > >
> > > Actually that sounds like the best idea so far. Currently the format is:
> > >
> > > <bytes> <count> <file>:<line> [<module>] func:<function>
> > >
> > > We can adopt a rule that after this, the line can contain additional
> > > key:value pairs. In that case for inaccurate lines we can add:
> > >
> > > 49152      48 arch/x86/kernel/cpu/mce/core.c:2709
> > > func:mce_device_create accurate:no
> > >
> > > In the future we can append more key:value pairs if we need them.
> > > Parsers which don't know how to parse a new key can simply ignore
> > > them.
> > >
> > > Does that sound good to everyone?
> >
> > Yeah I agree on this proposal. We can keep this convention.
> 
> Ok, if no further objections I'll post the next version and will
> document that v2 allows additional key:value pairs in each line.
> Thanks,
> Suren.
> 

Agree on this.

> >
> > >
> > > >
> > > >
> > > > Whatev.  I'll drop this version from mm.git.

Thanks
Pan

Re: [PATCH 1/1] alloc_tag: mark inaccurate allocation counters in /proc/allocinfo output

[Suren Baghdasaryan]

On Thu, Sep 11, 2025 at 2:31 PM Andrew Morton <akpm@linux-foundation.org> wrote:
>
> On Thu, 11 Sep 2025 12:00:23 -0400 Usama Arif <usamaarif642@gmail.com> wrote:
>
> > > I think simply adding * to the end of function name or filename is sufficient
> > > as they are already str.
> > >
> >
> > Instead of:
> >
> > 49152*      48* arch/x86/kernel/cpu/mce/core.c:2709 func:mce_device_create
> >
> > Could we do something like:
> >
> > 49152      48 arch/x86/kernel/cpu/mce/core.c:2709 func:mce_device_create(inaccurate)
>
> Can we add another row, saying "the previous row was inaccurate"?  I
> guess that would break parsers also.
>
>
>
> I don't know if this was by design, but the present format does provide
> extensibility.  It is basically
>
>         NNNN NNN name:value name:value
>
> one could arguably append a third name:value and hope that authors of
> existing parsers figured this out.
>
>
> Whatev.  I'll drop this version from mm.git.

v2 is posted at:
https://lore.kernel.org/all/20250915230224.4115531-1-surenb@google.com/
Thanks!