[PATCH] powerpc/perf/hv-gpci: Increase request buffer size

Subsystems: linux for powerpc (32-bit and 64-bit), the rest

STALE3837d

3 messages, 2 authors, 2016-02-10 · open the first message on its own page

[PATCH] powerpc/perf/hv-gpci: Increase request buffer size

From: Sukadev Bhattiprolu <hidden>
Date: 2016-02-09 03:08:49

From 31edd352fb7c2a72913f1977fa1bf168109089ad Mon Sep 17 00:00:00 2001

From: Sukadev Bhattiprolu <redacted>
Date: Tue, 9 Feb 2016 02:47:45 -0500
Subject: [PATCH] powerpc/perf/hv-gpci: Increase request buffer size

The GPCI hcall allows for a 4K buffer but we limit the buffer
to 1K. The problem with a 1K buffer is if a request results in
returning more values than can be accomodated in the 1K buffer
the request will fail.

The buffer we are using is currently allocated on the stack and
hence limited in size. Instead use a per-CPU 4K buffer like we do
with 24x7 counters (hv-24x7.c).

Signed-off-by: Sukadev Bhattiprolu <redacted>
---
 arch/powerpc/perf/hv-gpci.c | 37 ++++++++++++++++++++++---------------
 1 file changed, 22 insertions(+), 15 deletions(-)

diff --git a/arch/powerpc/perf/hv-gpci.c b/arch/powerpc/perf/hv-gpci.c
index 856fe6e..e6fad73 100644
--- a/arch/powerpc/perf/hv-gpci.c
+++ b/arch/powerpc/perf/hv-gpci.c

@@ -127,8 +127,16 @@ static const struct attribute_group *attr_groups[] = {
 	NULL,
 };
 
+#define HGPCI_REQ_BUFFER_SIZE	4096
 #define GPCI_MAX_DATA_BYTES \
-	(1024 - sizeof(struct hv_get_perf_counter_info_params))
+	(HGPCI_REQ_BUFFER_SIZE - sizeof(struct hv_get_perf_counter_info_params))
+
+DEFINE_PER_CPU(char, hv_gpci_reqb[HGPCI_REQ_BUFFER_SIZE]) __aligned(sizeof(uint64_t));
+
+struct hv_gpci_request_buffer {
+	struct hv_get_perf_counter_info_params params;
+	uint8_t bytes[1];
+} __packed;
 
 static unsigned long single_gpci_request(u32 req, u32 starting_index,
 		u16 secondary_index, u8 version_in, u32 offset, u8 length,

@@ -137,24 +145,21 @@ static unsigned long single_gpci_request(u32 req, u32 starting_index,
 	unsigned long ret;
 	size_t i;
 	u64 count;
+	struct hv_gpci_request_buffer *arg;
+
+	arg = (void *)get_cpu_var(hv_gpci_reqb);
+	memset(arg, 0, HGPCI_REQ_BUFFER_SIZE);
 
-	struct {
-		struct hv_get_perf_counter_info_params params;
-		uint8_t bytes[GPCI_MAX_DATA_BYTES];
-	} __packed __aligned(sizeof(uint64_t)) arg = {
-		.params = {
-			.counter_request = cpu_to_be32(req),
-			.starting_index = cpu_to_be32(starting_index),
-			.secondary_index = cpu_to_be16(secondary_index),
-			.counter_info_version_in = version_in,
-		}
-	};
+	arg->params.counter_request = cpu_to_be32(req);
+	arg->params.starting_index = cpu_to_be32(starting_index);
+	arg->params.secondary_index = cpu_to_be16(secondary_index);
+	arg->params.counter_info_version_in = version_in;
 
 	ret = plpar_hcall_norets(H_GET_PERF_COUNTER_INFO,
-			virt_to_phys(&arg), sizeof(arg));
+			virt_to_phys(arg), HGPCI_REQ_BUFFER_SIZE);
 	if (ret) {
 		pr_devel("hcall failed: 0x%lx\n", ret);
-		return ret;
+		goto out;
 	}
 
 	/*

@@ -163,9 +168,11 @@ static unsigned long single_gpci_request(u32 req, u32 starting_index,
 	 */
 	count = 0;
 	for (i = offset; i < offset + length; i++)
-		count |= arg.bytes[i] << (i - offset);
+		count |= arg->bytes[i] << (i - offset);
 
 	*value = count;
+out:
+	put_cpu_var(hv_gpci_reqb);
 	return ret;
 }

-- 
1.8.3.1

Re: powerpc/perf/hv-gpci: Increase request buffer size

From: Michael Ellerman <mpe@ellerman.id.au>
Date: 2016-02-09 09:45:23

On Tue, 2016-09-02 at 03:08:30 UTC, Sukadev Bhattiprolu wrote:

quoted hunk

quoted

From 31edd352fb7c2a72913f1977fa1bf168109089ad Mon Sep 17 00:00:00 2001

From: Sukadev Bhattiprolu <redacted>
Date: Tue, 9 Feb 2016 02:47:45 -0500
Subject: [PATCH] powerpc/perf/hv-gpci: Increase request buffer size

The GPCI hcall allows for a 4K buffer but we limit the buffer
to 1K. The problem with a 1K buffer is if a request results in
returning more values than can be accomodated in the 1K buffer
the request will fail.

The buffer we are using is currently allocated on the stack and
hence limited in size. Instead use a per-CPU 4K buffer like we do
with 24x7 counters (hv-24x7.c).

diff --git a/arch/powerpc/perf/hv-gpci.c b/arch/powerpc/perf/hv-gpci.c
index 856fe6e..e6fad73 100644
--- a/arch/powerpc/perf/hv-gpci.c
+++ b/arch/powerpc/perf/hv-gpci.c

@@ -127,8 +127,16 @@ static const struct attribute_group *attr_groups[] = {
 	NULL,
 };
 
+#define HGPCI_REQ_BUFFER_SIZE	4096
 #define GPCI_MAX_DATA_BYTES \
-	(1024 - sizeof(struct hv_get_perf_counter_info_params))
+	(HGPCI_REQ_BUFFER_SIZE - sizeof(struct hv_get_perf_counter_info_params))
+
+DEFINE_PER_CPU(char, hv_gpci_reqb[HGPCI_REQ_BUFFER_SIZE]) __aligned(sizeof(uint64_t));
+
+struct hv_gpci_request_buffer {
+	struct hv_get_perf_counter_info_params params;
+	uint8_t bytes[1];

bytes is 1 byte long, but ..

quoted hunk

@@ -163,9 +168,11 @@ static unsigned long single_gpci_request(u32 req, u32 starting_index,
 	 */
 	count = 0;
 	for (i = offset; i < offset + length; i++)
-		count |= arg.bytes[i] << (i - offset);
+		count |= arg->bytes[i] << (i - offset);


Here you read from bytes[i] where i can be > 1 (AFAICS).

That's fishy at best, and newer GCCs just don't allow it.

I think you could do this and it would work, but untested:

   struct hv_gpci_request_buffer {
   	struct hv_get_perf_counter_info_params params;
   	uint8_t bytes[4096 - sizeof(struct hv_get_perf_counter_info_parms)];
   };

cheers

Re: powerpc/perf/hv-gpci: Increase request buffer size

From: Sukadev Bhattiprolu <hidden>
Date: 2016-02-10 03:14:31

Michael Ellerman [mpe@ellerman.id.au] wrote:

Here you read from bytes[i] where i can be > 1 (AFAICS).

Yes, buffer is large enough and I thought this construct of
array was used in a several places. Maybe they are being
changed out now (struct pid has one such usage).

That's fishy at best, and newer GCCs just don't allow it.

Ah, ok.

I think you could do this and it would work, but untested:

   struct hv_gpci_request_buffer {
   	struct hv_get_perf_counter_info_params params;
   	uint8_t bytes[4096 - sizeof(struct hv_get_perf_counter_info_parms)];

There is a macro for this computation in that file. I could have
used that. Will change it and repost.

Thanks,

Sukadev

`h`	back out one level
`j`	next message in thread
`k`	previous message in thread
`l`	drill in
`Esc`	close help / fold thread tree
`?`	toggle this help