All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] perf tools: return errcode in cs_etm__process_auxtrace_info
@ 2019-03-15  2:26 ` Yue Haibing
  0 siblings, 0 replies; 16+ messages in thread
From: Yue Haibing @ 2019-03-15  2:26 UTC (permalink / raw)
  To: mathieu.poirier, suzuki.poulose, peterz, mingo, acme,
	alexander.shishkin, jolsa, namhyung
  Cc: linux-kernel, linux-arm-kernel, YueHaibing

From: YueHaibing <yuehaibing@huawei.com>

'err' is set in err path, but it's not returned to callers.
Also fix a pass zero to PTR_ERR issue.

Fixes: cd8bfd8c973e ("perf tools: Add processing of coresight metadata")
Signed-off-by: YueHaibing <yuehaibing@huawei.com>
---
 tools/perf/util/cs-etm.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/perf/util/cs-etm.c b/tools/perf/util/cs-etm.c
index 1108049..111f33c 100644
--- a/tools/perf/util/cs-etm.c
+++ b/tools/perf/util/cs-etm.c
@@ -1908,7 +1908,7 @@ int cs_etm__process_auxtrace_info(union perf_event *event,
 
 		/* Something went wrong, no need to continue */
 		if (!inode) {
-			err = PTR_ERR(inode);
+			err = -ENOMEM;
 			goto err_free_metadata;
 		}
 
@@ -2023,5 +2023,5 @@ int cs_etm__process_auxtrace_info(union perf_event *event,
 err_free_hdr:
 	zfree(&hdr);
 
-	return -EINVAL;
+	return err;
 }
-- 
2.7.0



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

* [PATCH] perf tools: return errcode in cs_etm__process_auxtrace_info
@ 2019-03-15  2:26 ` Yue Haibing
  0 siblings, 0 replies; 16+ messages in thread
From: Yue Haibing @ 2019-03-15  2:26 UTC (permalink / raw)
  To: mathieu.poirier, suzuki.poulose, peterz, mingo, acme,
	alexander.shishkin, jolsa, namhyung
  Cc: YueHaibing, linux-kernel, linux-arm-kernel

From: YueHaibing <yuehaibing@huawei.com>

'err' is set in err path, but it's not returned to callers.
Also fix a pass zero to PTR_ERR issue.

Fixes: cd8bfd8c973e ("perf tools: Add processing of coresight metadata")
Signed-off-by: YueHaibing <yuehaibing@huawei.com>
---
 tools/perf/util/cs-etm.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/perf/util/cs-etm.c b/tools/perf/util/cs-etm.c
index 1108049..111f33c 100644
--- a/tools/perf/util/cs-etm.c
+++ b/tools/perf/util/cs-etm.c
@@ -1908,7 +1908,7 @@ int cs_etm__process_auxtrace_info(union perf_event *event,
 
 		/* Something went wrong, no need to continue */
 		if (!inode) {
-			err = PTR_ERR(inode);
+			err = -ENOMEM;
 			goto err_free_metadata;
 		}
 
@@ -2023,5 +2023,5 @@ int cs_etm__process_auxtrace_info(union perf_event *event,
 err_free_hdr:
 	zfree(&hdr);
 
-	return -EINVAL;
+	return err;
 }
-- 
2.7.0



_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* Re: [PATCH] perf tools: return errcode in cs_etm__process_auxtrace_info
  2019-03-15  2:26 ` Yue Haibing
@ 2019-03-18 17:13   ` Mathieu Poirier
  -1 siblings, 0 replies; 16+ messages in thread
From: Mathieu Poirier @ 2019-03-18 17:13 UTC (permalink / raw)
  To: Yue Haibing
  Cc: Suzuki K. Poulose, Peter Zijlstra, Ingo Molnar,
	Arnaldo Carvalho de Melo, Alexander Shishkin, Jiri Olsa,
	Namhyung Kim, Linux Kernel Mailing List, linux-arm-kernel

Good day Yue,

On Thu, 14 Mar 2019 at 20:28, Yue Haibing <yuehaibing@huawei.com> wrote:
>
> From: YueHaibing <yuehaibing@huawei.com>
>
> 'err' is set in err path, but it's not returned to callers.
> Also fix a pass zero to PTR_ERR issue.
>
> Fixes: cd8bfd8c973e ("perf tools: Add processing of coresight metadata")
> Signed-off-by: YueHaibing <yuehaibing@huawei.com>
> ---
>  tools/perf/util/cs-etm.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/tools/perf/util/cs-etm.c b/tools/perf/util/cs-etm.c
> index 1108049..111f33c 100644
> --- a/tools/perf/util/cs-etm.c
> +++ b/tools/perf/util/cs-etm.c
> @@ -1908,7 +1908,7 @@ int cs_etm__process_auxtrace_info(union perf_event *event,
>
>                 /* Something went wrong, no need to continue */
>                 if (!inode) {
> -                       err = PTR_ERR(inode);
> +                       err = -ENOMEM;

Good catch.

>                         goto err_free_metadata;
>                 }
>
> @@ -2023,5 +2023,5 @@ int cs_etm__process_auxtrace_info(union perf_event *event,
>  err_free_hdr:
>         zfree(&hdr);
>
> -       return -EINVAL;
> +       return err;

If we do that we need to set 'err' to an error code here [1] and here
[2].  Do you think you could fix this and roll another version of your
patch?

[1]. https://elixir.bootlin.com/linux/v5.1-rc1/source/tools/perf/util/cs-etm.c#L1967
[2]. https://elixir.bootlin.com/linux/v5.1-rc1/source/tools/perf/util/cs-etm.c#L1981
>  }
> --
> 2.7.0
>
>

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

* Re: [PATCH] perf tools: return errcode in cs_etm__process_auxtrace_info
@ 2019-03-18 17:13   ` Mathieu Poirier
  0 siblings, 0 replies; 16+ messages in thread
From: Mathieu Poirier @ 2019-03-18 17:13 UTC (permalink / raw)
  To: Yue Haibing
  Cc: Suzuki K. Poulose, Peter Zijlstra, Linux Kernel Mailing List,
	Arnaldo Carvalho de Melo, Alexander Shishkin, Ingo Molnar,
	Namhyung Kim, Jiri Olsa, linux-arm-kernel

Good day Yue,

On Thu, 14 Mar 2019 at 20:28, Yue Haibing <yuehaibing@huawei.com> wrote:
>
> From: YueHaibing <yuehaibing@huawei.com>
>
> 'err' is set in err path, but it's not returned to callers.
> Also fix a pass zero to PTR_ERR issue.
>
> Fixes: cd8bfd8c973e ("perf tools: Add processing of coresight metadata")
> Signed-off-by: YueHaibing <yuehaibing@huawei.com>
> ---
>  tools/perf/util/cs-etm.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/tools/perf/util/cs-etm.c b/tools/perf/util/cs-etm.c
> index 1108049..111f33c 100644
> --- a/tools/perf/util/cs-etm.c
> +++ b/tools/perf/util/cs-etm.c
> @@ -1908,7 +1908,7 @@ int cs_etm__process_auxtrace_info(union perf_event *event,
>
>                 /* Something went wrong, no need to continue */
>                 if (!inode) {
> -                       err = PTR_ERR(inode);
> +                       err = -ENOMEM;

Good catch.

>                         goto err_free_metadata;
>                 }
>
> @@ -2023,5 +2023,5 @@ int cs_etm__process_auxtrace_info(union perf_event *event,
>  err_free_hdr:
>         zfree(&hdr);
>
> -       return -EINVAL;
> +       return err;

If we do that we need to set 'err' to an error code here [1] and here
[2].  Do you think you could fix this and roll another version of your
patch?

[1]. https://elixir.bootlin.com/linux/v5.1-rc1/source/tools/perf/util/cs-etm.c#L1967
[2]. https://elixir.bootlin.com/linux/v5.1-rc1/source/tools/perf/util/cs-etm.c#L1981
>  }
> --
> 2.7.0
>
>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* Re: [PATCH] perf tools: return errcode in cs_etm__process_auxtrace_info
  2019-03-15  2:26 ` Yue Haibing
@ 2019-03-18 17:15   ` Arnaldo Carvalho de Melo
  -1 siblings, 0 replies; 16+ messages in thread
From: Arnaldo Carvalho de Melo @ 2019-03-18 17:15 UTC (permalink / raw)
  To: Yue Haibing
  Cc: mathieu.poirier, suzuki.poulose, peterz, mingo,
	alexander.shishkin, jolsa, namhyung, linux-kernel,
	linux-arm-kernel

Em Fri, Mar 15, 2019 at 10:26:49AM +0800, Yue Haibing escreveu:
> From: YueHaibing <yuehaibing@huawei.com>
> 
> 'err' is set in err path, but it's not returned to callers.
> Also fix a pass zero to PTR_ERR issue.

Next time please submit two patches, one for the PTR_ERR and another for
not throwing away the err = -E!INVAL and returning just -EINVAL, I'm
doing it this time.
 
 - Arnaldo

> Fixes: cd8bfd8c973e ("perf tools: Add processing of coresight metadata")
> Signed-off-by: YueHaibing <yuehaibing@huawei.com>
> ---
>  tools/perf/util/cs-etm.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/perf/util/cs-etm.c b/tools/perf/util/cs-etm.c
> index 1108049..111f33c 100644
> --- a/tools/perf/util/cs-etm.c
> +++ b/tools/perf/util/cs-etm.c
> @@ -1908,7 +1908,7 @@ int cs_etm__process_auxtrace_info(union perf_event *event,
>  
>  		/* Something went wrong, no need to continue */
>  		if (!inode) {
> -			err = PTR_ERR(inode);
> +			err = -ENOMEM;
>  			goto err_free_metadata;
>  		}
>  
> @@ -2023,5 +2023,5 @@ int cs_etm__process_auxtrace_info(union perf_event *event,
>  err_free_hdr:
>  	zfree(&hdr);
>  
> -	return -EINVAL;
> +	return err;
>  }
> -- 
> 2.7.0
> 

-- 

- Arnaldo

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

* Re: [PATCH] perf tools: return errcode in cs_etm__process_auxtrace_info
@ 2019-03-18 17:15   ` Arnaldo Carvalho de Melo
  0 siblings, 0 replies; 16+ messages in thread
From: Arnaldo Carvalho de Melo @ 2019-03-18 17:15 UTC (permalink / raw)
  To: Yue Haibing
  Cc: mathieu.poirier, suzuki.poulose, peterz, linux-kernel,
	alexander.shishkin, mingo, namhyung, jolsa, linux-arm-kernel

Em Fri, Mar 15, 2019 at 10:26:49AM +0800, Yue Haibing escreveu:
> From: YueHaibing <yuehaibing@huawei.com>
> 
> 'err' is set in err path, but it's not returned to callers.
> Also fix a pass zero to PTR_ERR issue.

Next time please submit two patches, one for the PTR_ERR and another for
not throwing away the err = -E!INVAL and returning just -EINVAL, I'm
doing it this time.
 
 - Arnaldo

> Fixes: cd8bfd8c973e ("perf tools: Add processing of coresight metadata")
> Signed-off-by: YueHaibing <yuehaibing@huawei.com>
> ---
>  tools/perf/util/cs-etm.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/perf/util/cs-etm.c b/tools/perf/util/cs-etm.c
> index 1108049..111f33c 100644
> --- a/tools/perf/util/cs-etm.c
> +++ b/tools/perf/util/cs-etm.c
> @@ -1908,7 +1908,7 @@ int cs_etm__process_auxtrace_info(union perf_event *event,
>  
>  		/* Something went wrong, no need to continue */
>  		if (!inode) {
> -			err = PTR_ERR(inode);
> +			err = -ENOMEM;
>  			goto err_free_metadata;
>  		}
>  
> @@ -2023,5 +2023,5 @@ int cs_etm__process_auxtrace_info(union perf_event *event,
>  err_free_hdr:
>  	zfree(&hdr);
>  
> -	return -EINVAL;
> +	return err;
>  }
> -- 
> 2.7.0
> 

-- 

- Arnaldo

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* Re: [PATCH] perf tools: return errcode in cs_etm__process_auxtrace_info
  2019-03-18 17:15   ` Arnaldo Carvalho de Melo
@ 2019-03-19 14:38     ` Mathieu Poirier
  -1 siblings, 0 replies; 16+ messages in thread
From: Mathieu Poirier @ 2019-03-19 14:38 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Yue Haibing, Suzuki K. Poulose, Peter Zijlstra, Ingo Molnar,
	Alexander Shishkin, Jiri Olsa, Namhyung Kim,
	Linux Kernel Mailing List, linux-arm-kernel

On Mon, 18 Mar 2019 at 11:15, Arnaldo Carvalho de Melo
<arnaldo.melo@gmail.com> wrote:
>
> Em Fri, Mar 15, 2019 at 10:26:49AM +0800, Yue Haibing escreveu:
> > From: YueHaibing <yuehaibing@huawei.com>
> >
> > 'err' is set in err path, but it's not returned to callers.
> > Also fix a pass zero to PTR_ERR issue.
>
> Next time please submit two patches, one for the PTR_ERR and another for
> not throwing away the err = -E!INVAL and returning just -EINVAL, I'm
> doing it this time.
>

Please hold off on that - I've asked for other modifications to be
done on this patch.

Mathieu

>  - Arnaldo
>
> > Fixes: cd8bfd8c973e ("perf tools: Add processing of coresight metadata")
> > Signed-off-by: YueHaibing <yuehaibing@huawei.com>
> > ---
> >  tools/perf/util/cs-etm.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/tools/perf/util/cs-etm.c b/tools/perf/util/cs-etm.c
> > index 1108049..111f33c 100644
> > --- a/tools/perf/util/cs-etm.c
> > +++ b/tools/perf/util/cs-etm.c
> > @@ -1908,7 +1908,7 @@ int cs_etm__process_auxtrace_info(union perf_event *event,
> >
> >               /* Something went wrong, no need to continue */
> >               if (!inode) {
> > -                     err = PTR_ERR(inode);
> > +                     err = -ENOMEM;
> >                       goto err_free_metadata;
> >               }
> >
> > @@ -2023,5 +2023,5 @@ int cs_etm__process_auxtrace_info(union perf_event *event,
> >  err_free_hdr:
> >       zfree(&hdr);
> >
> > -     return -EINVAL;
> > +     return err;
> >  }
> > --
> > 2.7.0
> >
>
> --
>
> - Arnaldo

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

* Re: [PATCH] perf tools: return errcode in cs_etm__process_auxtrace_info
@ 2019-03-19 14:38     ` Mathieu Poirier
  0 siblings, 0 replies; 16+ messages in thread
From: Mathieu Poirier @ 2019-03-19 14:38 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Suzuki K. Poulose, Peter Zijlstra, Yue Haibing,
	Linux Kernel Mailing List, Alexander Shishkin, Ingo Molnar,
	Namhyung Kim, Jiri Olsa, linux-arm-kernel

On Mon, 18 Mar 2019 at 11:15, Arnaldo Carvalho de Melo
<arnaldo.melo@gmail.com> wrote:
>
> Em Fri, Mar 15, 2019 at 10:26:49AM +0800, Yue Haibing escreveu:
> > From: YueHaibing <yuehaibing@huawei.com>
> >
> > 'err' is set in err path, but it's not returned to callers.
> > Also fix a pass zero to PTR_ERR issue.
>
> Next time please submit two patches, one for the PTR_ERR and another for
> not throwing away the err = -E!INVAL and returning just -EINVAL, I'm
> doing it this time.
>

Please hold off on that - I've asked for other modifications to be
done on this patch.

Mathieu

>  - Arnaldo
>
> > Fixes: cd8bfd8c973e ("perf tools: Add processing of coresight metadata")
> > Signed-off-by: YueHaibing <yuehaibing@huawei.com>
> > ---
> >  tools/perf/util/cs-etm.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/tools/perf/util/cs-etm.c b/tools/perf/util/cs-etm.c
> > index 1108049..111f33c 100644
> > --- a/tools/perf/util/cs-etm.c
> > +++ b/tools/perf/util/cs-etm.c
> > @@ -1908,7 +1908,7 @@ int cs_etm__process_auxtrace_info(union perf_event *event,
> >
> >               /* Something went wrong, no need to continue */
> >               if (!inode) {
> > -                     err = PTR_ERR(inode);
> > +                     err = -ENOMEM;
> >                       goto err_free_metadata;
> >               }
> >
> > @@ -2023,5 +2023,5 @@ int cs_etm__process_auxtrace_info(union perf_event *event,
> >  err_free_hdr:
> >       zfree(&hdr);
> >
> > -     return -EINVAL;
> > +     return err;
> >  }
> > --
> > 2.7.0
> >
>
> --
>
> - Arnaldo

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* Re: [PATCH] perf tools: return errcode in cs_etm__process_auxtrace_info
  2019-03-19 14:38     ` Mathieu Poirier
@ 2019-03-19 14:46       ` Arnaldo Carvalho de Melo
  -1 siblings, 0 replies; 16+ messages in thread
From: Arnaldo Carvalho de Melo @ 2019-03-19 14:46 UTC (permalink / raw)
  To: Mathieu Poirier
  Cc: Arnaldo Carvalho de Melo, Yue Haibing, Suzuki K. Poulose,
	Peter Zijlstra, Ingo Molnar, Alexander Shishkin, Jiri Olsa,
	Namhyung Kim, Linux Kernel Mailing List, linux-arm-kernel

Em Tue, Mar 19, 2019 at 08:38:32AM -0600, Mathieu Poirier escreveu:
> On Mon, 18 Mar 2019 at 11:15, Arnaldo Carvalho de Melo
> <arnaldo.melo@gmail.com> wrote:
> >
> > Em Fri, Mar 15, 2019 at 10:26:49AM +0800, Yue Haibing escreveu:
> > > From: YueHaibing <yuehaibing@huawei.com>
> > >
> > > 'err' is set in err path, but it's not returned to callers.
> > > Also fix a pass zero to PTR_ERR issue.
> >
> > Next time please submit two patches, one for the PTR_ERR and another for
> > not throwing away the err = -E!INVAL and returning just -EINVAL, I'm
> > doing it this time.
> >
> 
> Please hold off on that - I've asked for other modifications to be
> done on this patch.

Do you really think that it is necessary to hold? The fixes are trivial
and I already have them split and applied, see them below, I think
whatever other changes can be done in further patches, no?

- Arnaldo

From 6c5c248935a4da12a582b1518a38ec35044833ee Mon Sep 17 00:00:00 2001
From: YueHaibing <yuehaibing@huawei.com>
Date: Fri, 15 Mar 2019 10:26:49 +0800
Subject: [PATCH 24/42] perf cs-etm: return errcode in
 cs_etm__process_auxtrace_info()

'err' is set in err path, but it's not returned to callers. Don't always return
-EINVAL, return err.

Signed-off-by: YueHaibing <yuehaibing@huawei.com>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Suzuki K Poulouse <suzuki.poulose@arm.com>
Cc: linux-arm-kernel@lists.infradead.org
Fixes: cd8bfd8c973e ("perf tools: Add processing of coresight metadata")
Link: http://lkml.kernel.org/r/20190315022649.17848-1-yuehaibing@huawei.com
[ split from a larger patch ]
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/cs-etm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/perf/util/cs-etm.c b/tools/perf/util/cs-etm.c
index 110804936fc3..2257ac4dbff2 100644
--- a/tools/perf/util/cs-etm.c
+++ b/tools/perf/util/cs-etm.c
@@ -2023,5 +2023,5 @@ int cs_etm__process_auxtrace_info(union perf_event *event,
 err_free_hdr:
 	zfree(&hdr);
 
-	return -EINVAL;
+	return err;
 }
-- 
2.20.1

From 4ab1e71e45a91220bdca7fc3b79c600df81d9234 Mon Sep 17 00:00:00 2001
From: YueHaibing <yuehaibing@huawei.com>
Date: Fri, 15 Mar 2019 10:26:49 +0800
Subject: [PATCH 25/42] perf cs-etm: Remove errnoeous ERR_PTR() usage in in
 cs_etm__process_auxtrace_info

intlist__findnew() doesn't uses ERR_PTR() as a return mechanism so its callers
shouldn't try to extract the error using PTR_ERR(ret-from-intlist__findnew()),
make cs_etm__process_auxtrace_info9) return -ENOMEM instead.

Signed-off-by: YueHaibing <yuehaibing@huawei.com>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Suzuki K Poulouse <suzuki.poulose@arm.com>
Cc: linux-arm-kernel@lists.infradead.org
Fixes: cd8bfd8c973e ("perf tools: Add processing of coresight metadata")
Link: http://lkml.kernel.org/r/20190315022649.17848-1-yuehaibing@huawei.com
[ split from a larger patch ]
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/cs-etm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/perf/util/cs-etm.c b/tools/perf/util/cs-etm.c
index 2257ac4dbff2..111f33cd1204 100644
--- a/tools/perf/util/cs-etm.c
+++ b/tools/perf/util/cs-etm.c
@@ -1908,7 +1908,7 @@ int cs_etm__process_auxtrace_info(union perf_event *event,
 
 		/* Something went wrong, no need to continue */
 		if (!inode) {
-			err = PTR_ERR(inode);
+			err = -ENOMEM;
 			goto err_free_metadata;
 		}
 
-- 
2.20.1


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

* Re: [PATCH] perf tools: return errcode in cs_etm__process_auxtrace_info
@ 2019-03-19 14:46       ` Arnaldo Carvalho de Melo
  0 siblings, 0 replies; 16+ messages in thread
From: Arnaldo Carvalho de Melo @ 2019-03-19 14:46 UTC (permalink / raw)
  To: Mathieu Poirier
  Cc: Suzuki K. Poulose, Peter Zijlstra, Yue Haibing,
	Arnaldo Carvalho de Melo, Linux Kernel Mailing List,
	Alexander Shishkin, Ingo Molnar, Namhyung Kim, Jiri Olsa,
	linux-arm-kernel

Em Tue, Mar 19, 2019 at 08:38:32AM -0600, Mathieu Poirier escreveu:
> On Mon, 18 Mar 2019 at 11:15, Arnaldo Carvalho de Melo
> <arnaldo.melo@gmail.com> wrote:
> >
> > Em Fri, Mar 15, 2019 at 10:26:49AM +0800, Yue Haibing escreveu:
> > > From: YueHaibing <yuehaibing@huawei.com>
> > >
> > > 'err' is set in err path, but it's not returned to callers.
> > > Also fix a pass zero to PTR_ERR issue.
> >
> > Next time please submit two patches, one for the PTR_ERR and another for
> > not throwing away the err = -E!INVAL and returning just -EINVAL, I'm
> > doing it this time.
> >
> 
> Please hold off on that - I've asked for other modifications to be
> done on this patch.

Do you really think that it is necessary to hold? The fixes are trivial
and I already have them split and applied, see them below, I think
whatever other changes can be done in further patches, no?

- Arnaldo

From 6c5c248935a4da12a582b1518a38ec35044833ee Mon Sep 17 00:00:00 2001
From: YueHaibing <yuehaibing@huawei.com>
Date: Fri, 15 Mar 2019 10:26:49 +0800
Subject: [PATCH 24/42] perf cs-etm: return errcode in
 cs_etm__process_auxtrace_info()

'err' is set in err path, but it's not returned to callers. Don't always return
-EINVAL, return err.

Signed-off-by: YueHaibing <yuehaibing@huawei.com>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Suzuki K Poulouse <suzuki.poulose@arm.com>
Cc: linux-arm-kernel@lists.infradead.org
Fixes: cd8bfd8c973e ("perf tools: Add processing of coresight metadata")
Link: http://lkml.kernel.org/r/20190315022649.17848-1-yuehaibing@huawei.com
[ split from a larger patch ]
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/cs-etm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/perf/util/cs-etm.c b/tools/perf/util/cs-etm.c
index 110804936fc3..2257ac4dbff2 100644
--- a/tools/perf/util/cs-etm.c
+++ b/tools/perf/util/cs-etm.c
@@ -2023,5 +2023,5 @@ int cs_etm__process_auxtrace_info(union perf_event *event,
 err_free_hdr:
 	zfree(&hdr);
 
-	return -EINVAL;
+	return err;
 }
-- 
2.20.1

From 4ab1e71e45a91220bdca7fc3b79c600df81d9234 Mon Sep 17 00:00:00 2001
From: YueHaibing <yuehaibing@huawei.com>
Date: Fri, 15 Mar 2019 10:26:49 +0800
Subject: [PATCH 25/42] perf cs-etm: Remove errnoeous ERR_PTR() usage in in
 cs_etm__process_auxtrace_info

intlist__findnew() doesn't uses ERR_PTR() as a return mechanism so its callers
shouldn't try to extract the error using PTR_ERR(ret-from-intlist__findnew()),
make cs_etm__process_auxtrace_info9) return -ENOMEM instead.

Signed-off-by: YueHaibing <yuehaibing@huawei.com>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Suzuki K Poulouse <suzuki.poulose@arm.com>
Cc: linux-arm-kernel@lists.infradead.org
Fixes: cd8bfd8c973e ("perf tools: Add processing of coresight metadata")
Link: http://lkml.kernel.org/r/20190315022649.17848-1-yuehaibing@huawei.com
[ split from a larger patch ]
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/cs-etm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/perf/util/cs-etm.c b/tools/perf/util/cs-etm.c
index 2257ac4dbff2..111f33cd1204 100644
--- a/tools/perf/util/cs-etm.c
+++ b/tools/perf/util/cs-etm.c
@@ -1908,7 +1908,7 @@ int cs_etm__process_auxtrace_info(union perf_event *event,
 
 		/* Something went wrong, no need to continue */
 		if (!inode) {
-			err = PTR_ERR(inode);
+			err = -ENOMEM;
 			goto err_free_metadata;
 		}
 
-- 
2.20.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* Re: [PATCH] perf tools: return errcode in cs_etm__process_auxtrace_info
  2019-03-19 14:46       ` Arnaldo Carvalho de Melo
@ 2019-03-19 14:55         ` Mathieu Poirier
  -1 siblings, 0 replies; 16+ messages in thread
From: Mathieu Poirier @ 2019-03-19 14:55 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Yue Haibing, Suzuki K. Poulose, Peter Zijlstra, Ingo Molnar,
	Alexander Shishkin, Jiri Olsa, Namhyung Kim,
	Linux Kernel Mailing List, linux-arm-kernel

On Tue, Mar 19, 2019 at 11:46:31AM -0300, Arnaldo Carvalho de Melo wrote:
> Em Tue, Mar 19, 2019 at 08:38:32AM -0600, Mathieu Poirier escreveu:
> > On Mon, 18 Mar 2019 at 11:15, Arnaldo Carvalho de Melo
> > <arnaldo.melo@gmail.com> wrote:
> > >
> > > Em Fri, Mar 15, 2019 at 10:26:49AM +0800, Yue Haibing escreveu:
> > > > From: YueHaibing <yuehaibing@huawei.com>
> > > >
> > > > 'err' is set in err path, but it's not returned to callers.
> > > > Also fix a pass zero to PTR_ERR issue.
> > >
> > > Next time please submit two patches, one for the PTR_ERR and another for
> > > not throwing away the err = -E!INVAL and returning just -EINVAL, I'm
> > > doing it this time.
> > >
> > 
> > Please hold off on that - I've asked for other modifications to be
> > done on this patch.
> 
> Do you really think that it is necessary to hold? The fixes are trivial
> and I already have them split and applied, see them below, I think
> whatever other changes can be done in further patches, no?

Proceeding with the patches below would created two new bugs, hence asking Yue
for modifications.

Mathieu  

> 
> - Arnaldo
> 
> From 6c5c248935a4da12a582b1518a38ec35044833ee Mon Sep 17 00:00:00 2001
> From: YueHaibing <yuehaibing@huawei.com>
> Date: Fri, 15 Mar 2019 10:26:49 +0800
> Subject: [PATCH 24/42] perf cs-etm: return errcode in
>  cs_etm__process_auxtrace_info()
> 
> 'err' is set in err path, but it's not returned to callers. Don't always return
> -EINVAL, return err.
> 
> Signed-off-by: YueHaibing <yuehaibing@huawei.com>
> Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
> Cc: Jiri Olsa <jolsa@redhat.com>
> Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
> Cc: Namhyung Kim <namhyung@kernel.org>
> Cc: Peter Zijlstra <peterz@infradead.org>
> Cc: Suzuki K Poulouse <suzuki.poulose@arm.com>
> Cc: linux-arm-kernel@lists.infradead.org
> Fixes: cd8bfd8c973e ("perf tools: Add processing of coresight metadata")
> Link: http://lkml.kernel.org/r/20190315022649.17848-1-yuehaibing@huawei.com
> [ split from a larger patch ]
> Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
> ---
>  tools/perf/util/cs-etm.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tools/perf/util/cs-etm.c b/tools/perf/util/cs-etm.c
> index 110804936fc3..2257ac4dbff2 100644
> --- a/tools/perf/util/cs-etm.c
> +++ b/tools/perf/util/cs-etm.c
> @@ -2023,5 +2023,5 @@ int cs_etm__process_auxtrace_info(union perf_event *event,
>  err_free_hdr:
>  	zfree(&hdr);
>  
> -	return -EINVAL;
> +	return err;
>  }
> -- 
> 2.20.1
> 
> From 4ab1e71e45a91220bdca7fc3b79c600df81d9234 Mon Sep 17 00:00:00 2001
> From: YueHaibing <yuehaibing@huawei.com>
> Date: Fri, 15 Mar 2019 10:26:49 +0800
> Subject: [PATCH 25/42] perf cs-etm: Remove errnoeous ERR_PTR() usage in in
>  cs_etm__process_auxtrace_info
> 
> intlist__findnew() doesn't uses ERR_PTR() as a return mechanism so its callers
> shouldn't try to extract the error using PTR_ERR(ret-from-intlist__findnew()),
> make cs_etm__process_auxtrace_info9) return -ENOMEM instead.
> 
> Signed-off-by: YueHaibing <yuehaibing@huawei.com>
> Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
> Cc: Jiri Olsa <jolsa@redhat.com>
> Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
> Cc: Namhyung Kim <namhyung@kernel.org>
> Cc: Peter Zijlstra <peterz@infradead.org>
> Cc: Suzuki K Poulouse <suzuki.poulose@arm.com>
> Cc: linux-arm-kernel@lists.infradead.org
> Fixes: cd8bfd8c973e ("perf tools: Add processing of coresight metadata")
> Link: http://lkml.kernel.org/r/20190315022649.17848-1-yuehaibing@huawei.com
> [ split from a larger patch ]
> Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
> ---
>  tools/perf/util/cs-etm.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tools/perf/util/cs-etm.c b/tools/perf/util/cs-etm.c
> index 2257ac4dbff2..111f33cd1204 100644
> --- a/tools/perf/util/cs-etm.c
> +++ b/tools/perf/util/cs-etm.c
> @@ -1908,7 +1908,7 @@ int cs_etm__process_auxtrace_info(union perf_event *event,
>  
>  		/* Something went wrong, no need to continue */
>  		if (!inode) {
> -			err = PTR_ERR(inode);
> +			err = -ENOMEM;
>  			goto err_free_metadata;
>  		}
>  
> -- 
> 2.20.1
> 

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

* Re: [PATCH] perf tools: return errcode in cs_etm__process_auxtrace_info
@ 2019-03-19 14:55         ` Mathieu Poirier
  0 siblings, 0 replies; 16+ messages in thread
From: Mathieu Poirier @ 2019-03-19 14:55 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Suzuki K. Poulose, Peter Zijlstra, Yue Haibing,
	Linux Kernel Mailing List, Alexander Shishkin, Ingo Molnar,
	Namhyung Kim, Jiri Olsa, linux-arm-kernel

On Tue, Mar 19, 2019 at 11:46:31AM -0300, Arnaldo Carvalho de Melo wrote:
> Em Tue, Mar 19, 2019 at 08:38:32AM -0600, Mathieu Poirier escreveu:
> > On Mon, 18 Mar 2019 at 11:15, Arnaldo Carvalho de Melo
> > <arnaldo.melo@gmail.com> wrote:
> > >
> > > Em Fri, Mar 15, 2019 at 10:26:49AM +0800, Yue Haibing escreveu:
> > > > From: YueHaibing <yuehaibing@huawei.com>
> > > >
> > > > 'err' is set in err path, but it's not returned to callers.
> > > > Also fix a pass zero to PTR_ERR issue.
> > >
> > > Next time please submit two patches, one for the PTR_ERR and another for
> > > not throwing away the err = -E!INVAL and returning just -EINVAL, I'm
> > > doing it this time.
> > >
> > 
> > Please hold off on that - I've asked for other modifications to be
> > done on this patch.
> 
> Do you really think that it is necessary to hold? The fixes are trivial
> and I already have them split and applied, see them below, I think
> whatever other changes can be done in further patches, no?

Proceeding with the patches below would created two new bugs, hence asking Yue
for modifications.

Mathieu  

> 
> - Arnaldo
> 
> From 6c5c248935a4da12a582b1518a38ec35044833ee Mon Sep 17 00:00:00 2001
> From: YueHaibing <yuehaibing@huawei.com>
> Date: Fri, 15 Mar 2019 10:26:49 +0800
> Subject: [PATCH 24/42] perf cs-etm: return errcode in
>  cs_etm__process_auxtrace_info()
> 
> 'err' is set in err path, but it's not returned to callers. Don't always return
> -EINVAL, return err.
> 
> Signed-off-by: YueHaibing <yuehaibing@huawei.com>
> Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
> Cc: Jiri Olsa <jolsa@redhat.com>
> Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
> Cc: Namhyung Kim <namhyung@kernel.org>
> Cc: Peter Zijlstra <peterz@infradead.org>
> Cc: Suzuki K Poulouse <suzuki.poulose@arm.com>
> Cc: linux-arm-kernel@lists.infradead.org
> Fixes: cd8bfd8c973e ("perf tools: Add processing of coresight metadata")
> Link: http://lkml.kernel.org/r/20190315022649.17848-1-yuehaibing@huawei.com
> [ split from a larger patch ]
> Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
> ---
>  tools/perf/util/cs-etm.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tools/perf/util/cs-etm.c b/tools/perf/util/cs-etm.c
> index 110804936fc3..2257ac4dbff2 100644
> --- a/tools/perf/util/cs-etm.c
> +++ b/tools/perf/util/cs-etm.c
> @@ -2023,5 +2023,5 @@ int cs_etm__process_auxtrace_info(union perf_event *event,
>  err_free_hdr:
>  	zfree(&hdr);
>  
> -	return -EINVAL;
> +	return err;
>  }
> -- 
> 2.20.1
> 
> From 4ab1e71e45a91220bdca7fc3b79c600df81d9234 Mon Sep 17 00:00:00 2001
> From: YueHaibing <yuehaibing@huawei.com>
> Date: Fri, 15 Mar 2019 10:26:49 +0800
> Subject: [PATCH 25/42] perf cs-etm: Remove errnoeous ERR_PTR() usage in in
>  cs_etm__process_auxtrace_info
> 
> intlist__findnew() doesn't uses ERR_PTR() as a return mechanism so its callers
> shouldn't try to extract the error using PTR_ERR(ret-from-intlist__findnew()),
> make cs_etm__process_auxtrace_info9) return -ENOMEM instead.
> 
> Signed-off-by: YueHaibing <yuehaibing@huawei.com>
> Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
> Cc: Jiri Olsa <jolsa@redhat.com>
> Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
> Cc: Namhyung Kim <namhyung@kernel.org>
> Cc: Peter Zijlstra <peterz@infradead.org>
> Cc: Suzuki K Poulouse <suzuki.poulose@arm.com>
> Cc: linux-arm-kernel@lists.infradead.org
> Fixes: cd8bfd8c973e ("perf tools: Add processing of coresight metadata")
> Link: http://lkml.kernel.org/r/20190315022649.17848-1-yuehaibing@huawei.com
> [ split from a larger patch ]
> Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
> ---
>  tools/perf/util/cs-etm.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tools/perf/util/cs-etm.c b/tools/perf/util/cs-etm.c
> index 2257ac4dbff2..111f33cd1204 100644
> --- a/tools/perf/util/cs-etm.c
> +++ b/tools/perf/util/cs-etm.c
> @@ -1908,7 +1908,7 @@ int cs_etm__process_auxtrace_info(union perf_event *event,
>  
>  		/* Something went wrong, no need to continue */
>  		if (!inode) {
> -			err = PTR_ERR(inode);
> +			err = -ENOMEM;
>  			goto err_free_metadata;
>  		}
>  
> -- 
> 2.20.1
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* Re: [PATCH] perf tools: return errcode in cs_etm__process_auxtrace_info
  2019-03-19 14:55         ` Mathieu Poirier
@ 2019-03-19 15:12           ` YueHaibing
  -1 siblings, 0 replies; 16+ messages in thread
From: YueHaibing @ 2019-03-19 15:12 UTC (permalink / raw)
  To: Mathieu Poirier, Arnaldo Carvalho de Melo
  Cc: Suzuki K. Poulose, Peter Zijlstra, Ingo Molnar,
	Alexander Shishkin, Jiri Olsa, Namhyung Kim,
	Linux Kernel Mailing List, linux-arm-kernel


On 2019/3/19 22:55, Mathieu Poirier wrote:
> On Tue, Mar 19, 2019 at 11:46:31AM -0300, Arnaldo Carvalho de Melo wrote:
>> Em Tue, Mar 19, 2019 at 08:38:32AM -0600, Mathieu Poirier escreveu:
>>> On Mon, 18 Mar 2019 at 11:15, Arnaldo Carvalho de Melo
>>> <arnaldo.melo@gmail.com> wrote:
>>>>
>>>> Em Fri, Mar 15, 2019 at 10:26:49AM +0800, Yue Haibing escreveu:
>>>>> From: YueHaibing <yuehaibing@huawei.com>
>>>>>
>>>>> 'err' is set in err path, but it's not returned to callers.
>>>>> Also fix a pass zero to PTR_ERR issue.
>>>>
>>>> Next time please submit two patches, one for the PTR_ERR and another for
>>>> not throwing away the err = -E!INVAL and returning just -EINVAL, I'm
>>>> doing it this time.
>>>>
>>>
>>> Please hold off on that - I've asked for other modifications to be
>>> done on this patch.
>>
>> Do you really think that it is necessary to hold? The fixes are trivial
>> and I already have them split and applied, see them below, I think
>> whatever other changes can be done in further patches, no?
> 
> Proceeding with the patches below would created two new bugs, hence asking Yue
> for modifications.


Sorry for late, then I can send a new patch to fix the two new bugs.

> 
> Mathieu  
> 
>>
>> - Arnaldo
>>
>> From 6c5c248935a4da12a582b1518a38ec35044833ee Mon Sep 17 00:00:00 2001
>> From: YueHaibing <yuehaibing@huawei.com>
>> Date: Fri, 15 Mar 2019 10:26:49 +0800
>> Subject: [PATCH 24/42] perf cs-etm: return errcode in
>>  cs_etm__process_auxtrace_info()
>>
>> 'err' is set in err path, but it's not returned to callers. Don't always return
>> -EINVAL, return err.
>>
>> Signed-off-by: YueHaibing <yuehaibing@huawei.com>
>> Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
>> Cc: Jiri Olsa <jolsa@redhat.com>
>> Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
>> Cc: Namhyung Kim <namhyung@kernel.org>
>> Cc: Peter Zijlstra <peterz@infradead.org>
>> Cc: Suzuki K Poulouse <suzuki.poulose@arm.com>
>> Cc: linux-arm-kernel@lists.infradead.org
>> Fixes: cd8bfd8c973e ("perf tools: Add processing of coresight metadata")
>> Link: http://lkml.kernel.org/r/20190315022649.17848-1-yuehaibing@huawei.com
>> [ split from a larger patch ]
>> Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
>> ---
>>  tools/perf/util/cs-etm.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/tools/perf/util/cs-etm.c b/tools/perf/util/cs-etm.c
>> index 110804936fc3..2257ac4dbff2 100644
>> --- a/tools/perf/util/cs-etm.c
>> +++ b/tools/perf/util/cs-etm.c
>> @@ -2023,5 +2023,5 @@ int cs_etm__process_auxtrace_info(union perf_event *event,
>>  err_free_hdr:
>>  	zfree(&hdr);
>>  
>> -	return -EINVAL;
>> +	return err;
>>  }
>> -- 
>> 2.20.1
>>
>> From 4ab1e71e45a91220bdca7fc3b79c600df81d9234 Mon Sep 17 00:00:00 2001
>> From: YueHaibing <yuehaibing@huawei.com>
>> Date: Fri, 15 Mar 2019 10:26:49 +0800
>> Subject: [PATCH 25/42] perf cs-etm: Remove errnoeous ERR_PTR() usage in in
>>  cs_etm__process_auxtrace_info
>>
>> intlist__findnew() doesn't uses ERR_PTR() as a return mechanism so its callers
>> shouldn't try to extract the error using PTR_ERR(ret-from-intlist__findnew()),
>> make cs_etm__process_auxtrace_info9) return -ENOMEM instead.
>>
>> Signed-off-by: YueHaibing <yuehaibing@huawei.com>
>> Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
>> Cc: Jiri Olsa <jolsa@redhat.com>
>> Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
>> Cc: Namhyung Kim <namhyung@kernel.org>
>> Cc: Peter Zijlstra <peterz@infradead.org>
>> Cc: Suzuki K Poulouse <suzuki.poulose@arm.com>
>> Cc: linux-arm-kernel@lists.infradead.org
>> Fixes: cd8bfd8c973e ("perf tools: Add processing of coresight metadata")
>> Link: http://lkml.kernel.org/r/20190315022649.17848-1-yuehaibing@huawei.com
>> [ split from a larger patch ]
>> Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
>> ---
>>  tools/perf/util/cs-etm.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/tools/perf/util/cs-etm.c b/tools/perf/util/cs-etm.c
>> index 2257ac4dbff2..111f33cd1204 100644
>> --- a/tools/perf/util/cs-etm.c
>> +++ b/tools/perf/util/cs-etm.c
>> @@ -1908,7 +1908,7 @@ int cs_etm__process_auxtrace_info(union perf_event *event,
>>  
>>  		/* Something went wrong, no need to continue */
>>  		if (!inode) {
>> -			err = PTR_ERR(inode);
>> +			err = -ENOMEM;
>>  			goto err_free_metadata;
>>  		}
>>  
>> -- 
>> 2.20.1
>>
> 
> .
> 


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

* Re: [PATCH] perf tools: return errcode in cs_etm__process_auxtrace_info
@ 2019-03-19 15:12           ` YueHaibing
  0 siblings, 0 replies; 16+ messages in thread
From: YueHaibing @ 2019-03-19 15:12 UTC (permalink / raw)
  To: Mathieu Poirier, Arnaldo Carvalho de Melo
  Cc: Suzuki K. Poulose, Peter Zijlstra, Linux Kernel Mailing List,
	Alexander Shishkin, Ingo Molnar, Namhyung Kim, Jiri Olsa,
	linux-arm-kernel


On 2019/3/19 22:55, Mathieu Poirier wrote:
> On Tue, Mar 19, 2019 at 11:46:31AM -0300, Arnaldo Carvalho de Melo wrote:
>> Em Tue, Mar 19, 2019 at 08:38:32AM -0600, Mathieu Poirier escreveu:
>>> On Mon, 18 Mar 2019 at 11:15, Arnaldo Carvalho de Melo
>>> <arnaldo.melo@gmail.com> wrote:
>>>>
>>>> Em Fri, Mar 15, 2019 at 10:26:49AM +0800, Yue Haibing escreveu:
>>>>> From: YueHaibing <yuehaibing@huawei.com>
>>>>>
>>>>> 'err' is set in err path, but it's not returned to callers.
>>>>> Also fix a pass zero to PTR_ERR issue.
>>>>
>>>> Next time please submit two patches, one for the PTR_ERR and another for
>>>> not throwing away the err = -E!INVAL and returning just -EINVAL, I'm
>>>> doing it this time.
>>>>
>>>
>>> Please hold off on that - I've asked for other modifications to be
>>> done on this patch.
>>
>> Do you really think that it is necessary to hold? The fixes are trivial
>> and I already have them split and applied, see them below, I think
>> whatever other changes can be done in further patches, no?
> 
> Proceeding with the patches below would created two new bugs, hence asking Yue
> for modifications.


Sorry for late, then I can send a new patch to fix the two new bugs.

> 
> Mathieu  
> 
>>
>> - Arnaldo
>>
>> From 6c5c248935a4da12a582b1518a38ec35044833ee Mon Sep 17 00:00:00 2001
>> From: YueHaibing <yuehaibing@huawei.com>
>> Date: Fri, 15 Mar 2019 10:26:49 +0800
>> Subject: [PATCH 24/42] perf cs-etm: return errcode in
>>  cs_etm__process_auxtrace_info()
>>
>> 'err' is set in err path, but it's not returned to callers. Don't always return
>> -EINVAL, return err.
>>
>> Signed-off-by: YueHaibing <yuehaibing@huawei.com>
>> Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
>> Cc: Jiri Olsa <jolsa@redhat.com>
>> Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
>> Cc: Namhyung Kim <namhyung@kernel.org>
>> Cc: Peter Zijlstra <peterz@infradead.org>
>> Cc: Suzuki K Poulouse <suzuki.poulose@arm.com>
>> Cc: linux-arm-kernel@lists.infradead.org
>> Fixes: cd8bfd8c973e ("perf tools: Add processing of coresight metadata")
>> Link: http://lkml.kernel.org/r/20190315022649.17848-1-yuehaibing@huawei.com
>> [ split from a larger patch ]
>> Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
>> ---
>>  tools/perf/util/cs-etm.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/tools/perf/util/cs-etm.c b/tools/perf/util/cs-etm.c
>> index 110804936fc3..2257ac4dbff2 100644
>> --- a/tools/perf/util/cs-etm.c
>> +++ b/tools/perf/util/cs-etm.c
>> @@ -2023,5 +2023,5 @@ int cs_etm__process_auxtrace_info(union perf_event *event,
>>  err_free_hdr:
>>  	zfree(&hdr);
>>  
>> -	return -EINVAL;
>> +	return err;
>>  }
>> -- 
>> 2.20.1
>>
>> From 4ab1e71e45a91220bdca7fc3b79c600df81d9234 Mon Sep 17 00:00:00 2001
>> From: YueHaibing <yuehaibing@huawei.com>
>> Date: Fri, 15 Mar 2019 10:26:49 +0800
>> Subject: [PATCH 25/42] perf cs-etm: Remove errnoeous ERR_PTR() usage in in
>>  cs_etm__process_auxtrace_info
>>
>> intlist__findnew() doesn't uses ERR_PTR() as a return mechanism so its callers
>> shouldn't try to extract the error using PTR_ERR(ret-from-intlist__findnew()),
>> make cs_etm__process_auxtrace_info9) return -ENOMEM instead.
>>
>> Signed-off-by: YueHaibing <yuehaibing@huawei.com>
>> Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
>> Cc: Jiri Olsa <jolsa@redhat.com>
>> Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
>> Cc: Namhyung Kim <namhyung@kernel.org>
>> Cc: Peter Zijlstra <peterz@infradead.org>
>> Cc: Suzuki K Poulouse <suzuki.poulose@arm.com>
>> Cc: linux-arm-kernel@lists.infradead.org
>> Fixes: cd8bfd8c973e ("perf tools: Add processing of coresight metadata")
>> Link: http://lkml.kernel.org/r/20190315022649.17848-1-yuehaibing@huawei.com
>> [ split from a larger patch ]
>> Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
>> ---
>>  tools/perf/util/cs-etm.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/tools/perf/util/cs-etm.c b/tools/perf/util/cs-etm.c
>> index 2257ac4dbff2..111f33cd1204 100644
>> --- a/tools/perf/util/cs-etm.c
>> +++ b/tools/perf/util/cs-etm.c
>> @@ -1908,7 +1908,7 @@ int cs_etm__process_auxtrace_info(union perf_event *event,
>>  
>>  		/* Something went wrong, no need to continue */
>>  		if (!inode) {
>> -			err = PTR_ERR(inode);
>> +			err = -ENOMEM;
>>  			goto err_free_metadata;
>>  		}
>>  
>> -- 
>> 2.20.1
>>
> 
> .
> 


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* Re: [PATCH] perf tools: return errcode in cs_etm__process_auxtrace_info
  2019-03-19 14:55         ` Mathieu Poirier
@ 2019-03-19 16:53           ` Arnaldo Carvalho de Melo
  -1 siblings, 0 replies; 16+ messages in thread
From: Arnaldo Carvalho de Melo @ 2019-03-19 16:53 UTC (permalink / raw)
  To: Mathieu Poirier
  Cc: Arnaldo Carvalho de Melo, Yue Haibing, Suzuki K. Poulose,
	Peter Zijlstra, Ingo Molnar, Alexander Shishkin, Jiri Olsa,
	Namhyung Kim, Linux Kernel Mailing List, linux-arm-kernel

Em Tue, Mar 19, 2019 at 08:55:25AM -0600, Mathieu Poirier escreveu:
> On Tue, Mar 19, 2019 at 11:46:31AM -0300, Arnaldo Carvalho de Melo wrote:
> > Em Tue, Mar 19, 2019 at 08:38:32AM -0600, Mathieu Poirier escreveu:
> > > On Mon, 18 Mar 2019 at 11:15, Arnaldo Carvalho de Melo
> > > <arnaldo.melo@gmail.com> wrote:
> > > >
> > > > Em Fri, Mar 15, 2019 at 10:26:49AM +0800, Yue Haibing escreveu:
> > > > > From: YueHaibing <yuehaibing@huawei.com>
> > > > >
> > > > > 'err' is set in err path, but it's not returned to callers.
> > > > > Also fix a pass zero to PTR_ERR issue.
> > > >
> > > > Next time please submit two patches, one for the PTR_ERR and another for
> > > > not throwing away the err = -E!INVAL and returning just -EINVAL, I'm
> > > > doing it this time.
> > > >
> > > 
> > > Please hold off on that - I've asked for other modifications to be
> > > done on this patch.
> > 
> > Do you really think that it is necessary to hold? The fixes are trivial
> > and I already have them split and applied, see them below, I think
> > whatever other changes can be done in further patches, no?
> 
> Proceeding with the patches below would created two new bugs, hence asking Yue
> for modifications.

Ok, I'll take your word on it then, dropping the patches.

- Arnaldo
 
> Mathieu  
> 
> > 
> > - Arnaldo
> > 
> > From 6c5c248935a4da12a582b1518a38ec35044833ee Mon Sep 17 00:00:00 2001
> > From: YueHaibing <yuehaibing@huawei.com>
> > Date: Fri, 15 Mar 2019 10:26:49 +0800
> > Subject: [PATCH 24/42] perf cs-etm: return errcode in
> >  cs_etm__process_auxtrace_info()
> > 
> > 'err' is set in err path, but it's not returned to callers. Don't always return
> > -EINVAL, return err.
> > 
> > Signed-off-by: YueHaibing <yuehaibing@huawei.com>
> > Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
> > Cc: Jiri Olsa <jolsa@redhat.com>
> > Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
> > Cc: Namhyung Kim <namhyung@kernel.org>
> > Cc: Peter Zijlstra <peterz@infradead.org>
> > Cc: Suzuki K Poulouse <suzuki.poulose@arm.com>
> > Cc: linux-arm-kernel@lists.infradead.org
> > Fixes: cd8bfd8c973e ("perf tools: Add processing of coresight metadata")
> > Link: http://lkml.kernel.org/r/20190315022649.17848-1-yuehaibing@huawei.com
> > [ split from a larger patch ]
> > Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
> > ---
> >  tools/perf/util/cs-etm.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/tools/perf/util/cs-etm.c b/tools/perf/util/cs-etm.c
> > index 110804936fc3..2257ac4dbff2 100644
> > --- a/tools/perf/util/cs-etm.c
> > +++ b/tools/perf/util/cs-etm.c
> > @@ -2023,5 +2023,5 @@ int cs_etm__process_auxtrace_info(union perf_event *event,
> >  err_free_hdr:
> >  	zfree(&hdr);
> >  
> > -	return -EINVAL;
> > +	return err;
> >  }
> > -- 
> > 2.20.1
> > 
> > From 4ab1e71e45a91220bdca7fc3b79c600df81d9234 Mon Sep 17 00:00:00 2001
> > From: YueHaibing <yuehaibing@huawei.com>
> > Date: Fri, 15 Mar 2019 10:26:49 +0800
> > Subject: [PATCH 25/42] perf cs-etm: Remove errnoeous ERR_PTR() usage in in
> >  cs_etm__process_auxtrace_info
> > 
> > intlist__findnew() doesn't uses ERR_PTR() as a return mechanism so its callers
> > shouldn't try to extract the error using PTR_ERR(ret-from-intlist__findnew()),
> > make cs_etm__process_auxtrace_info9) return -ENOMEM instead.
> > 
> > Signed-off-by: YueHaibing <yuehaibing@huawei.com>
> > Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
> > Cc: Jiri Olsa <jolsa@redhat.com>
> > Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
> > Cc: Namhyung Kim <namhyung@kernel.org>
> > Cc: Peter Zijlstra <peterz@infradead.org>
> > Cc: Suzuki K Poulouse <suzuki.poulose@arm.com>
> > Cc: linux-arm-kernel@lists.infradead.org
> > Fixes: cd8bfd8c973e ("perf tools: Add processing of coresight metadata")
> > Link: http://lkml.kernel.org/r/20190315022649.17848-1-yuehaibing@huawei.com
> > [ split from a larger patch ]
> > Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
> > ---
> >  tools/perf/util/cs-etm.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/tools/perf/util/cs-etm.c b/tools/perf/util/cs-etm.c
> > index 2257ac4dbff2..111f33cd1204 100644
> > --- a/tools/perf/util/cs-etm.c
> > +++ b/tools/perf/util/cs-etm.c
> > @@ -1908,7 +1908,7 @@ int cs_etm__process_auxtrace_info(union perf_event *event,
> >  
> >  		/* Something went wrong, no need to continue */
> >  		if (!inode) {
> > -			err = PTR_ERR(inode);
> > +			err = -ENOMEM;
> >  			goto err_free_metadata;
> >  		}
> >  
> > -- 
> > 2.20.1
> > 

-- 

- Arnaldo

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

* Re: [PATCH] perf tools: return errcode in cs_etm__process_auxtrace_info
@ 2019-03-19 16:53           ` Arnaldo Carvalho de Melo
  0 siblings, 0 replies; 16+ messages in thread
From: Arnaldo Carvalho de Melo @ 2019-03-19 16:53 UTC (permalink / raw)
  To: Mathieu Poirier
  Cc: Suzuki K. Poulose, Peter Zijlstra, Yue Haibing,
	Arnaldo Carvalho de Melo, Linux Kernel Mailing List,
	Alexander Shishkin, Ingo Molnar, Namhyung Kim, Jiri Olsa,
	linux-arm-kernel

Em Tue, Mar 19, 2019 at 08:55:25AM -0600, Mathieu Poirier escreveu:
> On Tue, Mar 19, 2019 at 11:46:31AM -0300, Arnaldo Carvalho de Melo wrote:
> > Em Tue, Mar 19, 2019 at 08:38:32AM -0600, Mathieu Poirier escreveu:
> > > On Mon, 18 Mar 2019 at 11:15, Arnaldo Carvalho de Melo
> > > <arnaldo.melo@gmail.com> wrote:
> > > >
> > > > Em Fri, Mar 15, 2019 at 10:26:49AM +0800, Yue Haibing escreveu:
> > > > > From: YueHaibing <yuehaibing@huawei.com>
> > > > >
> > > > > 'err' is set in err path, but it's not returned to callers.
> > > > > Also fix a pass zero to PTR_ERR issue.
> > > >
> > > > Next time please submit two patches, one for the PTR_ERR and another for
> > > > not throwing away the err = -E!INVAL and returning just -EINVAL, I'm
> > > > doing it this time.
> > > >
> > > 
> > > Please hold off on that - I've asked for other modifications to be
> > > done on this patch.
> > 
> > Do you really think that it is necessary to hold? The fixes are trivial
> > and I already have them split and applied, see them below, I think
> > whatever other changes can be done in further patches, no?
> 
> Proceeding with the patches below would created two new bugs, hence asking Yue
> for modifications.

Ok, I'll take your word on it then, dropping the patches.

- Arnaldo
 
> Mathieu  
> 
> > 
> > - Arnaldo
> > 
> > From 6c5c248935a4da12a582b1518a38ec35044833ee Mon Sep 17 00:00:00 2001
> > From: YueHaibing <yuehaibing@huawei.com>
> > Date: Fri, 15 Mar 2019 10:26:49 +0800
> > Subject: [PATCH 24/42] perf cs-etm: return errcode in
> >  cs_etm__process_auxtrace_info()
> > 
> > 'err' is set in err path, but it's not returned to callers. Don't always return
> > -EINVAL, return err.
> > 
> > Signed-off-by: YueHaibing <yuehaibing@huawei.com>
> > Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
> > Cc: Jiri Olsa <jolsa@redhat.com>
> > Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
> > Cc: Namhyung Kim <namhyung@kernel.org>
> > Cc: Peter Zijlstra <peterz@infradead.org>
> > Cc: Suzuki K Poulouse <suzuki.poulose@arm.com>
> > Cc: linux-arm-kernel@lists.infradead.org
> > Fixes: cd8bfd8c973e ("perf tools: Add processing of coresight metadata")
> > Link: http://lkml.kernel.org/r/20190315022649.17848-1-yuehaibing@huawei.com
> > [ split from a larger patch ]
> > Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
> > ---
> >  tools/perf/util/cs-etm.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/tools/perf/util/cs-etm.c b/tools/perf/util/cs-etm.c
> > index 110804936fc3..2257ac4dbff2 100644
> > --- a/tools/perf/util/cs-etm.c
> > +++ b/tools/perf/util/cs-etm.c
> > @@ -2023,5 +2023,5 @@ int cs_etm__process_auxtrace_info(union perf_event *event,
> >  err_free_hdr:
> >  	zfree(&hdr);
> >  
> > -	return -EINVAL;
> > +	return err;
> >  }
> > -- 
> > 2.20.1
> > 
> > From 4ab1e71e45a91220bdca7fc3b79c600df81d9234 Mon Sep 17 00:00:00 2001
> > From: YueHaibing <yuehaibing@huawei.com>
> > Date: Fri, 15 Mar 2019 10:26:49 +0800
> > Subject: [PATCH 25/42] perf cs-etm: Remove errnoeous ERR_PTR() usage in in
> >  cs_etm__process_auxtrace_info
> > 
> > intlist__findnew() doesn't uses ERR_PTR() as a return mechanism so its callers
> > shouldn't try to extract the error using PTR_ERR(ret-from-intlist__findnew()),
> > make cs_etm__process_auxtrace_info9) return -ENOMEM instead.
> > 
> > Signed-off-by: YueHaibing <yuehaibing@huawei.com>
> > Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
> > Cc: Jiri Olsa <jolsa@redhat.com>
> > Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
> > Cc: Namhyung Kim <namhyung@kernel.org>
> > Cc: Peter Zijlstra <peterz@infradead.org>
> > Cc: Suzuki K Poulouse <suzuki.poulose@arm.com>
> > Cc: linux-arm-kernel@lists.infradead.org
> > Fixes: cd8bfd8c973e ("perf tools: Add processing of coresight metadata")
> > Link: http://lkml.kernel.org/r/20190315022649.17848-1-yuehaibing@huawei.com
> > [ split from a larger patch ]
> > Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
> > ---
> >  tools/perf/util/cs-etm.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/tools/perf/util/cs-etm.c b/tools/perf/util/cs-etm.c
> > index 2257ac4dbff2..111f33cd1204 100644
> > --- a/tools/perf/util/cs-etm.c
> > +++ b/tools/perf/util/cs-etm.c
> > @@ -1908,7 +1908,7 @@ int cs_etm__process_auxtrace_info(union perf_event *event,
> >  
> >  		/* Something went wrong, no need to continue */
> >  		if (!inode) {
> > -			err = PTR_ERR(inode);
> > +			err = -ENOMEM;
> >  			goto err_free_metadata;
> >  		}
> >  
> > -- 
> > 2.20.1
> > 

-- 

- Arnaldo

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

end of thread, other threads:[~2019-03-19 16:54 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-03-15  2:26 [PATCH] perf tools: return errcode in cs_etm__process_auxtrace_info Yue Haibing
2019-03-15  2:26 ` Yue Haibing
2019-03-18 17:13 ` Mathieu Poirier
2019-03-18 17:13   ` Mathieu Poirier
2019-03-18 17:15 ` Arnaldo Carvalho de Melo
2019-03-18 17:15   ` Arnaldo Carvalho de Melo
2019-03-19 14:38   ` Mathieu Poirier
2019-03-19 14:38     ` Mathieu Poirier
2019-03-19 14:46     ` Arnaldo Carvalho de Melo
2019-03-19 14:46       ` Arnaldo Carvalho de Melo
2019-03-19 14:55       ` Mathieu Poirier
2019-03-19 14:55         ` Mathieu Poirier
2019-03-19 15:12         ` YueHaibing
2019-03-19 15:12           ` YueHaibing
2019-03-19 16:53         ` Arnaldo Carvalho de Melo
2019-03-19 16:53           ` 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.