[PATCH v1 1/2] perf lock: Fix option value type in parse_max_stack

[PATCH v1 1/2] perf lock: Fix option value type in parse_max_stack

[Ian Rogers]

The value is a void* and the address of an int, max_stack_depth, is
set up in the perf lock options. The parse_max_stack function treats
the int* as a long*, make this more correct by declaring the value to
be an int*.

Fixes: 0a277b622670 ("perf lock contention: Check --max-stack option")
Signed-off-by: Ian Rogers <irogers@google.com>
---
 tools/perf/builtin-lock.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/perf/builtin-lock.c b/tools/perf/builtin-lock.c
index e8962c985d34..5585aeb97684 100644
--- a/tools/perf/builtin-lock.c
+++ b/tools/perf/builtin-lock.c
@@ -2250,7 +2250,7 @@ static int parse_map_entry(const struct option *opt, const char *str,
 static int parse_max_stack(const struct option *opt, const char *str,
 			   int unset __maybe_unused)
 {
-	unsigned long *len = (unsigned long *)opt->value;
+	int *len = opt->value;
 	long val;
 	char *endptr;
 
-- 
2.53.0.959.g497ff81fa9-goog

[PATCH v1 2/2] perf stat: Fix opt->value type for parse_cache_level

[Ian Rogers]

Commit f5803651b4a4 ("perf stat: Choose the most disaggregate command
line option") changed aggregation option handling for `perf stat` but
not `perf stat report` leading to parse_cache_level being passed a
struct in the `perf stat` case but erroneously an aggr_mode enum value
for `perf stat report`. Change the `perf stat report` aggregation
handling to use the same opt_aggr_mode as `perf stat`. Also, just pass
the boolean for consistency with other boolean argument handling.

Fixes: f5803651b4a4 ("perf stat: Choose the most disaggregate command line option")
Signed-off-by: Ian Rogers <irogers@google.com>
---
 tools/perf/builtin-stat.c | 41 ++++++++++++++++++++-------------------
 1 file changed, 21 insertions(+), 20 deletions(-)

diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
index 73c2ba7e3076..a00af7cfd50e 100644
--- a/tools/perf/builtin-stat.c
+++ b/tools/perf/builtin-stat.c
@@ -164,7 +164,7 @@ struct opt_aggr_mode {
 };
 
 /* Turn command line option into most generic aggregation mode setting. */
-static enum aggr_mode opt_aggr_mode_to_aggr_mode(struct opt_aggr_mode *opt_mode)
+static enum aggr_mode opt_aggr_mode_to_aggr_mode(const struct opt_aggr_mode *opt_mode)
 {
 	enum aggr_mode mode = AGGR_GLOBAL;
 
@@ -1219,8 +1219,8 @@ static int parse_cache_level(const struct option *opt,
 			     int unset __maybe_unused)
 {
 	int level;
-	struct opt_aggr_mode *opt_aggr_mode = (struct opt_aggr_mode *)opt->value;
-	u32 *aggr_level = (u32 *)opt->data;
+	bool *per_cache = opt->value;
+	u32 *aggr_level = opt->data;
 
 	/*
 	 * If no string is specified, aggregate based on the topology of
@@ -1258,7 +1258,7 @@ static int parse_cache_level(const struct option *opt,
 		return -EINVAL;
 	}
 out:
-	opt_aggr_mode->cache = true;
+	*per_cache = true;
 	*aggr_level = level;
 	return 0;
 }
@@ -2305,24 +2305,23 @@ static struct perf_stat perf_stat = {
 static int __cmd_report(int argc, const char **argv)
 {
 	struct perf_session *session;
+	struct opt_aggr_mode opt_mode = {};
 	const struct option options[] = {
 	OPT_STRING('i', "input", &input_name, "file", "input file name"),
-	OPT_SET_UINT(0, "per-socket", &perf_stat.aggr_mode,
-		     "aggregate counts per processor socket", AGGR_SOCKET),
-	OPT_SET_UINT(0, "per-die", &perf_stat.aggr_mode,
-		     "aggregate counts per processor die", AGGR_DIE),
-	OPT_SET_UINT(0, "per-cluster", &perf_stat.aggr_mode,
-		     "aggregate counts perf processor cluster", AGGR_CLUSTER),
-	OPT_CALLBACK_OPTARG(0, "per-cache", &perf_stat.aggr_mode, &perf_stat.aggr_level,
-			    "cache level",
-			    "aggregate count at this cache level (Default: LLC)",
+		OPT_BOOLEAN(0, "per-thread", &opt_mode.thread, "aggregate counts per thread"),
+	OPT_BOOLEAN(0, "per-socket", &opt_mode.socket,
+		    "aggregate counts per processor socket"),
+	OPT_BOOLEAN(0, "per-die", &opt_mode.die, "aggregate counts per processor die"),
+	OPT_BOOLEAN(0, "per-cluster", &opt_mode.cluster,
+		    "aggregate counts per processor cluster"),
+	OPT_CALLBACK_OPTARG(0, "per-cache", &opt_mode.cache, &stat_config.aggr_level,
+			    "cache level", "aggregate count at this cache level (Default: LLC)",
 			    parse_cache_level),
-	OPT_SET_UINT(0, "per-core", &perf_stat.aggr_mode,
-		     "aggregate counts per physical processor core", AGGR_CORE),
-	OPT_SET_UINT(0, "per-node", &perf_stat.aggr_mode,
-		     "aggregate counts per numa node", AGGR_NODE),
-	OPT_SET_UINT('A', "no-aggr", &perf_stat.aggr_mode,
-		     "disable CPU count aggregation", AGGR_NONE),
+	OPT_BOOLEAN(0, "per-core", &opt_mode.core,
+		    "aggregate counts per physical processor core"),
+	OPT_BOOLEAN(0, "per-node", &opt_mode.node, "aggregate counts per numa node"),
+	OPT_BOOLEAN('A', "no-aggr", &opt_mode.no_aggr,
+		    "disable aggregation across CPUs or PMUs"),
 	OPT_END()
 	};
 	struct stat st;
@@ -2330,6 +2329,8 @@ static int __cmd_report(int argc, const char **argv)
 
 	argc = parse_options(argc, argv, options, stat_report_usage, 0);
 
+	perf_stat.aggr_mode = opt_aggr_mode_to_aggr_mode(&opt_mode);
+
 	if (!input_name || !strlen(input_name)) {
 		if (!fstat(STDIN_FILENO, &st) && S_ISFIFO(st.st_mode))
 			input_name = "-";
@@ -2506,7 +2507,7 @@ int cmd_stat(int argc, const char **argv)
 		OPT_BOOLEAN(0, "per-die", &opt_mode.die, "aggregate counts per processor die"),
 		OPT_BOOLEAN(0, "per-cluster", &opt_mode.cluster,
 			"aggregate counts per processor cluster"),
-		OPT_CALLBACK_OPTARG(0, "per-cache", &opt_mode, &stat_config.aggr_level,
+		OPT_CALLBACK_OPTARG(0, "per-cache", &opt_mode.cache, &stat_config.aggr_level,
 				"cache level", "aggregate count at this cache level (Default: LLC)",
 				parse_cache_level),
 		OPT_BOOLEAN(0, "per-core", &opt_mode.core,
-- 
2.53.0.959.g497ff81fa9-goog

[PATCH v2 1/2] perf lock: Fix option value type in parse_max_stack

[Ian Rogers]

The value is a void* and the address of an int, max_stack_depth, is
set up in the perf lock options. The parse_max_stack function treats
the int* as a long*, make this more correct by declaring the value to
be an int*.

Fixes: 0a277b622670 ("perf lock contention: Check --max-stack option")
Signed-off-by: Ian Rogers <irogers@google.com>
---
 tools/perf/builtin-lock.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/perf/builtin-lock.c b/tools/perf/builtin-lock.c
index e8962c985d34..5585aeb97684 100644
--- a/tools/perf/builtin-lock.c
+++ b/tools/perf/builtin-lock.c
@@ -2250,7 +2250,7 @@ static int parse_map_entry(const struct option *opt, const char *str,
 static int parse_max_stack(const struct option *opt, const char *str,
 			   int unset __maybe_unused)
 {
-	unsigned long *len = (unsigned long *)opt->value;
+	int *len = opt->value;
 	long val;
 	char *endptr;
 
-- 
2.53.0.959.g497ff81fa9-goog

[PATCH v2 2/2] perf stat: Fix opt->value type for parse_cache_level

[Ian Rogers]

Commit f5803651b4a4 ("perf stat: Choose the most disaggregate command
line option") changed aggregation option handling for `perf stat` but
not `perf stat report` leading to parse_cache_level being passed a
struct in the `perf stat` case but erroneously an aggr_mode enum value
for `perf stat report`. Change the `perf stat report` aggregation
handling to use the same opt_aggr_mode as `perf stat`. Also, just pass
the boolean for consistency with other boolean argument handling.

Fixes: f5803651b4a4 ("perf stat: Choose the most disaggregate command line option")
Signed-off-by: Ian Rogers <irogers@google.com>
---
v2: Add in fixes suggested by Sashiko.
---
 tools/perf/builtin-stat.c | 43 +++++++++++++++++++++------------------
 1 file changed, 23 insertions(+), 20 deletions(-)

diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
index 73c2ba7e3076..02c1c02d4b27 100644
--- a/tools/perf/builtin-stat.c
+++ b/tools/perf/builtin-stat.c
@@ -164,7 +164,7 @@ struct opt_aggr_mode {
 };
 
 /* Turn command line option into most generic aggregation mode setting. */
-static enum aggr_mode opt_aggr_mode_to_aggr_mode(struct opt_aggr_mode *opt_mode)
+static enum aggr_mode opt_aggr_mode_to_aggr_mode(const struct opt_aggr_mode *opt_mode)
 {
 	enum aggr_mode mode = AGGR_GLOBAL;
 
@@ -1219,8 +1219,8 @@ static int parse_cache_level(const struct option *opt,
 			     int unset __maybe_unused)
 {
 	int level;
-	struct opt_aggr_mode *opt_aggr_mode = (struct opt_aggr_mode *)opt->value;
-	u32 *aggr_level = (u32 *)opt->data;
+	bool *per_cache = opt->value;
+	u32 *aggr_level = opt->data;
 
 	/*
 	 * If no string is specified, aggregate based on the topology of
@@ -1258,7 +1258,7 @@ static int parse_cache_level(const struct option *opt,
 		return -EINVAL;
 	}
 out:
-	opt_aggr_mode->cache = true;
+	*per_cache = true;
 	*aggr_level = level;
 	return 0;
 }
@@ -2305,24 +2305,23 @@ static struct perf_stat perf_stat = {
 static int __cmd_report(int argc, const char **argv)
 {
 	struct perf_session *session;
+	struct opt_aggr_mode opt_mode = {};
 	const struct option options[] = {
 	OPT_STRING('i', "input", &input_name, "file", "input file name"),
-	OPT_SET_UINT(0, "per-socket", &perf_stat.aggr_mode,
-		     "aggregate counts per processor socket", AGGR_SOCKET),
-	OPT_SET_UINT(0, "per-die", &perf_stat.aggr_mode,
-		     "aggregate counts per processor die", AGGR_DIE),
-	OPT_SET_UINT(0, "per-cluster", &perf_stat.aggr_mode,
-		     "aggregate counts perf processor cluster", AGGR_CLUSTER),
-	OPT_CALLBACK_OPTARG(0, "per-cache", &perf_stat.aggr_mode, &perf_stat.aggr_level,
-			    "cache level",
-			    "aggregate count at this cache level (Default: LLC)",
+		OPT_BOOLEAN(0, "per-thread", &opt_mode.thread, "aggregate counts per thread"),
+	OPT_BOOLEAN(0, "per-socket", &opt_mode.socket,
+		    "aggregate counts per processor socket"),
+	OPT_BOOLEAN(0, "per-die", &opt_mode.die, "aggregate counts per processor die"),
+	OPT_BOOLEAN(0, "per-cluster", &opt_mode.cluster,
+		    "aggregate counts per processor cluster"),
+	OPT_CALLBACK_OPTARG(0, "per-cache", &opt_mode.cache, &perf_stat.aggr_level,
+			    "cache level", "aggregate count at this cache level (Default: LLC)",
 			    parse_cache_level),
-	OPT_SET_UINT(0, "per-core", &perf_stat.aggr_mode,
-		     "aggregate counts per physical processor core", AGGR_CORE),
-	OPT_SET_UINT(0, "per-node", &perf_stat.aggr_mode,
-		     "aggregate counts per numa node", AGGR_NODE),
-	OPT_SET_UINT('A', "no-aggr", &perf_stat.aggr_mode,
-		     "disable CPU count aggregation", AGGR_NONE),
+	OPT_BOOLEAN(0, "per-core", &opt_mode.core,
+		    "aggregate counts per physical processor core"),
+	OPT_BOOLEAN(0, "per-node", &opt_mode.node, "aggregate counts per numa node"),
+	OPT_BOOLEAN('A', "no-aggr", &opt_mode.no_aggr,
+		    "disable aggregation across CPUs or PMUs"),
 	OPT_END()
 	};
 	struct stat st;
@@ -2330,6 +2329,10 @@ static int __cmd_report(int argc, const char **argv)
 
 	argc = parse_options(argc, argv, options, stat_report_usage, 0);
 
+	perf_stat.aggr_mode = opt_aggr_mode_to_aggr_mode(&opt_mode);
+	if (perf_stat.aggr_mode == AGGR_GLOBAL)
+		perf_stat.aggr_mode = AGGR_UNSET; /* No option found so leave unset. */
+
 	if (!input_name || !strlen(input_name)) {
 		if (!fstat(STDIN_FILENO, &st) && S_ISFIFO(st.st_mode))
 			input_name = "-";
@@ -2506,7 +2509,7 @@ int cmd_stat(int argc, const char **argv)
 		OPT_BOOLEAN(0, "per-die", &opt_mode.die, "aggregate counts per processor die"),
 		OPT_BOOLEAN(0, "per-cluster", &opt_mode.cluster,
 			"aggregate counts per processor cluster"),
-		OPT_CALLBACK_OPTARG(0, "per-cache", &opt_mode, &stat_config.aggr_level,
+		OPT_CALLBACK_OPTARG(0, "per-cache", &opt_mode.cache, &stat_config.aggr_level,
 				"cache level", "aggregate count at this cache level (Default: LLC)",
 				parse_cache_level),
 		OPT_BOOLEAN(0, "per-core", &opt_mode.core,
-- 
2.53.0.959.g497ff81fa9-goog

Re: [PATCH v2 2/2] perf stat: Fix opt->value type for parse_cache_level

[Namhyung Kim]

On Thu, Mar 19, 2026 at 04:33:49PM -0700, Ian Rogers wrote:
> Commit f5803651b4a4 ("perf stat: Choose the most disaggregate command
> line option") changed aggregation option handling for `perf stat` but
> not `perf stat report` leading to parse_cache_level being passed a
> struct in the `perf stat` case but erroneously an aggr_mode enum value
> for `perf stat report`. Change the `perf stat report` aggregation
> handling to use the same opt_aggr_mode as `perf stat`. Also, just pass
> the boolean for consistency with other boolean argument handling.
> 
> Fixes: f5803651b4a4 ("perf stat: Choose the most disaggregate command line option")
> Signed-off-by: Ian Rogers <irogers@google.com>
> ---
> v2: Add in fixes suggested by Sashiko.
> ---
>  tools/perf/builtin-stat.c | 43 +++++++++++++++++++++------------------
>  1 file changed, 23 insertions(+), 20 deletions(-)
> 
> diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
> index 73c2ba7e3076..02c1c02d4b27 100644
> --- a/tools/perf/builtin-stat.c
> +++ b/tools/perf/builtin-stat.c
> @@ -164,7 +164,7 @@ struct opt_aggr_mode {
>  };
>  
>  /* Turn command line option into most generic aggregation mode setting. */
> -static enum aggr_mode opt_aggr_mode_to_aggr_mode(struct opt_aggr_mode *opt_mode)
> +static enum aggr_mode opt_aggr_mode_to_aggr_mode(const struct opt_aggr_mode *opt_mode)
>  {
>  	enum aggr_mode mode = AGGR_GLOBAL;
>  
> @@ -1219,8 +1219,8 @@ static int parse_cache_level(const struct option *opt,
>  			     int unset __maybe_unused)
>  {
>  	int level;
> -	struct opt_aggr_mode *opt_aggr_mode = (struct opt_aggr_mode *)opt->value;
> -	u32 *aggr_level = (u32 *)opt->data;
> +	bool *per_cache = opt->value;
> +	u32 *aggr_level = opt->data;
>  
>  	/*
>  	 * If no string is specified, aggregate based on the topology of
> @@ -1258,7 +1258,7 @@ static int parse_cache_level(const struct option *opt,
>  		return -EINVAL;
>  	}
>  out:
> -	opt_aggr_mode->cache = true;
> +	*per_cache = true;
>  	*aggr_level = level;
>  	return 0;
>  }
> @@ -2305,24 +2305,23 @@ static struct perf_stat perf_stat = {
>  static int __cmd_report(int argc, const char **argv)
>  {
>  	struct perf_session *session;
> +	struct opt_aggr_mode opt_mode = {};
>  	const struct option options[] = {
>  	OPT_STRING('i', "input", &input_name, "file", "input file name"),
> -	OPT_SET_UINT(0, "per-socket", &perf_stat.aggr_mode,
> -		     "aggregate counts per processor socket", AGGR_SOCKET),
> -	OPT_SET_UINT(0, "per-die", &perf_stat.aggr_mode,
> -		     "aggregate counts per processor die", AGGR_DIE),
> -	OPT_SET_UINT(0, "per-cluster", &perf_stat.aggr_mode,
> -		     "aggregate counts perf processor cluster", AGGR_CLUSTER),
> -	OPT_CALLBACK_OPTARG(0, "per-cache", &perf_stat.aggr_mode, &perf_stat.aggr_level,
> -			    "cache level",
> -			    "aggregate count at this cache level (Default: LLC)",
> +		OPT_BOOLEAN(0, "per-thread", &opt_mode.thread, "aggregate counts per thread"),

A wrong indentation.  I'll fix this time.


> +	OPT_BOOLEAN(0, "per-socket", &opt_mode.socket,
> +		    "aggregate counts per processor socket"),
> +	OPT_BOOLEAN(0, "per-die", &opt_mode.die, "aggregate counts per processor die"),
> +	OPT_BOOLEAN(0, "per-cluster", &opt_mode.cluster,
> +		    "aggregate counts per processor cluster"),
> +	OPT_CALLBACK_OPTARG(0, "per-cache", &opt_mode.cache, &perf_stat.aggr_level,
> +			    "cache level", "aggregate count at this cache level (Default: LLC)",
>  			    parse_cache_level),
> -	OPT_SET_UINT(0, "per-core", &perf_stat.aggr_mode,
> -		     "aggregate counts per physical processor core", AGGR_CORE),
> -	OPT_SET_UINT(0, "per-node", &perf_stat.aggr_mode,
> -		     "aggregate counts per numa node", AGGR_NODE),
> -	OPT_SET_UINT('A', "no-aggr", &perf_stat.aggr_mode,
> -		     "disable CPU count aggregation", AGGR_NONE),
> +	OPT_BOOLEAN(0, "per-core", &opt_mode.core,
> +		    "aggregate counts per physical processor core"),
> +	OPT_BOOLEAN(0, "per-node", &opt_mode.node, "aggregate counts per numa node"),
> +	OPT_BOOLEAN('A', "no-aggr", &opt_mode.no_aggr,
> +		    "disable aggregation across CPUs or PMUs"),
>  	OPT_END()
>  	};
>  	struct stat st;
> @@ -2330,6 +2329,10 @@ static int __cmd_report(int argc, const char **argv)
>  
>  	argc = parse_options(argc, argv, options, stat_report_usage, 0);
>  
> +	perf_stat.aggr_mode = opt_aggr_mode_to_aggr_mode(&opt_mode);
> +	if (perf_stat.aggr_mode == AGGR_GLOBAL)
> +		perf_stat.aggr_mode = AGGR_UNSET; /* No option found so leave unset. */

I think it's more intuitive if the function returns AGGR_UNSET if none
of the field was set.  Then we need to change record and stat parts to
set GLOBAL when it's UNSET.  But this could be a separate change.

Thanks,
Namhyung


> +
>  	if (!input_name || !strlen(input_name)) {
>  		if (!fstat(STDIN_FILENO, &st) && S_ISFIFO(st.st_mode))
>  			input_name = "-";
> @@ -2506,7 +2509,7 @@ int cmd_stat(int argc, const char **argv)
>  		OPT_BOOLEAN(0, "per-die", &opt_mode.die, "aggregate counts per processor die"),
>  		OPT_BOOLEAN(0, "per-cluster", &opt_mode.cluster,
>  			"aggregate counts per processor cluster"),
> -		OPT_CALLBACK_OPTARG(0, "per-cache", &opt_mode, &stat_config.aggr_level,
> +		OPT_CALLBACK_OPTARG(0, "per-cache", &opt_mode.cache, &stat_config.aggr_level,
>  				"cache level", "aggregate count at this cache level (Default: LLC)",
>  				parse_cache_level),
>  		OPT_BOOLEAN(0, "per-core", &opt_mode.core,
> -- 
> 2.53.0.959.g497ff81fa9-goog
> 

Re: [PATCH v2 1/2] perf lock: Fix option value type in parse_max_stack

[Namhyung Kim]

On Thu, 19 Mar 2026 16:33:48 -0700, Ian Rogers wrote:
> The value is a void* and the address of an int, max_stack_depth, is
> set up in the perf lock options. The parse_max_stack function treats
> the int* as a long*, make this more correct by declaring the value to
> be an int*.
> 
> 
Applied to perf-tools-next, thanks!

Best regards,
Namhyung