* [PATCH 1/2] powerpc/perf/imc: use cpu_to_node instead topology_physical_package_id
@ 2017-10-15 18:43 Madhavan Srinivasan
2017-10-15 18:43 ` [PATCH 2/2] powerpc/perf/imc: use NUMA_NO_NODE for alloc_pages_node Madhavan Srinivasan
2017-11-24 9:46 ` [1/2] powerpc/perf/imc: use cpu_to_node instead topology_physical_package_id Michael Ellerman
0 siblings, 2 replies; 5+ messages in thread
From: Madhavan Srinivasan @ 2017-10-15 18:43 UTC (permalink / raw)
To: mpe; +Cc: linuxppc-dev, Madhavan Srinivasan
...
[1069001518.000000] [c000003f95b3f770] [c0000000000b2574] init_imc_pmu+0x1f4/0xc40
[1069005374.000000] [c000003f95b3f850] [c00000000008fec8] opal_imc_counters_probe+0x2e8/0x3e0
[1069009426.000000] [c000003f95b3f950] [c0000000006153a4] platform_drv_probe+0x44/0x90
[1069012818.000000] [c000003f95b3f9c0] [c0000000006124c0] really_probe+0x290/0x370
[1069016302.000000] [c000003f95b3fa50] [c0000000006126c8] __driver_attach+0x128/0x130
[1069019564.000000] [c000003f95b3fa90] [c00000000060f38c] bus_for_each_dev+0x9c/0x110
[1069022838.000000] [c000003f95b3fae0] [c000000000611dfc] driver_attach+0x3c/0x60
[1069026104.000000] [c000003f95b3fb10] [c0000000006118d8] bus_add_driver+0x298/0x320
[1069029428.000000] [c000003f95b3fb90] [c0000000006135b8] driver_register+0xb8/0x1a0
[1069033016.000000] [c000003f95b3fc00] [c00000000061533c] __platform_driver_register+0x8c/0xb0
[1069036362.000000] [c000003f95b3fc30] [c000000000cbb0fc] opal_imc_driver_init+0x24/0x38
[1069039756.000000] [c000003f95b3fc50] [c00000000000cc70] do_one_initcall+0xd0/0x250
[1069043094.000000] [c000003f95b3fd20] [c000000000ca44d0] kernel_init_freeable+0x244/0x324
[1069046490.000000] [c000003f95b3fdc0] [c00000000000d600] kernel_init+0x30/0x1b0
[1069050348.000000] [c000003f95b3fe30] [c00000000000b268] ret_from_kernel_thread+0x5c/0x74
init_imc_pmu() use topology_physical_package_id() to detect the phy_id
of the processor it is on to get local memory. But this cause crashes
when node_id are not same as physicaly id. As a fix use cpu_to_node().
Reported-By: Rob Lippert <rlippert@google.com>
Tested-By: Madhavan Srinivasan <maddy@linux.vnet.ibm.com>
Signed-off-by: Madhavan Srinivasan <maddy@linux.vnet.ibm.com>
---
arch/powerpc/perf/imc-pmu.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/arch/powerpc/perf/imc-pmu.c b/arch/powerpc/perf/imc-pmu.c
index 88126245881b..88d1415e2547 100644
--- a/arch/powerpc/perf/imc-pmu.c
+++ b/arch/powerpc/perf/imc-pmu.c
@@ -467,7 +467,7 @@ static int nest_imc_event_init(struct perf_event *event)
* Nest HW counter memory resides in a per-chip reserve-memory (HOMER).
* Get the base memory addresss for this cpu.
*/
- chip_id = topology_physical_package_id(event->cpu);
+ chip_id = cpu_to_chip_id(event->cpu);
pcni = pmu->mem_info;
do {
if (pcni->id == chip_id) {
@@ -524,19 +524,19 @@ static int nest_imc_event_init(struct perf_event *event)
*/
static int core_imc_mem_init(int cpu, int size)
{
- int phys_id, rc = 0, core_id = (cpu / threads_per_core);
+ int nid, rc = 0, core_id = (cpu / threads_per_core);
struct imc_mem_info *mem_info;
/*
* alloc_pages_node() will allocate memory for core in the
* local node only.
*/
- phys_id = topology_physical_package_id(cpu);
+ nid = cpu_to_node(cpu);
mem_info = &core_imc_pmu->mem_info[core_id];
mem_info->id = core_id;
/* We need only vbase for core counters */
- mem_info->vbase = page_address(alloc_pages_node(phys_id,
+ mem_info->vbase = page_address(alloc_pages_node(nid,
GFP_KERNEL | __GFP_ZERO | __GFP_THISNODE |
__GFP_NOWARN, get_order(size)));
if (!mem_info->vbase)
@@ -783,14 +783,14 @@ static int core_imc_event_init(struct perf_event *event)
static int thread_imc_mem_alloc(int cpu_id, int size)
{
u64 ldbar_value, *local_mem = per_cpu(thread_imc_mem, cpu_id);
- int phys_id = topology_physical_package_id(cpu_id);
+ int nid = cpu_to_node(cpu_id);
if (!local_mem) {
/*
* This case could happen only once at start, since we dont
* free the memory in cpu offline path.
*/
- local_mem = page_address(alloc_pages_node(phys_id,
+ local_mem = page_address(alloc_pages_node(nid,
GFP_KERNEL | __GFP_ZERO | __GFP_THISNODE |
__GFP_NOWARN, get_order(size)));
if (!local_mem)
--
2.7.4
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH 2/2] powerpc/perf/imc: use NUMA_NO_NODE for alloc_pages_node
2017-10-15 18:43 [PATCH 1/2] powerpc/perf/imc: use cpu_to_node instead topology_physical_package_id Madhavan Srinivasan
@ 2017-10-15 18:43 ` Madhavan Srinivasan
2017-10-16 2:18 ` Balbir Singh
2017-11-24 9:46 ` [1/2] powerpc/perf/imc: use cpu_to_node instead topology_physical_package_id Michael Ellerman
1 sibling, 1 reply; 5+ messages in thread
From: Madhavan Srinivasan @ 2017-10-15 18:43 UTC (permalink / raw)
To: mpe; +Cc: linuxppc-dev, Madhavan Srinivasan
alloc_pages_node() when passed NUMA_NO_NODE for the
node_id, could get memory from closest node. Cleanup
core imc and thread imc memory init functions to use
NUMA_NO_NODE.
Signed-off-by: Madhavan Srinivasan <maddy@linux.vnet.ibm.com>
---
arch/powerpc/perf/imc-pmu.c | 8 +++-----
1 file changed, 3 insertions(+), 5 deletions(-)
diff --git a/arch/powerpc/perf/imc-pmu.c b/arch/powerpc/perf/imc-pmu.c
index 88d1415e2547..897098fb2a31 100644
--- a/arch/powerpc/perf/imc-pmu.c
+++ b/arch/powerpc/perf/imc-pmu.c
@@ -524,19 +524,18 @@ static int nest_imc_event_init(struct perf_event *event)
*/
static int core_imc_mem_init(int cpu, int size)
{
- int nid, rc = 0, core_id = (cpu / threads_per_core);
+ int rc = 0, core_id = (cpu / threads_per_core);
struct imc_mem_info *mem_info;
/*
* alloc_pages_node() will allocate memory for core in the
* local node only.
*/
- nid = cpu_to_node(cpu);
mem_info = &core_imc_pmu->mem_info[core_id];
mem_info->id = core_id;
/* We need only vbase for core counters */
- mem_info->vbase = page_address(alloc_pages_node(nid,
+ mem_info->vbase = page_address(alloc_pages_node(NUMA_NO_NODE,
GFP_KERNEL | __GFP_ZERO | __GFP_THISNODE |
__GFP_NOWARN, get_order(size)));
if (!mem_info->vbase)
@@ -783,14 +782,13 @@ static int core_imc_event_init(struct perf_event *event)
static int thread_imc_mem_alloc(int cpu_id, int size)
{
u64 ldbar_value, *local_mem = per_cpu(thread_imc_mem, cpu_id);
- int nid = cpu_to_node(cpu_id);
if (!local_mem) {
/*
* This case could happen only once at start, since we dont
* free the memory in cpu offline path.
*/
- local_mem = page_address(alloc_pages_node(nid,
+ local_mem = page_address(alloc_pages_node(NUMA_NO_NODE,
GFP_KERNEL | __GFP_ZERO | __GFP_THISNODE |
__GFP_NOWARN, get_order(size)));
if (!local_mem)
--
2.7.4
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH 2/2] powerpc/perf/imc: use NUMA_NO_NODE for alloc_pages_node
2017-10-15 18:43 ` [PATCH 2/2] powerpc/perf/imc: use NUMA_NO_NODE for alloc_pages_node Madhavan Srinivasan
@ 2017-10-16 2:18 ` Balbir Singh
2017-10-16 16:09 ` Madhavan Srinivasan
0 siblings, 1 reply; 5+ messages in thread
From: Balbir Singh @ 2017-10-16 2:18 UTC (permalink / raw)
To: Madhavan Srinivasan; +Cc: mpe, linuxppc-dev
On Mon, 16 Oct 2017 00:13:42 +0530
Madhavan Srinivasan <maddy@linux.vnet.ibm.com> wrote:
> alloc_pages_node() when passed NUMA_NO_NODE for the
> node_id, could get memory from closest node. Cleanup
> core imc and thread imc memory init functions to use
> NUMA_NO_NODE.
The changelog is not clear, alloc_pages_node() takes a
preferred node id and creates a node zonelist from it.
How is NUMA_NO_NODE better?
Balbir Singh.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 2/2] powerpc/perf/imc: use NUMA_NO_NODE for alloc_pages_node
2017-10-16 2:18 ` Balbir Singh
@ 2017-10-16 16:09 ` Madhavan Srinivasan
0 siblings, 0 replies; 5+ messages in thread
From: Madhavan Srinivasan @ 2017-10-16 16:09 UTC (permalink / raw)
To: Balbir Singh; +Cc: mpe, linuxppc-dev
On Monday 16 October 2017 07:48 AM, Balbir Singh wrote:
> On Mon, 16 Oct 2017 00:13:42 +0530
> Madhavan Srinivasan <maddy@linux.vnet.ibm.com> wrote:
>
>> alloc_pages_node() when passed NUMA_NO_NODE for the
>> node_id, could get memory from closest node. Cleanup
>> core imc and thread imc memory init functions to use
>> NUMA_NO_NODE.
>
> The changelog is not clear, alloc_pages_node() takes a
> preferred node id and creates a node zonelist from it.
> How is NUMA_NO_NODE better?
IIUC with NUMA_NO_NODE we could remove the __GFP_NOWARN
for alloc_pages_node(). That said, one must be careful to make
sure we dont end up allocating memory from the boot cpu.
And incase of In Memory Collection (IMC) counters, it is handled
in the cpu online path.
Will send a v2 with __GFP_NOWARN removed which
I missed in this :) .
Maddy
> Balbir Singh.
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [1/2] powerpc/perf/imc: use cpu_to_node instead topology_physical_package_id
2017-10-15 18:43 [PATCH 1/2] powerpc/perf/imc: use cpu_to_node instead topology_physical_package_id Madhavan Srinivasan
2017-10-15 18:43 ` [PATCH 2/2] powerpc/perf/imc: use NUMA_NO_NODE for alloc_pages_node Madhavan Srinivasan
@ 2017-11-24 9:46 ` Michael Ellerman
1 sibling, 0 replies; 5+ messages in thread
From: Michael Ellerman @ 2017-11-24 9:46 UTC (permalink / raw)
To: Madhavan Srinivasan; +Cc: Madhavan Srinivasan, linuxppc-dev
On Sun, 2017-10-15 at 18:43:41 UTC, Madhavan Srinivasan wrote:
> ...
> [1069001518.000000] [c000003f95b3f770] [c0000000000b2574] init_imc_pmu+0x1f4/0xc40
> [1069005374.000000] [c000003f95b3f850] [c00000000008fec8] opal_imc_counters_probe+0x2e8/0x3e0
> [1069009426.000000] [c000003f95b3f950] [c0000000006153a4] platform_drv_probe+0x44/0x90
> [1069012818.000000] [c000003f95b3f9c0] [c0000000006124c0] really_probe+0x290/0x370
> [1069016302.000000] [c000003f95b3fa50] [c0000000006126c8] __driver_attach+0x128/0x130
> [1069019564.000000] [c000003f95b3fa90] [c00000000060f38c] bus_for_each_dev+0x9c/0x110
> [1069022838.000000] [c000003f95b3fae0] [c000000000611dfc] driver_attach+0x3c/0x60
> [1069026104.000000] [c000003f95b3fb10] [c0000000006118d8] bus_add_driver+0x298/0x320
> [1069029428.000000] [c000003f95b3fb90] [c0000000006135b8] driver_register+0xb8/0x1a0
> [1069033016.000000] [c000003f95b3fc00] [c00000000061533c] __platform_driver_register+0x8c/0xb0
> [1069036362.000000] [c000003f95b3fc30] [c000000000cbb0fc] opal_imc_driver_init+0x24/0x38
> [1069039756.000000] [c000003f95b3fc50] [c00000000000cc70] do_one_initcall+0xd0/0x250
> [1069043094.000000] [c000003f95b3fd20] [c000000000ca44d0] kernel_init_freeable+0x244/0x324
> [1069046490.000000] [c000003f95b3fdc0] [c00000000000d600] kernel_init+0x30/0x1b0
> [1069050348.000000] [c000003f95b3fe30] [c00000000000b268] ret_from_kernel_thread+0x5c/0x74
>
> init_imc_pmu() use topology_physical_package_id() to detect the phy_id
> of the processor it is on to get local memory. But this cause crashes
> when node_id are not same as physicaly id. As a fix use cpu_to_node().
>
> Reported-By: Rob Lippert <rlippert@google.com>
> Tested-By: Madhavan Srinivasan <maddy@linux.vnet.ibm.com>
> Signed-off-by: Madhavan Srinivasan <maddy@linux.vnet.ibm.com>
Applied to powerpc fixes, thanks.
https://git.kernel.org/powerpc/c/f3f1dfd600ff82b18b7ea73d80eb27
cheers
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2017-11-24 9:46 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-10-15 18:43 [PATCH 1/2] powerpc/perf/imc: use cpu_to_node instead topology_physical_package_id Madhavan Srinivasan
2017-10-15 18:43 ` [PATCH 2/2] powerpc/perf/imc: use NUMA_NO_NODE for alloc_pages_node Madhavan Srinivasan
2017-10-16 2:18 ` Balbir Singh
2017-10-16 16:09 ` Madhavan Srinivasan
2017-11-24 9:46 ` [1/2] powerpc/perf/imc: use cpu_to_node instead topology_physical_package_id Michael Ellerman
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).