2014-04-02 22:11:07

by Cody P Schafer

[permalink] [raw]
Subject: [PATCH 0/2] powerpc/perf: fixup 2 patches from the 24x7 series

mpe: these are fixups for 2 patches already in your merge tree (and in benh's next branch).

f3e622941a7cec587c00c0d17ea31514457c63c8 powerpc/perf: Add support for the hv 24x7 interface
edd354ea4a6774bf9f380b0acf30e699070f4e8a powerpc/perf: Add support for the hv gpci (get performance counter info) interface

The only change is to a pr_info() printed when the interface is not detected.

Anton: I'm hesitant to switch these to pr_debug() as they are the only way
users expecting these PMUs to exist to tell why the kernel decided they didn't
have them. As a result, I've kept them as pr_info() instead of converting to
pr_debug().


Cody P Schafer (2):
fixup: "powerpc/perf: Add support for the hv 24x7 interface"
fixup: "powerpc/perf: Add support for the hv gpci (get performance
counter info) interface"

arch/powerpc/perf/hv-24x7.c | 2 +-
arch/powerpc/perf/hv-gpci.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)

--
1.9.1


2014-04-02 22:11:05

by Cody P Schafer

[permalink] [raw]
Subject: [PATCH 2/2] fixup: "powerpc/perf: Add support for the hv gpci (get performance counter info) interface"

Make the "not enabled" message less awful.

Signed-off-by: Cody P Schafer <[email protected]>
---
arch/powerpc/perf/hv-gpci.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/powerpc/perf/hv-gpci.c b/arch/powerpc/perf/hv-gpci.c
index 278ba7b..f6c471d 100644
--- a/arch/powerpc/perf/hv-gpci.c
+++ b/arch/powerpc/perf/hv-gpci.c
@@ -279,7 +279,7 @@ static int hv_gpci_init(void)

hret = hv_perf_caps_get(&caps);
if (hret) {
- pr_info("could not obtain capabilities, error 0x%80lx, not enabling\n",
+ pr_info("could not obtain capabilities, not enabling (%ld)\n",
hret);
return -ENODEV;
}
--
1.9.1

2014-04-02 22:12:09

by Cody P Schafer

[permalink] [raw]
Subject: [PATCH 1/2] fixup: "powerpc/perf: Add support for the hv 24x7 interface"

Make the "not enabled" message less awful.

Signed-off-by: Cody P Schafer <[email protected]>
---
arch/powerpc/perf/hv-24x7.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/powerpc/perf/hv-24x7.c b/arch/powerpc/perf/hv-24x7.c
index 297c9105..3246ea2 100644
--- a/arch/powerpc/perf/hv-24x7.c
+++ b/arch/powerpc/perf/hv-24x7.c
@@ -491,7 +491,7 @@ static int hv_24x7_init(void)

hret = hv_perf_caps_get(&caps);
if (hret) {
- pr_info("could not obtain capabilities, error 0x%80lx, not enabling\n",
+ pr_info("could not obtain capabilities, not enabling (%ld)\n",
hret);
return -ENODEV;
}
--
1.9.1

2014-04-06 23:49:03

by Benjamin Herrenschmidt

[permalink] [raw]
Subject: Re: [PATCH 1/2] fixup: "powerpc/perf: Add support for the hv 24x7 interface"

On Wed, 2014-04-02 at 15:10 -0700, Cody P Schafer wrote:
> Make the "not enabled" message less awful.
>
> Signed-off-by: Cody P Schafer <[email protected]>
> ---
> arch/powerpc/perf/hv-24x7.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/powerpc/perf/hv-24x7.c b/arch/powerpc/perf/hv-24x7.c
> index 297c9105..3246ea2 100644
> --- a/arch/powerpc/perf/hv-24x7.c
> +++ b/arch/powerpc/perf/hv-24x7.c
> @@ -491,7 +491,7 @@ static int hv_24x7_init(void)
>
> hret = hv_perf_caps_get(&caps);
> if (hret) {
> - pr_info("could not obtain capabilities, error 0x%80lx, not enabling\n",
> + pr_info("could not obtain capabilities, not enabling (%ld)\n",
> hret);

That's not much less awful... The message gives 0 informations about
where it comes from .. what is it that couldn't obtain capabilities ?

Also, sentences start with a Capital.

Cheers,
Ben.

2014-04-07 01:22:40

by Anton Blanchard

[permalink] [raw]
Subject: Re: [PATCH 1/2] fixup: "powerpc/perf: Add support for the hv 24x7 interface"

Hi

> That's not much less awful... The message gives 0 informations about
> where it comes from .. what is it that couldn't obtain capabilities ?

Cody is at least using pr_fmt to give some information on what went
wrong:

[ 1.635040] hv-24x7: not a virtualized system, not enabling
[ 1.635079] hv-gpci: not a virtualized system, not enabling

I think this is way too chatty though. If every driver printed a couple
of lines we'd end up with thousands of useless lines in the dmesg.

I still would prefer to see these as pr_debug lines.

Anton