All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH V5 1/2] powerpc tools perf: Initialize error code in auxtrace_record_init function
@ 2026-05-04 15:13 Athira Rajeev
  2026-05-04 15:13 ` [PATCH V5 2/2] tools/perf: Add kernel-doc comment to auxtrace_record__init() function Athira Rajeev
  2026-05-07  6:13 ` [PATCH V5 1/2] powerpc tools perf: Initialize error code in auxtrace_record_init function Namhyung Kim
  0 siblings, 2 replies; 6+ messages in thread
From: Athira Rajeev @ 2026-05-04 15:13 UTC (permalink / raw)
  To: acme, jolsa, adrian.hunter, mpetlan, tmricht, maddy, irogers,
	namhyung
  Cc: linux-perf-users, linuxppc-dev, atrajeev, hbathini, Tejas.Manhas1,
	Tanushree.Shah, shivani

perf trace record fails some cases in powerpc

 # ./perf test "perf trace record and replay"
 128: perf trace record and replay                                    : FAILED!

 # ./perf trace record sleep 1
 # echo $?
   32

This is happening because of non-zero err value from
auxtrace_record__init() function.

 static int record__auxtrace_init(struct record *rec)
 {
        int err;

        if ((rec->opts.auxtrace_snapshot_opts || rec->opts.auxtrace_sample_opts)
            && record__threads_enabled(rec)) {
                pr_err("AUX area tracing options are not available in parallel streaming mode.\n");
                return -EINVAL;
        }

        if (!rec->itr) {
                rec->itr = auxtrace_record__init(rec->evlist, &err);
                if (err)
                        return err;
        }

Here "int err" is not initialised. The code expects "err" to be set
from auxtrace_record__init() function.

Update auxtrace_record__init() in arch/powerpc/util/auxtrace.c to clear
err value in the beginning.

- Clear err value in beginning of function. Any fail later will
set appropriate return code to err.
- Even if we haven't found any event for auxtrace, perf record
should continue for other events. NULL return
will indicate that there is no auxtrace record initialized.
- Not having "err" set here will affect monitoring of other events
also because perf record will fail seeing random value in err.

Set err to -EINVAL before invoking auxtrace_record__init() in
builtin-record.c

With the fix,

 # ./perf trace record sleep 1
 [ perf record: Woken up 2 times to write data ]
 [ perf record: Captured and wrote 0.033 MB perf.data (228 samples) ]

Fixes: 1dbfaf94cf66 ("perf powerpc: Add basic CONFIG_AUXTRACE support for VPA pmu on powerpc")
Reviewed-by: Adrian Hunter <adrian.hunter@intel.com>
Signed-off-by: Athira Rajeev <atrajeev@linux.ibm.com>
---
Changelog:
v4:
Added Reviewed-by from Adrian

v1 -> v2
Addressed review comment from Adrian:
- Set err to -EINVAL before invoking auxtrace_record__init() in
  builtin-record.c
- Added kernel-doc to auxtrace_record__init() in tools/perf/util/auxtrace.c
Addressed review comment from Namhyung:
- Added fixes tag

 tools/perf/arch/powerpc/util/auxtrace.c | 6 ++++++
 tools/perf/builtin-record.c             | 1 +
 2 files changed, 7 insertions(+)

diff --git a/tools/perf/arch/powerpc/util/auxtrace.c b/tools/perf/arch/powerpc/util/auxtrace.c
index e39deff6c857..4600a1661b4f 100644
--- a/tools/perf/arch/powerpc/util/auxtrace.c
+++ b/tools/perf/arch/powerpc/util/auxtrace.c
@@ -71,6 +71,12 @@ struct auxtrace_record *auxtrace_record__init(struct evlist *evlist,
 	struct evsel *pos;
 	int found = 0;
 
+	/*
+	 * Set err value to zero here. Any fail later
+	 * will set appropriate return code to err.
+	 */
+	*err = 0;
+
 	evlist__for_each_entry(evlist, pos) {
 		if (strstarts(pos->name, "vpa_dtl")) {
 			found = 1;
diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index 4a5eba498c02..708825747af5 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -865,6 +865,7 @@ static int record__auxtrace_init(struct record *rec)
 	}
 
 	if (!rec->itr) {
+		err = -EINVAL;
 		rec->itr = auxtrace_record__init(rec->evlist, &err);
 		if (err)
 			return err;
-- 
2.47.3


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH V5 2/2] tools/perf: Add kernel-doc comment to auxtrace_record__init() function
  2026-05-04 15:13 [PATCH V5 1/2] powerpc tools perf: Initialize error code in auxtrace_record_init function Athira Rajeev
@ 2026-05-04 15:13 ` Athira Rajeev
  2026-05-05 13:29   ` Adrian Hunter
  2026-05-07  6:13 ` [PATCH V5 1/2] powerpc tools perf: Initialize error code in auxtrace_record_init function Namhyung Kim
  1 sibling, 1 reply; 6+ messages in thread
From: Athira Rajeev @ 2026-05-04 15:13 UTC (permalink / raw)
  To: acme, jolsa, adrian.hunter, mpetlan, tmricht, maddy, irogers,
	namhyung
  Cc: linux-perf-users, linuxppc-dev, atrajeev, hbathini, Tejas.Manhas1,
	Tanushree.Shah, shivani

Add documentation comment describing the parameters
and return code for auxtrace_record__init() in util/auxtrace.c

Signed-off-by: Athira Rajeev <atrajeev@linux.ibm.com>
---
Changelog:
v4 > v5:
Addressed review comment from Namhyung:
- Used original placement for __maybe_unused as
"struct evlist *evlist __maybe_unused"
- Added return code expectation on success case

v3 -> v4:
Addressed review comment from Sashiko:
Update return value expectation for fail as
non zero return err code.

v2 -> v3:
Addressed review comment from Sashiko:
Update return value expectation for success and fail
correctly.

 tools/perf/util/auxtrace.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/tools/perf/util/auxtrace.c b/tools/perf/util/auxtrace.c
index a224687ffbc1..a9f007d47c0b 100644
--- a/tools/perf/util/auxtrace.c
+++ b/tools/perf/util/auxtrace.c
@@ -896,6 +896,21 @@ int auxtrace_parse_aux_action(struct evlist *evlist)
 	return 0;
 }
 
+/**
+ * auxtrace_record__init - Initialize an AUX area tracing record.
+ * @evlist: The list of events to check for AUX area tracing event.
+ * @err: Pointer to an integer to store return code.
+ *
+ * This function looks through the @evlist to determine which AUX area
+ * tracing hardware is being used and initializes the auxtrace_record
+ * structure.
+ *
+ * Return:
+ * a) A pointer to the struct auxtrace_record with @err = 0 on success.
+ * b) NULL with @err = 0 if no AUX area tracing event is found/supported
+ *    (not considered an error).
+ * c) NULL with non-zero @err on actual auxtrace_record__init failure.
+ */
 struct auxtrace_record *__weak
 auxtrace_record__init(struct evlist *evlist __maybe_unused, int *err)
 {
-- 
2.47.3


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH V5 2/2] tools/perf: Add kernel-doc comment to auxtrace_record__init() function
  2026-05-04 15:13 ` [PATCH V5 2/2] tools/perf: Add kernel-doc comment to auxtrace_record__init() function Athira Rajeev
@ 2026-05-05 13:29   ` Adrian Hunter
  0 siblings, 0 replies; 6+ messages in thread
From: Adrian Hunter @ 2026-05-05 13:29 UTC (permalink / raw)
  To: Athira Rajeev, acme, jolsa, mpetlan, tmricht, maddy, irogers,
	namhyung
  Cc: linux-perf-users, linuxppc-dev, hbathini, Tejas.Manhas1,
	Tanushree.Shah, shivani

On 04/05/2026 18:13, Athira Rajeev wrote:
> Add documentation comment describing the parameters
> and return code for auxtrace_record__init() in util/auxtrace.c
> 
> Signed-off-by: Athira Rajeev <atrajeev@linux.ibm.com>

Reviewed-by: Adrian Hunter <adrian.hunter@intel.com>

> ---
> Changelog:
> v4 > v5:
> Addressed review comment from Namhyung:
> - Used original placement for __maybe_unused as
> "struct evlist *evlist __maybe_unused"
> - Added return code expectation on success case
> 
> v3 -> v4:
> Addressed review comment from Sashiko:
> Update return value expectation for fail as
> non zero return err code.
> 
> v2 -> v3:
> Addressed review comment from Sashiko:
> Update return value expectation for success and fail
> correctly.
> 
>  tools/perf/util/auxtrace.c | 15 +++++++++++++++
>  1 file changed, 15 insertions(+)
> 
> diff --git a/tools/perf/util/auxtrace.c b/tools/perf/util/auxtrace.c
> index a224687ffbc1..a9f007d47c0b 100644
> --- a/tools/perf/util/auxtrace.c
> +++ b/tools/perf/util/auxtrace.c
> @@ -896,6 +896,21 @@ int auxtrace_parse_aux_action(struct evlist *evlist)
>  	return 0;
>  }
>  
> +/**
> + * auxtrace_record__init - Initialize an AUX area tracing record.
> + * @evlist: The list of events to check for AUX area tracing event.
> + * @err: Pointer to an integer to store return code.
> + *
> + * This function looks through the @evlist to determine which AUX area
> + * tracing hardware is being used and initializes the auxtrace_record
> + * structure.
> + *
> + * Return:
> + * a) A pointer to the struct auxtrace_record with @err = 0 on success.
> + * b) NULL with @err = 0 if no AUX area tracing event is found/supported
> + *    (not considered an error).
> + * c) NULL with non-zero @err on actual auxtrace_record__init failure.
> + */
>  struct auxtrace_record *__weak
>  auxtrace_record__init(struct evlist *evlist __maybe_unused, int *err)
>  {


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH V5 1/2] powerpc tools perf: Initialize error code in auxtrace_record_init function
  2026-05-04 15:13 [PATCH V5 1/2] powerpc tools perf: Initialize error code in auxtrace_record_init function Athira Rajeev
  2026-05-04 15:13 ` [PATCH V5 2/2] tools/perf: Add kernel-doc comment to auxtrace_record__init() function Athira Rajeev
@ 2026-05-07  6:13 ` Namhyung Kim
  2026-05-21  7:31   ` Athira Rajeev
  2026-05-27 11:10   ` Arnaldo Carvalho de Melo
  1 sibling, 2 replies; 6+ messages in thread
From: Namhyung Kim @ 2026-05-07  6:13 UTC (permalink / raw)
  To: Athira Rajeev
  Cc: acme, jolsa, adrian.hunter, mpetlan, tmricht, maddy, irogers,
	linux-perf-users, linuxppc-dev, hbathini, Tejas.Manhas1,
	Tanushree.Shah, shivani

On Mon, May 04, 2026 at 08:43:20PM +0530, Athira Rajeev wrote:
> perf trace record fails some cases in powerpc
> 
>  # ./perf test "perf trace record and replay"
>  128: perf trace record and replay                                    : FAILED!
> 
>  # ./perf trace record sleep 1
>  # echo $?
>    32
> 
> This is happening because of non-zero err value from
> auxtrace_record__init() function.
> 
>  static int record__auxtrace_init(struct record *rec)
>  {
>         int err;
> 
>         if ((rec->opts.auxtrace_snapshot_opts || rec->opts.auxtrace_sample_opts)
>             && record__threads_enabled(rec)) {
>                 pr_err("AUX area tracing options are not available in parallel streaming mode.\n");
>                 return -EINVAL;
>         }
> 
>         if (!rec->itr) {
>                 rec->itr = auxtrace_record__init(rec->evlist, &err);
>                 if (err)
>                         return err;
>         }
> 
> Here "int err" is not initialised. The code expects "err" to be set
> from auxtrace_record__init() function.
> 
> Update auxtrace_record__init() in arch/powerpc/util/auxtrace.c to clear
> err value in the beginning.
> 
> - Clear err value in beginning of function. Any fail later will
> set appropriate return code to err.
> - Even if we haven't found any event for auxtrace, perf record
> should continue for other events. NULL return
> will indicate that there is no auxtrace record initialized.
> - Not having "err" set here will affect monitoring of other events
> also because perf record will fail seeing random value in err.
> 
> Set err to -EINVAL before invoking auxtrace_record__init() in
> builtin-record.c
> 
> With the fix,
> 
>  # ./perf trace record sleep 1
>  [ perf record: Woken up 2 times to write data ]
>  [ perf record: Captured and wrote 0.033 MB perf.data (228 samples) ]
> 
> Fixes: 1dbfaf94cf66 ("perf powerpc: Add basic CONFIG_AUXTRACE support for VPA pmu on powerpc")
> Reviewed-by: Adrian Hunter <adrian.hunter@intel.com>
> Signed-off-by: Athira Rajeev <atrajeev@linux.ibm.com>

For both patches,

Acked-by: Namhyung Kim <namhyung@kernel.org>

Thanks,
Namhyung

> ---
> Changelog:
> v4:
> Added Reviewed-by from Adrian
> 
> v1 -> v2
> Addressed review comment from Adrian:
> - Set err to -EINVAL before invoking auxtrace_record__init() in
>   builtin-record.c
> - Added kernel-doc to auxtrace_record__init() in tools/perf/util/auxtrace.c
> Addressed review comment from Namhyung:
> - Added fixes tag
> 
>  tools/perf/arch/powerpc/util/auxtrace.c | 6 ++++++
>  tools/perf/builtin-record.c             | 1 +
>  2 files changed, 7 insertions(+)
> 
> diff --git a/tools/perf/arch/powerpc/util/auxtrace.c b/tools/perf/arch/powerpc/util/auxtrace.c
> index e39deff6c857..4600a1661b4f 100644
> --- a/tools/perf/arch/powerpc/util/auxtrace.c
> +++ b/tools/perf/arch/powerpc/util/auxtrace.c
> @@ -71,6 +71,12 @@ struct auxtrace_record *auxtrace_record__init(struct evlist *evlist,
>  	struct evsel *pos;
>  	int found = 0;
>  
> +	/*
> +	 * Set err value to zero here. Any fail later
> +	 * will set appropriate return code to err.
> +	 */
> +	*err = 0;
> +
>  	evlist__for_each_entry(evlist, pos) {
>  		if (strstarts(pos->name, "vpa_dtl")) {
>  			found = 1;
> diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
> index 4a5eba498c02..708825747af5 100644
> --- a/tools/perf/builtin-record.c
> +++ b/tools/perf/builtin-record.c
> @@ -865,6 +865,7 @@ static int record__auxtrace_init(struct record *rec)
>  	}
>  
>  	if (!rec->itr) {
> +		err = -EINVAL;
>  		rec->itr = auxtrace_record__init(rec->evlist, &err);
>  		if (err)
>  			return err;
> -- 
> 2.47.3
> 

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH V5 1/2] powerpc tools perf: Initialize error code in auxtrace_record_init function
  2026-05-07  6:13 ` [PATCH V5 1/2] powerpc tools perf: Initialize error code in auxtrace_record_init function Namhyung Kim
@ 2026-05-21  7:31   ` Athira Rajeev
  2026-05-27 11:10   ` Arnaldo Carvalho de Melo
  1 sibling, 0 replies; 6+ messages in thread
From: Athira Rajeev @ 2026-05-21  7:31 UTC (permalink / raw)
  To: Namhyung Kim, Arnaldo Carvalho de Melo, Ian Rogers, Adrian Hunter
  Cc: acme, jolsa, adrian.hunter, mpetlan, tmricht, maddy, irogers,
	linux-perf-users, linuxppc-dev, hbathini, Tejas.Manhas1,
	Tanushree.Shah, shivani



> On 7 May 2026, at 11:43 AM, Namhyung Kim <namhyung@kernel.org> wrote:
> 
> On Mon, May 04, 2026 at 08:43:20PM +0530, Athira Rajeev wrote:
>> perf trace record fails some cases in powerpc
>> 
>> # ./perf test "perf trace record and replay"
>> 128: perf trace record and replay                                    : FAILED!
>> 
>> # ./perf trace record sleep 1
>> # echo $?
>>   32
>> 
>> This is happening because of non-zero err value from
>> auxtrace_record__init() function.
>> 
>> static int record__auxtrace_init(struct record *rec)
>> {
>>        int err;
>> 
>>        if ((rec->opts.auxtrace_snapshot_opts || rec->opts.auxtrace_sample_opts)
>>            && record__threads_enabled(rec)) {
>>                pr_err("AUX area tracing options are not available in parallel streaming mode.\n");
>>                return -EINVAL;
>>        }
>> 
>>        if (!rec->itr) {
>>                rec->itr = auxtrace_record__init(rec->evlist, &err);
>>                if (err)
>>                        return err;
>>        }
>> 
>> Here "int err" is not initialised. The code expects "err" to be set
>> from auxtrace_record__init() function.
>> 
>> Update auxtrace_record__init() in arch/powerpc/util/auxtrace.c to clear
>> err value in the beginning.
>> 
>> - Clear err value in beginning of function. Any fail later will
>> set appropriate return code to err.
>> - Even if we haven't found any event for auxtrace, perf record
>> should continue for other events. NULL return
>> will indicate that there is no auxtrace record initialized.
>> - Not having "err" set here will affect monitoring of other events
>> also because perf record will fail seeing random value in err.
>> 
>> Set err to -EINVAL before invoking auxtrace_record__init() in
>> builtin-record.c
>> 
>> With the fix,
>> 
>> # ./perf trace record sleep 1
>> [ perf record: Woken up 2 times to write data ]
>> [ perf record: Captured and wrote 0.033 MB perf.data (228 samples) ]
>> 
>> Fixes: 1dbfaf94cf66 ("perf powerpc: Add basic CONFIG_AUXTRACE support for VPA pmu on powerpc")
>> Reviewed-by: Adrian Hunter <adrian.hunter@intel.com>
>> Signed-off-by: Athira Rajeev <atrajeev@linux.ibm.com>
> 
> For both patches,
> 
> Acked-by: Namhyung Kim <namhyung@kernel.org>
> 
> Thanks,
> Namhyung
> 

Hi,

Can we please have this pulled in, if the patches looks fine ?

Thanks
Athira
>> ---
>> Changelog:
>> v4:
>> Added Reviewed-by from Adrian
>> 
>> v1 -> v2
>> Addressed review comment from Adrian:
>> - Set err to -EINVAL before invoking auxtrace_record__init() in
>>  builtin-record.c
>> - Added kernel-doc to auxtrace_record__init() in tools/perf/util/auxtrace.c
>> Addressed review comment from Namhyung:
>> - Added fixes tag
>> 
>> tools/perf/arch/powerpc/util/auxtrace.c | 6 ++++++
>> tools/perf/builtin-record.c             | 1 +
>> 2 files changed, 7 insertions(+)
>> 
>> diff --git a/tools/perf/arch/powerpc/util/auxtrace.c b/tools/perf/arch/powerpc/util/auxtrace.c
>> index e39deff6c857..4600a1661b4f 100644
>> --- a/tools/perf/arch/powerpc/util/auxtrace.c
>> +++ b/tools/perf/arch/powerpc/util/auxtrace.c
>> @@ -71,6 +71,12 @@ struct auxtrace_record *auxtrace_record__init(struct evlist *evlist,
>> struct evsel *pos;
>> int found = 0;
>> 
>> + /*
>> +  * Set err value to zero here. Any fail later
>> +  * will set appropriate return code to err.
>> +  */
>> + *err = 0;
>> +
>> evlist__for_each_entry(evlist, pos) {
>> if (strstarts(pos->name, "vpa_dtl")) {
>> found = 1;
>> diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
>> index 4a5eba498c02..708825747af5 100644
>> --- a/tools/perf/builtin-record.c
>> +++ b/tools/perf/builtin-record.c
>> @@ -865,6 +865,7 @@ static int record__auxtrace_init(struct record *rec)
>> }
>> 
>> if (!rec->itr) {
>> + err = -EINVAL;
>> rec->itr = auxtrace_record__init(rec->evlist, &err);
>> if (err)
>> return err;
>> -- 
>> 2.47.3




^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH V5 1/2] powerpc tools perf: Initialize error code in auxtrace_record_init function
  2026-05-07  6:13 ` [PATCH V5 1/2] powerpc tools perf: Initialize error code in auxtrace_record_init function Namhyung Kim
  2026-05-21  7:31   ` Athira Rajeev
@ 2026-05-27 11:10   ` Arnaldo Carvalho de Melo
  1 sibling, 0 replies; 6+ messages in thread
From: Arnaldo Carvalho de Melo @ 2026-05-27 11:10 UTC (permalink / raw)
  To: Namhyung Kim
  Cc: Athira Rajeev, jolsa, adrian.hunter, mpetlan, tmricht, maddy,
	irogers, linux-perf-users, linuxppc-dev, hbathini, Tejas.Manhas1,
	Tanushree.Shah, shivani

On Wed, May 06, 2026 at 11:13:43PM -0700, Namhyung Kim wrote:
> On Mon, May 04, 2026 at 08:43:20PM +0530, Athira Rajeev wrote:
> > Fixes: 1dbfaf94cf66 ("perf powerpc: Add basic CONFIG_AUXTRACE support for VPA pmu on powerpc")
> > Reviewed-by: Adrian Hunter <adrian.hunter@intel.com>
> > Signed-off-by: Athira Rajeev <atrajeev@linux.ibm.com>
> 
> For both patches,
> 
> Acked-by: Namhyung Kim <namhyung@kernel.org>

Thanks, applied to perf-tools-next, for v7.2.

- Arnaldo


^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2026-05-27 11:11 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-05-04 15:13 [PATCH V5 1/2] powerpc tools perf: Initialize error code in auxtrace_record_init function Athira Rajeev
2026-05-04 15:13 ` [PATCH V5 2/2] tools/perf: Add kernel-doc comment to auxtrace_record__init() function Athira Rajeev
2026-05-05 13:29   ` Adrian Hunter
2026-05-07  6:13 ` [PATCH V5 1/2] powerpc tools perf: Initialize error code in auxtrace_record_init function Namhyung Kim
2026-05-21  7:31   ` Athira Rajeev
2026-05-27 11:10   ` Arnaldo Carvalho de Melo

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.