2014-04-15 17:13:20

by Cody P Schafer

[permalink] [raw]
Subject: [PATCH v2 0/6] powerpc/perf/hv_{gpci,24x7}: fixes

- 24x7 and gpci probing now uses pr_debug() and doesn't pad to 80 characters
- Catalog access is fixed for LE kernels
- remove c99 feature sparse doesn't like
- 1 device attr made static


Cody P Schafer (6):
powerpc/perf/hv_24x7: probe errors changed to pr_debug(), padding
fixed
powerpc/perf/hv_gpci: probe failures use pr_debug(), and padding
reduced
powerpc/perf/hv-gpci: make device attr static
powerpc/perf/hv-24x7: use (unsigned long) not (u32) values when
calling plpar_hcall_norets()
powerpc/perf/hv-24x7: remove [static 4096], sparse chokes on it
powerpc/perf/hv-24x7: catalog version number is be64, not be32

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

--
1.9.2


2014-04-15 17:11:21

by Cody P Schafer

[permalink] [raw]
Subject: [PATCH v2 6/6] powerpc/perf/hv-24x7: catalog version number is be64, not be32

The catalog version number was changed from a be32 (with proceeding
32bits of padding) to a be64, update the code to treat it as a be64

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

diff --git a/arch/powerpc/perf/hv-24x7.c b/arch/powerpc/perf/hv-24x7.c
index 95a67f8..9d4badc 100644
--- a/arch/powerpc/perf/hv-24x7.c
+++ b/arch/powerpc/perf/hv-24x7.c
@@ -171,7 +171,7 @@ static unsigned long h_get_24x7_catalog_page_(unsigned long phys_4096,
}

static unsigned long h_get_24x7_catalog_page(char page[],
- u32 version, u32 index)
+ u64 version, u32 index)
{
return h_get_24x7_catalog_page_(virt_to_phys(page),
version, index);
@@ -185,7 +185,7 @@ static ssize_t catalog_read(struct file *filp, struct kobject *kobj,
ssize_t ret = 0;
size_t catalog_len = 0, catalog_page_len = 0, page_count = 0;
loff_t page_offset = 0;
- uint32_t catalog_version_num = 0;
+ uint64_t catalog_version_num = 0;
void *page = kmem_cache_alloc(hv_page_cache, GFP_USER);
struct hv_24x7_catalog_page_0 *page_0 = page;
if (!page)
@@ -197,7 +197,7 @@ static ssize_t catalog_read(struct file *filp, struct kobject *kobj,
goto e_free;
}

- catalog_version_num = be32_to_cpu(page_0->version);
+ catalog_version_num = be64_to_cpu(page_0->version);
catalog_page_len = be32_to_cpu(page_0->length);
catalog_len = catalog_page_len * 4096;

@@ -255,7 +255,7 @@ e_free: \
static DEVICE_ATTR_RO(_name)

PAGE_0_ATTR(catalog_version, "%lld\n",
- (unsigned long long)be32_to_cpu(page_0->version));
+ (unsigned long long)be64_to_cpu(page_0->version));
PAGE_0_ATTR(catalog_len, "%lld\n",
(unsigned long long)be32_to_cpu(page_0->length) * 4096);
static BIN_ATTR_RO(catalog, 0/* real length varies */);
--
1.9.2

2014-04-15 17:11:19

by Cody P Schafer

[permalink] [raw]
Subject: [PATCH v2 1/6] powerpc/perf/hv_24x7: probe errors changed to pr_debug(), padding fixed

fixup for "powerpc/perf: Add support for the hv 24x7 interface"

Makes the "not enabled" message less awful (and hides it in most cases).

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

diff --git a/arch/powerpc/perf/hv-24x7.c b/arch/powerpc/perf/hv-24x7.c
index 297c9105..f5bca73 100644
--- a/arch/powerpc/perf/hv-24x7.c
+++ b/arch/powerpc/perf/hv-24x7.c
@@ -485,13 +485,13 @@ static int hv_24x7_init(void)
struct hv_perf_caps caps;

if (!firmware_has_feature(FW_FEATURE_LPAR)) {
- pr_info("not a virtualized system, not enabling\n");
+ pr_debug("not a virtualized system, not enabling\n");
return -ENODEV;
}

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

2014-04-15 17:11:59

by Cody P Schafer

[permalink] [raw]
Subject: [PATCH v2 3/6] powerpc/perf/hv-gpci: make device attr static

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 8fee1dc..c9d399a 100644
--- a/arch/powerpc/perf/hv-gpci.c
+++ b/arch/powerpc/perf/hv-gpci.c
@@ -78,7 +78,7 @@ static ssize_t kernel_version_show(struct device *dev,
return sprintf(page, "0x%x\n", COUNTER_INFO_VERSION_CURRENT);
}

-DEVICE_ATTR_RO(kernel_version);
+static DEVICE_ATTR_RO(kernel_version);
HV_CAPS_ATTR(version, "0x%x\n");
HV_CAPS_ATTR(ga, "%d\n");
HV_CAPS_ATTR(expanded, "%d\n");
--
1.9.2

2014-04-15 17:11:57

by Cody P Schafer

[permalink] [raw]
Subject: [PATCH v2 5/6] powerpc/perf/hv-24x7: remove [static 4096], sparse chokes on it

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 3e8f60a..95a67f8 100644
--- a/arch/powerpc/perf/hv-24x7.c
+++ b/arch/powerpc/perf/hv-24x7.c
@@ -170,7 +170,7 @@ static unsigned long h_get_24x7_catalog_page_(unsigned long phys_4096,
index);
}

-static unsigned long h_get_24x7_catalog_page(char page[static 4096],
+static unsigned long h_get_24x7_catalog_page(char page[],
u32 version, u32 index)
{
return h_get_24x7_catalog_page_(virt_to_phys(page),
--
1.9.2

2014-04-15 17:11:18

by Cody P Schafer

[permalink] [raw]
Subject: [PATCH v2 4/6] powerpc/perf/hv-24x7: use (unsigned long) not (u32) values when calling plpar_hcall_norets()

Signed-off-by: Cody P Schafer <[email protected]>
---
arch/powerpc/perf/hv-24x7.c | 20 ++++++++++++++++----
1 file changed, 16 insertions(+), 4 deletions(-)

diff --git a/arch/powerpc/perf/hv-24x7.c b/arch/powerpc/perf/hv-24x7.c
index f5bca73..3e8f60a 100644
--- a/arch/powerpc/perf/hv-24x7.c
+++ b/arch/powerpc/perf/hv-24x7.c
@@ -155,16 +155,28 @@ static ssize_t read_offset_data(void *dest, size_t dest_len,
return copy_len;
}

-static unsigned long h_get_24x7_catalog_page(char page[static 4096],
- u32 version, u32 index)
+static unsigned long h_get_24x7_catalog_page_(unsigned long phys_4096,
+ unsigned long version,
+ unsigned long index)
{
- WARN_ON(!IS_ALIGNED((unsigned long)page, 4096));
+ pr_devel("h_get_24x7_catalog_page(0x%lx, %lu, %lu)",
+ phys_4096,
+ version,
+ index);
+ WARN_ON(!IS_ALIGNED(phys_4096, 4096));
return plpar_hcall_norets(H_GET_24X7_CATALOG_PAGE,
- virt_to_phys(page),
+ phys_4096,
version,
index);
}

+static unsigned long h_get_24x7_catalog_page(char page[static 4096],
+ u32 version, u32 index)
+{
+ return h_get_24x7_catalog_page_(virt_to_phys(page),
+ version, index);
+}
+
static ssize_t catalog_read(struct file *filp, struct kobject *kobj,
struct bin_attribute *bin_attr, char *buf,
loff_t offset, size_t count)
--
1.9.2

2014-04-15 17:12:56

by Cody P Schafer

[permalink] [raw]
Subject: [PATCH v2 2/6] powerpc/perf/hv_gpci: probe failures use pr_debug(), and padding reduced

fixup for "powerpc/perf: Add support for the hv gpci (get performance
counter info) interface".

Makes the "not enabled" message less awful (and hidden unless
debugging).

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

diff --git a/arch/powerpc/perf/hv-gpci.c b/arch/powerpc/perf/hv-gpci.c
index 278ba7b..8fee1dc 100644
--- a/arch/powerpc/perf/hv-gpci.c
+++ b/arch/powerpc/perf/hv-gpci.c
@@ -273,13 +273,13 @@ static int hv_gpci_init(void)
struct hv_perf_caps caps;

if (!firmware_has_feature(FW_FEATURE_LPAR)) {
- pr_info("not a virtualized system, not enabling\n");
+ pr_debug("not a virtualized system, not enabling\n");
return -ENODEV;
}

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

2014-04-28 04:48:32

by Benjamin Herrenschmidt

[permalink] [raw]
Subject: Re: [PATCH v2 6/6] powerpc/perf/hv-24x7: catalog version number is be64, not be32

On Tue, 2014-04-15 at 10:10 -0700, Cody P Schafer wrote:
> The catalog version number was changed from a be32 (with proceeding
> 32bits of padding) to a be64, update the code to treat it as a be64
>
> Signed-off-by: Cody P Schafer <[email protected]>
> --

Have you tested this ?

It doesn't build for me:

arch/powerpc/perf/hv-24x7.c: In function 'catalog_read':
arch/powerpc/perf/hv-24x7.c:223:3: error: format '%d' expects argument of type 'int', but argument 2 has type 'uint64_t' [-Werror=format]
cc1: all warnings being treated as errors

I'll fix that up in my tree.

Cheers,
Ben.

> arch/powerpc/perf/hv-24x7.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/arch/powerpc/perf/hv-24x7.c b/arch/powerpc/perf/hv-24x7.c
> index 95a67f8..9d4badc 100644
> --- a/arch/powerpc/perf/hv-24x7.c
> +++ b/arch/powerpc/perf/hv-24x7.c
> @@ -171,7 +171,7 @@ static unsigned long h_get_24x7_catalog_page_(unsigned long phys_4096,
> }
>
> static unsigned long h_get_24x7_catalog_page(char page[],
> - u32 version, u32 index)
> + u64 version, u32 index)
> {
> return h_get_24x7_catalog_page_(virt_to_phys(page),
> version, index);
> @@ -185,7 +185,7 @@ static ssize_t catalog_read(struct file *filp, struct kobject *kobj,
> ssize_t ret = 0;
> size_t catalog_len = 0, catalog_page_len = 0, page_count = 0;
> loff_t page_offset = 0;
> - uint32_t catalog_version_num = 0;
> + uint64_t catalog_version_num = 0;
> void *page = kmem_cache_alloc(hv_page_cache, GFP_USER);
> struct hv_24x7_catalog_page_0 *page_0 = page;
> if (!page)
> @@ -197,7 +197,7 @@ static ssize_t catalog_read(struct file *filp, struct kobject *kobj,
> goto e_free;
> }
>
> - catalog_version_num = be32_to_cpu(page_0->version);
> + catalog_version_num = be64_to_cpu(page_0->version);
> catalog_page_len = be32_to_cpu(page_0->length);
> catalog_len = catalog_page_len * 4096;
>
> @@ -255,7 +255,7 @@ e_free: \
> static DEVICE_ATTR_RO(_name)
>
> PAGE_0_ATTR(catalog_version, "%lld\n",
> - (unsigned long long)be32_to_cpu(page_0->version));
> + (unsigned long long)be64_to_cpu(page_0->version));
> PAGE_0_ATTR(catalog_len, "%lld\n",
> (unsigned long long)be32_to_cpu(page_0->length) * 4096);
> static BIN_ATTR_RO(catalog, 0/* real length varies */);

2014-04-28 05:00:22

by Cody P Schafer

[permalink] [raw]
Subject: Re: [PATCH v2 6/6] powerpc/perf/hv-24x7: catalog version number is be64, not be32

On 04/27/2014 09:47 PM, Benjamin Herrenschmidt wrote:
> On Tue, 2014-04-15 at 10:10 -0700, Cody P Schafer wrote:
>> The catalog version number was changed from a be32 (with proceeding
>> 32bits of padding) to a be64, update the code to treat it as a be64
>>
>> Signed-off-by: Cody P Schafer <[email protected]>
>> --
>
> Have you tested this ?
>
> It doesn't build for me:
>
> arch/powerpc/perf/hv-24x7.c: In function 'catalog_read':
> arch/powerpc/perf/hv-24x7.c:223:3: error: format '%d' expects argument of type 'int', but argument 2 has type 'uint64_t' [-Werror=format]
> cc1: all warnings being treated as errors

I have, and I wasn't initially sure how I managed to miss that
warning-as-error. On examination: My config (for some reason) has
CONFIG_PPC_DISABLE_WERROR=y set (probably because it's a variation of a
distro config). Must have been piping the warnings to a file and
forgotten to check the file.

> I'll fix that up in my tree.

Thanks.

2014-04-29 03:12:50

by Michael Ellerman

[permalink] [raw]
Subject: Re: [PATCH v2 6/6] powerpc/perf/hv-24x7: catalog version number is be64, not be32

On Sun, 2014-04-27 at 21:59 -0700, Cody P Schafer wrote:
> On 04/27/2014 09:47 PM, Benjamin Herrenschmidt wrote:
> > On Tue, 2014-04-15 at 10:10 -0700, Cody P Schafer wrote:
> >> The catalog version number was changed from a be32 (with proceeding
> >> 32bits of padding) to a be64, update the code to treat it as a be64
> >>
> >> Signed-off-by: Cody P Schafer <[email protected]>
> >> --
> >
> > Have you tested this ?
> >
> > It doesn't build for me:
> >
> > arch/powerpc/perf/hv-24x7.c: In function 'catalog_read':
> > arch/powerpc/perf/hv-24x7.c:223:3: error: format '%d' expects argument of type 'int', but argument 2 has type 'uint64_t' [-Werror=format]
> > cc1: all warnings being treated as errors
>
> I have, and I wasn't initially sure how I managed to miss that
> warning-as-error. On examination: My config (for some reason) has
> CONFIG_PPC_DISABLE_WERROR=y set (probably because it's a variation of a
> distro config).

Please test build with ppc64_defconfig at least.

cheers