[PATCH] trace: allocate fields with elt struct

[PATCH] trace: allocate fields with elt struct

[Rosen Penev]

Use a flexible array member to embed the fields array in the
tracing_map_elt allocation, reducing the number of allocations
per element.

Since the fields are now embedded in the struct, taking the address
of a field through a const-qualified elt pointer yields a
const-qualified pointer. Rather than adding casts, switch the
comparison functions to take const void * parameters. These are
all read-only operations.

Assisted-by: OpenCode:BigPickle
Signed-off-by: Rosen Penev <rosenp@gmail.com>
---
 kernel/trace/tracing_map.c | 41 ++++++++++++++++----------------------
 kernel/trace/tracing_map.h |  8 ++++----
 2 files changed, 21 insertions(+), 28 deletions(-)

diff --git a/kernel/trace/tracing_map.c b/kernel/trace/tracing_map.c
index 1404bf752d99..97f7e3cde262 100644
--- a/kernel/trace/tracing_map.c
+++ b/kernel/trace/tracing_map.c
@@ -125,32 +125,32 @@ u64 tracing_map_read_var_once(struct tracing_map_elt *elt, unsigned int i)
 	return (u64)atomic64_read(&elt->vars[i]);
 }
 
-int tracing_map_cmp_string(void *val_a, void *val_b)
+int tracing_map_cmp_string(const void *val_a, const void *val_b)
 {
-	char *a = val_a;
-	char *b = val_b;
+	const char *a = val_a;
+	const char *b = val_b;
 
 	return strcmp(a, b);
 }
 
-int tracing_map_cmp_none(void *val_a, void *val_b)
+int tracing_map_cmp_none(const void *val_a, const void *val_b)
 {
 	return 0;
 }
 
-static int tracing_map_cmp_atomic64(void *val_a, void *val_b)
+static int tracing_map_cmp_atomic64(const void *val_a, const void *val_b)
 {
-	u64 a = atomic64_read((atomic64_t *)val_a);
-	u64 b = atomic64_read((atomic64_t *)val_b);
+	u64 a = atomic64_read((const atomic64_t *)val_a);
+	u64 b = atomic64_read((const atomic64_t *)val_b);
 
 	return (a > b) ? 1 : ((a < b) ? -1 : 0);
 }
 
 #define DEFINE_TRACING_MAP_CMP_FN(type)					\
-static int tracing_map_cmp_##type(void *val_a, void *val_b)		\
+static int tracing_map_cmp_##type(const void *val_a, const void *val_b)	\
 {									\
-	type a = (type)(*(u64 *)val_a);					\
-	type b = (type)(*(u64 *)val_b);					\
+	type a = (type)(*(const u64 *)val_a);				\
+	type b = (type)(*(const u64 *)val_b);				\
 									\
 	return (a > b) ? 1 : ((a < b) ? -1 : 0);			\
 }
@@ -385,7 +385,6 @@ static void tracing_map_elt_free(struct tracing_map_elt *elt)
 
 	if (elt->map->ops && elt->map->ops->elt_free)
 		elt->map->ops->elt_free(elt);
-	kfree(elt->fields);
 	kfree(elt->vars);
 	kfree(elt->var_set);
 	kfree(elt->key);
@@ -397,7 +396,7 @@ static struct tracing_map_elt *tracing_map_elt_alloc(struct tracing_map *map)
 	struct tracing_map_elt *elt;
 	int err = 0;
 
-	elt = kzalloc_obj(*elt);
+	elt = kzalloc_flex(*elt, fields, map->n_fields);
 	if (!elt)
 		return ERR_PTR(-ENOMEM);
 
@@ -409,12 +408,6 @@ static struct tracing_map_elt *tracing_map_elt_alloc(struct tracing_map *map)
 		goto free;
 	}
 
-	elt->fields = kzalloc_objs(*elt->fields, map->n_fields);
-	if (!elt->fields) {
-		err = -ENOMEM;
-		goto free;
-	}
-
 	elt->vars = kzalloc_objs(*elt->vars, map->n_vars);
 	if (!elt->vars) {
 		err = -ENOMEM;
@@ -848,10 +841,10 @@ static int cmp_entries_sum(const void *A, const void *B)
 {
 	const struct tracing_map_elt *elt_a, *elt_b;
 	const struct tracing_map_sort_entry *a, *b;
-	struct tracing_map_sort_key *sort_key;
-	struct tracing_map_field *field;
+	const struct tracing_map_sort_key *sort_key;
+	const struct tracing_map_field *field;
 	tracing_map_cmp_fn_t cmp_fn;
-	void *val_a, *val_b;
+	const void *val_a, *val_b;
 	int ret = 0;
 
 	a = *(const struct tracing_map_sort_entry **)A;
@@ -879,10 +872,10 @@ static int cmp_entries_key(const void *A, const void *B)
 {
 	const struct tracing_map_elt *elt_a, *elt_b;
 	const struct tracing_map_sort_entry *a, *b;
-	struct tracing_map_sort_key *sort_key;
-	struct tracing_map_field *field;
+	const struct tracing_map_sort_key *sort_key;
+	const struct tracing_map_field *field;
 	tracing_map_cmp_fn_t cmp_fn;
-	void *val_a, *val_b;
+	const void *val_a, *val_b;
 	int ret = 0;
 
 	a = *(const struct tracing_map_sort_entry **)A;
diff --git a/kernel/trace/tracing_map.h b/kernel/trace/tracing_map.h
index 18a02959d77b..90a7fde5dd02 100644
--- a/kernel/trace/tracing_map.h
+++ b/kernel/trace/tracing_map.h
@@ -13,7 +13,7 @@
 #define TRACING_MAP_VARS_MAX		16
 #define TRACING_MAP_SORT_KEYS_MAX	2
 
-typedef int (*tracing_map_cmp_fn_t) (void *val_a, void *val_b);
+typedef int (*tracing_map_cmp_fn_t) (const void *val_a, const void *val_b);
 
 /*
  * This is an overview of the tracing_map data structures and how they
@@ -137,11 +137,11 @@ struct tracing_map_field {
 
 struct tracing_map_elt {
 	struct tracing_map		*map;
-	struct tracing_map_field	*fields;
 	atomic64_t			*vars;
 	bool				*var_set;
 	void				*key;
 	void				*private_data;
+	struct tracing_map_field	fields[];
 };
 
 struct tracing_map_entry {
@@ -260,8 +260,8 @@ tracing_map_lookup(struct tracing_map *map, void *key);
 
 extern tracing_map_cmp_fn_t tracing_map_cmp_num(int field_size,
 						int field_is_signed);
-extern int tracing_map_cmp_string(void *val_a, void *val_b);
-extern int tracing_map_cmp_none(void *val_a, void *val_b);
+extern int tracing_map_cmp_string(const void *val_a, const void *val_b);
+extern int tracing_map_cmp_none(const void *val_a, const void *val_b);
 
 extern void tracing_map_update_sum(struct tracing_map_elt *elt,
 				   unsigned int i, u64 n);
-- 
2.54.0

Re: [PATCH] trace: allocate fields with elt struct

[Masami Hiramatsu]

Hi Rosen,

Please check Sashiko's comment:

https://sashiko.dev/#/patchset/20260520223101.34710-1-rosenp%40gmail.com

One actual bug was found. I'll sent a fix.

Thanks,

On Wed, 20 May 2026 15:31:01 -0700
Rosen Penev <rosenp@gmail.com> wrote:

> Use a flexible array member to embed the fields array in the
> tracing_map_elt allocation, reducing the number of allocations
> per element.
> 
> Since the fields are now embedded in the struct, taking the address
> of a field through a const-qualified elt pointer yields a
> const-qualified pointer. Rather than adding casts, switch the
> comparison functions to take const void * parameters. These are
> all read-only operations.
> 
> Assisted-by: OpenCode:BigPickle
> Signed-off-by: Rosen Penev <rosenp@gmail.com>
> ---
>  kernel/trace/tracing_map.c | 41 ++++++++++++++++----------------------
>  kernel/trace/tracing_map.h |  8 ++++----
>  2 files changed, 21 insertions(+), 28 deletions(-)
> 
> diff --git a/kernel/trace/tracing_map.c b/kernel/trace/tracing_map.c
> index 1404bf752d99..97f7e3cde262 100644
> --- a/kernel/trace/tracing_map.c
> +++ b/kernel/trace/tracing_map.c
> @@ -125,32 +125,32 @@ u64 tracing_map_read_var_once(struct tracing_map_elt *elt, unsigned int i)
>  	return (u64)atomic64_read(&elt->vars[i]);
>  }
>  
> -int tracing_map_cmp_string(void *val_a, void *val_b)
> +int tracing_map_cmp_string(const void *val_a, const void *val_b)
>  {
> -	char *a = val_a;
> -	char *b = val_b;
> +	const char *a = val_a;
> +	const char *b = val_b;
>  
>  	return strcmp(a, b);
>  }
>  
> -int tracing_map_cmp_none(void *val_a, void *val_b)
> +int tracing_map_cmp_none(const void *val_a, const void *val_b)
>  {
>  	return 0;
>  }
>  
> -static int tracing_map_cmp_atomic64(void *val_a, void *val_b)
> +static int tracing_map_cmp_atomic64(const void *val_a, const void *val_b)
>  {
> -	u64 a = atomic64_read((atomic64_t *)val_a);
> -	u64 b = atomic64_read((atomic64_t *)val_b);
> +	u64 a = atomic64_read((const atomic64_t *)val_a);
> +	u64 b = atomic64_read((const atomic64_t *)val_b);
>  
>  	return (a > b) ? 1 : ((a < b) ? -1 : 0);
>  }
>  
>  #define DEFINE_TRACING_MAP_CMP_FN(type)					\
> -static int tracing_map_cmp_##type(void *val_a, void *val_b)		\
> +static int tracing_map_cmp_##type(const void *val_a, const void *val_b)	\
>  {									\
> -	type a = (type)(*(u64 *)val_a);					\
> -	type b = (type)(*(u64 *)val_b);					\
> +	type a = (type)(*(const u64 *)val_a);				\
> +	type b = (type)(*(const u64 *)val_b);				\
>  									\
>  	return (a > b) ? 1 : ((a < b) ? -1 : 0);			\
>  }
> @@ -385,7 +385,6 @@ static void tracing_map_elt_free(struct tracing_map_elt *elt)
>  
>  	if (elt->map->ops && elt->map->ops->elt_free)
>  		elt->map->ops->elt_free(elt);
> -	kfree(elt->fields);
>  	kfree(elt->vars);
>  	kfree(elt->var_set);
>  	kfree(elt->key);
> @@ -397,7 +396,7 @@ static struct tracing_map_elt *tracing_map_elt_alloc(struct tracing_map *map)
>  	struct tracing_map_elt *elt;
>  	int err = 0;
>  
> -	elt = kzalloc_obj(*elt);
> +	elt = kzalloc_flex(*elt, fields, map->n_fields);
>  	if (!elt)
>  		return ERR_PTR(-ENOMEM);
>  
> @@ -409,12 +408,6 @@ static struct tracing_map_elt *tracing_map_elt_alloc(struct tracing_map *map)
>  		goto free;
>  	}
>  
> -	elt->fields = kzalloc_objs(*elt->fields, map->n_fields);
> -	if (!elt->fields) {
> -		err = -ENOMEM;
> -		goto free;
> -	}
> -
>  	elt->vars = kzalloc_objs(*elt->vars, map->n_vars);
>  	if (!elt->vars) {
>  		err = -ENOMEM;
> @@ -848,10 +841,10 @@ static int cmp_entries_sum(const void *A, const void *B)
>  {
>  	const struct tracing_map_elt *elt_a, *elt_b;
>  	const struct tracing_map_sort_entry *a, *b;
> -	struct tracing_map_sort_key *sort_key;
> -	struct tracing_map_field *field;
> +	const struct tracing_map_sort_key *sort_key;
> +	const struct tracing_map_field *field;
>  	tracing_map_cmp_fn_t cmp_fn;
> -	void *val_a, *val_b;
> +	const void *val_a, *val_b;
>  	int ret = 0;
>  
>  	a = *(const struct tracing_map_sort_entry **)A;
> @@ -879,10 +872,10 @@ static int cmp_entries_key(const void *A, const void *B)
>  {
>  	const struct tracing_map_elt *elt_a, *elt_b;
>  	const struct tracing_map_sort_entry *a, *b;
> -	struct tracing_map_sort_key *sort_key;
> -	struct tracing_map_field *field;
> +	const struct tracing_map_sort_key *sort_key;
> +	const struct tracing_map_field *field;
>  	tracing_map_cmp_fn_t cmp_fn;
> -	void *val_a, *val_b;
> +	const void *val_a, *val_b;
>  	int ret = 0;
>  
>  	a = *(const struct tracing_map_sort_entry **)A;
> diff --git a/kernel/trace/tracing_map.h b/kernel/trace/tracing_map.h
> index 18a02959d77b..90a7fde5dd02 100644
> --- a/kernel/trace/tracing_map.h
> +++ b/kernel/trace/tracing_map.h
> @@ -13,7 +13,7 @@
>  #define TRACING_MAP_VARS_MAX		16
>  #define TRACING_MAP_SORT_KEYS_MAX	2
>  
> -typedef int (*tracing_map_cmp_fn_t) (void *val_a, void *val_b);
> +typedef int (*tracing_map_cmp_fn_t) (const void *val_a, const void *val_b);
>  
>  /*
>   * This is an overview of the tracing_map data structures and how they
> @@ -137,11 +137,11 @@ struct tracing_map_field {
>  
>  struct tracing_map_elt {
>  	struct tracing_map		*map;
> -	struct tracing_map_field	*fields;
>  	atomic64_t			*vars;
>  	bool				*var_set;
>  	void				*key;
>  	void				*private_data;
> +	struct tracing_map_field	fields[];
>  };
>  
>  struct tracing_map_entry {
> @@ -260,8 +260,8 @@ tracing_map_lookup(struct tracing_map *map, void *key);
>  
>  extern tracing_map_cmp_fn_t tracing_map_cmp_num(int field_size,
>  						int field_is_signed);
> -extern int tracing_map_cmp_string(void *val_a, void *val_b);
> -extern int tracing_map_cmp_none(void *val_a, void *val_b);
> +extern int tracing_map_cmp_string(const void *val_a, const void *val_b);
> +extern int tracing_map_cmp_none(const void *val_a, const void *val_b);
>  
>  extern void tracing_map_update_sum(struct tracing_map_elt *elt,
>  				   unsigned int i, u64 n);
> -- 
> 2.54.0
> 


-- 
Masami Hiramatsu (Google) <mhiramat@kernel.org>

[PATCH] tracing: Do not call map->ops->elt_free() if elt_alloc() is not succeeded

[Masami Hiramatsu (Google)]

From: Masami Hiramatsu (Google) <mhiramat@kernel.org>

In paths where tracing_map_elt_alloc() failed to allocate objects,
the map->ops->elt_alloc() call was never successful. In this case,
map->ops->elt_free() should not be called.

This bug was found by Sashiko.
Link: https://sashiko.dev/#/patchset/20260520223101.34710-1-rosenp%40gmail.com

Fixes: 2734b629525a ("tracing: Add per-element variable support to tracing_map")
Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
Cc: stable@vger.kernel.org
---
 kernel/trace/tracing_map.c |   17 +++++++++++++----
 1 file changed, 13 insertions(+), 4 deletions(-)

diff --git a/kernel/trace/tracing_map.c b/kernel/trace/tracing_map.c
index bf1a507695b6..0dd7927df22a 100644
--- a/kernel/trace/tracing_map.c
+++ b/kernel/trace/tracing_map.c
@@ -386,13 +386,11 @@ static void tracing_map_elt_init_fields(struct tracing_map_elt *elt)
 	}
 }
 
-static void tracing_map_elt_free(struct tracing_map_elt *elt)
+static void __tracing_map_elt_free(struct tracing_map_elt *elt)
 {
 	if (!elt)
 		return;
 
-	if (elt->map->ops && elt->map->ops->elt_free)
-		elt->map->ops->elt_free(elt);
 	kfree(elt->fields);
 	kfree(elt->vars);
 	kfree(elt->var_set);
@@ -400,6 +398,17 @@ static void tracing_map_elt_free(struct tracing_map_elt *elt)
 	kfree(elt);
 }
 
+static void tracing_map_elt_free(struct tracing_map_elt *elt)
+{
+	if (!elt)
+		return;
+
+	/* Only objects initialized with alloc_elt() should be passed to free_elt().*/
+	if (elt->map->ops && elt->map->ops->elt_free)
+		elt->map->ops->elt_free(elt);
+	__tracing_map_elt_free(elt);
+}
+
 static struct tracing_map_elt *tracing_map_elt_alloc(struct tracing_map *map)
 {
 	struct tracing_map_elt *elt;
@@ -444,7 +453,7 @@ static struct tracing_map_elt *tracing_map_elt_alloc(struct tracing_map *map)
 	}
 	return elt;
  free:
-	tracing_map_elt_free(elt);
+	__tracing_map_elt_free(elt);
 
 	return ERR_PTR(err);
 }