2013-09-17 06:48:21

by Yan, Zheng

Subject: [PATCH] perf/x86/intel/uncore: don't use smp_processor_id() in validate_group()

From: "Yan, Zheng" <[email protected]>

uncore_validate_group() can't call smp_processor_id() because it is
in preemptible context. Pass NUMA_NO_NODE to the allocator instead.

Signed-off-by: Yan, Zheng <[email protected]>
---
arch/x86/kernel/cpu/perf_event_intel_uncore.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/kernel/cpu/perf_event_intel_uncore.c b/arch/x86/kernel/cpu/perf_event_intel_uncore.c
index fd8011e..11b1582 100644
--- a/arch/x86/kernel/cpu/perf_event_intel_uncore.c
+++ b/arch/x86/kernel/cpu/perf_event_intel_uncore.c
@@ -3031,7 +3031,7 @@ static int uncore_validate_group(struct intel_uncore_pmu *pmu,
struct intel_uncore_box *fake_box;
int ret = -EINVAL, n;

- fake_box = uncore_alloc_box(pmu->type, smp_processor_id());
+ fake_box = uncore_alloc_box(pmu->type, NUMA_NO_NODE);
if (!fake_box)
return -ENOMEM;

--
1.8.1.4

2013-09-18 11:31:33

by Peter Zijlstra

Subject: Re: [PATCH] perf/x86/intel/uncore: don't use smp_processor_id() in validate_group()

On Tue, Sep 17, 2013 at 02:48:13PM +0800, Yan, Zheng wrote:
> From: "Yan, Zheng" <[email protected]>
>
> uncore_validate_group() can't call smp_processor_id() because it is
> in preemptible context. Pass NUMA_NO_NODE to the allocator instead.
>
> Signed-off-by: Yan, Zheng <[email protected]>
> ---
> arch/x86/kernel/cpu/perf_event_intel_uncore.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/x86/kernel/cpu/perf_event_intel_uncore.c b/arch/x86/kernel/cpu/perf_event_intel_uncore.c
> index fd8011e..11b1582 100644
> --- a/arch/x86/kernel/cpu/perf_event_intel_uncore.c
> +++ b/arch/x86/kernel/cpu/perf_event_intel_uncore.c
> @@ -3031,7 +3031,7 @@ static int uncore_validate_group(struct intel_uncore_pmu *pmu,
> struct intel_uncore_box *fake_box;
> int ret = -EINVAL, n;
>
> - fake_box = uncore_alloc_box(pmu->type, smp_processor_id());
> + fake_box = uncore_alloc_box(pmu->type, NUMA_NO_NODE);

Doesn't work since you're passing cpu, not node.

I changed it to the below. However upon doing so I noticed you
hard coded PCI devices live on Node0, this is not true in general
(although likely true for tiny systems).

I've absolutely no clue about the entire PCI layer, but it would be nice
if there's a method to extract the local node of a pci device.

Bjorn is there such a thing?

---
--- a/arch/x86/kernel/cpu/perf_event_intel_uncore.c
+++ b/arch/x86/kernel/cpu/perf_event_intel_uncore.c
@@ -2706,14 +2706,14 @@ static void uncore_pmu_init_hrtimer(stru
box->hrtimer.function = uncore_pmu_hrtimer;
}

-struct intel_uncore_box *uncore_alloc_box(struct intel_uncore_type *type, int cpu)
+static struct intel_uncore_box *uncore_alloc_box(struct intel_uncore_type *type, int node)
{
struct intel_uncore_box *box;
int i, size;

size = sizeof(*box) + type->num_shared_regs * sizeof(struct intel_uncore_extra_reg);

- box = kzalloc_node(size, GFP_KERNEL, cpu_to_node(cpu));
+ box = kzalloc_node(size, GFP_KERNEL, node);
if (!box)
return NULL;

@@ -3031,7 +3031,7 @@ static int uncore_validate_group(struct
struct intel_uncore_box *fake_box;
int ret = -EINVAL, n;

- fake_box = uncore_alloc_box(pmu->type, smp_processor_id());
+ fake_box = uncore_alloc_box(pmu->type, NUMA_NO_NODE);
if (!fake_box)
return -ENOMEM;

@@ -3294,7 +3294,7 @@ static int uncore_pci_probe(struct pci_d
}

type = pci_uncores[UNCORE_PCI_DEV_TYPE(id->driver_data)];
- box = uncore_alloc_box(type, 0);
+ box = uncore_alloc_box(type, NUMA_NO_NODE);
if (!box)
return -ENOMEM;

@@ -3499,7 +3499,7 @@ static int uncore_cpu_prepare(int cpu, i
if (pmu->func_id < 0)
pmu->func_id = j;

- box = uncore_alloc_box(type, cpu);
+ box = uncore_alloc_box(type, cpu_to_node(cpu));
if (!box)
return -ENOMEM;

2013-09-20 03:28:13

by Bjorn Helgaas

Subject: Re: [PATCH] perf/x86/intel/uncore: don't use smp_processor_id() in validate_group()

On Wed, Sep 18, 2013 at 5:31 AM, Peter Zijlstra <[email protected]> wrote:
> On Tue, Sep 17, 2013 at 02:48:13PM +0800, Yan, Zheng wrote:
>> From: "Yan, Zheng" <[email protected]>
>>
>> uncore_validate_group() can't call smp_processor_id() because it is
>> in preemptible context. Pass NUMA_NO_NODE to the allocator instead.
>>
>> Signed-off-by: Yan, Zheng <[email protected]>
>> ---
>> arch/x86/kernel/cpu/perf_event_intel_uncore.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/arch/x86/kernel/cpu/perf_event_intel_uncore.c b/arch/x86/kernel/cpu/perf_event_intel_uncore.c
>> index fd8011e..11b1582 100644
>> --- a/arch/x86/kernel/cpu/perf_event_intel_uncore.c
>> +++ b/arch/x86/kernel/cpu/perf_event_intel_uncore.c
>> @@ -3031,7 +3031,7 @@ static int uncore_validate_group(struct intel_uncore_pmu *pmu,
>> struct intel_uncore_box *fake_box;
>> int ret = -EINVAL, n;
>>
>> - fake_box = uncore_alloc_box(pmu->type, smp_processor_id());
>> + fake_box = uncore_alloc_box(pmu->type, NUMA_NO_NODE);
>
> Doesn't work since you're passing cpu, not node.
>
> I changed it to the below. However upon doing so I noticed you
> hard coded PCI devices live on Node0, this is not true in general
> (although likely true for tiny systems).
>
> I've absolutely no clue about the entire PCI layer, but it would be nice
> if there's a method to extract the local node of a pci device.
>
> Bjorn is there such a thing?

dev_to_node(&pdev->dev), as in pci_call_probe()?

2013-09-20 09:58:01

by tip-bot for Vasyl Gomonovych

Subject: [tip:perf/urgent] perf/x86/intel/uncore: Don' t use smp_processor_id() in validate_group()

Commit-ID: 73c4427c6ca3b32fa0441791e9c6eadceff7242f
Gitweb: http://git.kernel.org/tip/73c4427c6ca3b32fa0441791e9c6eadceff7242f
Author: Yan, Zheng <[email protected]>
AuthorDate: Tue, 17 Sep 2013 14:48:13 +0800
Committer: Ingo Molnar <[email protected]>
CommitDate: Fri, 20 Sep 2013 06:54:36 +0200

perf/x86/intel/uncore: Don't use smp_processor_id() in validate_group()

uncore_validate_group() can't call smp_processor_id() because it is
in preemptible context. Pass NUMA_NO_NODE to the allocator instead.

Signed-off-by: Yan, Zheng <[email protected]>
Signed-off-by: Peter Zijlstra <[email protected]>
Link: http://lkml.kernel.org/r/[email protected]
Signed-off-by: Ingo Molnar <[email protected]>
---
arch/x86/kernel/cpu/perf_event_intel_uncore.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/arch/x86/kernel/cpu/perf_event_intel_uncore.c b/arch/x86/kernel/cpu/perf_event_intel_uncore.c
index 8ed4458..4118f9f 100644
--- a/arch/x86/kernel/cpu/perf_event_intel_uncore.c
+++ b/arch/x86/kernel/cpu/perf_event_intel_uncore.c
@@ -2706,14 +2706,14 @@ static void uncore_pmu_init_hrtimer(struct intel_uncore_box *box)
box->hrtimer.function = uncore_pmu_hrtimer;
}

-struct intel_uncore_box *uncore_alloc_box(struct intel_uncore_type *type, int cpu)
+static struct intel_uncore_box *uncore_alloc_box(struct intel_uncore_type *type, int node)
{
struct intel_uncore_box *box;
int i, size;

size = sizeof(*box) + type->num_shared_regs * sizeof(struct intel_uncore_extra_reg);

- box = kzalloc_node(size, GFP_KERNEL, cpu_to_node(cpu));
+ box = kzalloc_node(size, GFP_KERNEL, node);
if (!box)
return NULL;

@@ -3031,7 +3031,7 @@ static int uncore_validate_group(struct intel_uncore_pmu *pmu,
struct intel_uncore_box *fake_box;
int ret = -EINVAL, n;

- fake_box = uncore_alloc_box(pmu->type, smp_processor_id());
+ fake_box = uncore_alloc_box(pmu->type, NUMA_NO_NODE);
if (!fake_box)
return -ENOMEM;

@@ -3294,7 +3294,7 @@ static int uncore_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id
}

type = pci_uncores[UNCORE_PCI_DEV_TYPE(id->driver_data)];
- box = uncore_alloc_box(type, 0);
+ box = uncore_alloc_box(type, NUMA_NO_NODE);
if (!box)
return -ENOMEM;

@@ -3499,7 +3499,7 @@ static int uncore_cpu_prepare(int cpu, int phys_id)
if (pmu->func_id < 0)
pmu->func_id = j;

- box = uncore_alloc_box(type, cpu);
+ box = uncore_alloc_box(type, cpu_to_node(cpu));
if (!box)
return -ENOMEM;