It is quite legitimate for CPUs to be numbered sparsely, meaning that
it possible for an online CPU to have a number which is greater than
the total count of possible CPUs.
Currently find_get_context() has a sanity check on the cpu number
where it checks it against num_possible_cpus(). This test can fail
for a legitimate cpu number if the cpu_possible_mask is sparsely
populated.
This fixes the problem by checking the CPU number against
nr_cpumask_bits instead, since that is the appropriate check to ensure
that the cpu number is same to pass to cpu_isset() subsequently.
Reported-by: Michael Neuling <[email protected]>
Tested-by: Michael Neuling <[email protected]>
Cc: [email protected]
Signed-off-by: Paul Mackerras <[email protected]>
---
kernel/perf_event.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/kernel/perf_event.c b/kernel/perf_event.c
index 6b7ddba..78551b3 100644
--- a/kernel/perf_event.c
+++ b/kernel/perf_event.c
@@ -1604,7 +1604,7 @@ static struct perf_event_context *find_get_context(pid_t pid, int cpu)
if (perf_paranoid_cpu() && !capable(CAP_SYS_ADMIN))
return ERR_PTR(-EACCES);
- if (cpu < 0 || cpu > num_possible_cpus())
+ if (cpu < 0 || cpu >= nr_cpumask_bits)
return ERR_PTR(-EINVAL);
/*
On Tue, 2009-12-15 at 19:40 +1100, Paul Mackerras wrote:
> It is quite legitimate for CPUs to be numbered sparsely, meaning that
> it possible for an online CPU to have a number which is greater than
> the total count of possible CPUs.
>
> Currently find_get_context() has a sanity check on the cpu number
> where it checks it against num_possible_cpus(). This test can fail
> for a legitimate cpu number if the cpu_possible_mask is sparsely
> populated.
>
> This fixes the problem by checking the CPU number against
> nr_cpumask_bits instead, since that is the appropriate check to ensure
> that the cpu number is same to pass to cpu_isset() subsequently.
Cute, do you actually have hardware that does this?
> Reported-by: Michael Neuling <[email protected]>
> Tested-by: Michael Neuling <[email protected]>
> Cc: [email protected]
> Signed-off-by: Paul Mackerras <[email protected]>
> ---
> kernel/perf_event.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/kernel/perf_event.c b/kernel/perf_event.c
> index 6b7ddba..78551b3 100644
> --- a/kernel/perf_event.c
> +++ b/kernel/perf_event.c
> @@ -1604,7 +1604,7 @@ static struct perf_event_context *find_get_context(pid_t pid, int cpu)
> if (perf_paranoid_cpu() && !capable(CAP_SYS_ADMIN))
> return ERR_PTR(-EACCES);
>
> - if (cpu < 0 || cpu > num_possible_cpus())
> + if (cpu < 0 || cpu >= nr_cpumask_bits)
> return ERR_PTR(-EINVAL);
>
> /*
On Tue, Dec 15, 2009 at 11:31:32AM +0100, Peter Zijlstra wrote:
> On Tue, 2009-12-15 at 19:40 +1100, Paul Mackerras wrote:
> > It is quite legitimate for CPUs to be numbered sparsely, meaning that
> > it possible for an online CPU to have a number which is greater than
> > the total count of possible CPUs.
> >
> > Currently find_get_context() has a sanity check on the cpu number
> > where it checks it against num_possible_cpus(). This test can fail
> > for a legitimate cpu number if the cpu_possible_mask is sparsely
> > populated.
> >
> > This fixes the problem by checking the CPU number against
> > nr_cpumask_bits instead, since that is the appropriate check to ensure
> > that the cpu number is same to pass to cpu_isset() subsequently.
>
> Cute, do you actually have hardware that does this?
Yeah, Mikey ran across this on a POWER7 box here.
Paul.
Commit-ID: 0f624e7e5625f4c30c836b7a5decfe2553582391
Gitweb: http://git.kernel.org/tip/0f624e7e5625f4c30c836b7a5decfe2553582391
Author: Paul Mackerras <[email protected]>
AuthorDate: Tue, 15 Dec 2009 19:40:32 +1100
Committer: Ingo Molnar <[email protected]>
CommitDate: Tue, 15 Dec 2009 13:09:55 +0100
perf_event: Fix incorrect range check on cpu number
It is quite legitimate for CPUs to be numbered sparsely, meaning
that it possible for an online CPU to have a number which is
greater than the total count of possible CPUs.
Currently find_get_context() has a sanity check on the cpu
number where it checks it against num_possible_cpus(). This
test can fail for a legitimate cpu number if the
cpu_possible_mask is sparsely populated.
This fixes the problem by checking the CPU number against
nr_cpumask_bits instead, since that is the appropriate check to
ensure that the cpu number is same to pass to cpu_isset()
subsequently.
Reported-by: Michael Neuling <[email protected]>
Signed-off-by: Paul Mackerras <[email protected]>
Tested-by: Michael Neuling <[email protected]>
Acked-by: Peter Zijlstra <[email protected]>
Cc: <[email protected]>
LKML-Reference: <[email protected]>
Signed-off-by: Ingo Molnar <[email protected]>
---
kernel/perf_event.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/kernel/perf_event.c b/kernel/perf_event.c
index d891ec4..8823b08 100644
--- a/kernel/perf_event.c
+++ b/kernel/perf_event.c
@@ -1603,7 +1603,7 @@ static struct perf_event_context *find_get_context(pid_t pid, int cpu)
if (perf_paranoid_cpu() && !capable(CAP_SYS_ADMIN))
return ERR_PTR(-EACCES);
- if (cpu < 0 || cpu > num_possible_cpus())
+ if (cpu < 0 || cpu >= nr_cpumask_bits)
return ERR_PTR(-EINVAL);
/*
Paul Mackerras wrote:
> On Tue, Dec 15, 2009 at 11:31:32AM +0100, Peter Zijlstra wrote:
>
>> On Tue, 2009-12-15 at 19:40 +1100, Paul Mackerras wrote:
>>> It is quite legitimate for CPUs to be numbered sparsely, meaning that
>>> it possible for an online CPU to have a number which is greater than
>>> the total count of possible CPUs.
>>>
>>> Currently find_get_context() has a sanity check on the cpu number
>>> where it checks it against num_possible_cpus(). This test can fail
>>> for a legitimate cpu number if the cpu_possible_mask is sparsely
>>> populated.
>>>
>>> This fixes the problem by checking the CPU number against
>>> nr_cpumask_bits instead, since that is the appropriate check to ensure
>>> that the cpu number is same to pass to cpu_isset() subsequently.
>> Cute, do you actually have hardware that does this?
>
> Yeah, Mikey ran across this on a POWER7 box here.
Does the perf tool need to be fixed too? The "perf stat" tool, at least, has a
"-a" switch that tells the tool to count the event on all cpus, and it does this
by iterating over the number of cpus, 0..n, assuming they are all contiguous.
- Corey
* Corey Ashford <[email protected]> wrote:
>
> Paul Mackerras wrote:
> >On Tue, Dec 15, 2009 at 11:31:32AM +0100, Peter Zijlstra wrote:
> >
> >>On Tue, 2009-12-15 at 19:40 +1100, Paul Mackerras wrote:
> >>>It is quite legitimate for CPUs to be numbered sparsely, meaning that
> >>>it possible for an online CPU to have a number which is greater than
> >>>the total count of possible CPUs.
> >>>
> >>>Currently find_get_context() has a sanity check on the cpu number
> >>>where it checks it against num_possible_cpus(). This test can fail
> >>>for a legitimate cpu number if the cpu_possible_mask is sparsely
> >>>populated.
> >>>
> >>>This fixes the problem by checking the CPU number against
> >>>nr_cpumask_bits instead, since that is the appropriate check to ensure
> >>>that the cpu number is same to pass to cpu_isset() subsequently.
> >>Cute, do you actually have hardware that does this?
> >
> >Yeah, Mikey ran across this on a POWER7 box here.
>
> Does the perf tool need to be fixed too? The "perf stat" tool, at
> least, has a "-a" switch that tells the tool to count the event on
> all cpus, and it does this by iterating over the number of cpus,
> 0..n, assuming they are all contiguous.
Yes, see patch 2/2 of this series.
Ingo
Ingo Molnar wrote:
> * Corey Ashford <[email protected]> wrote:
>
>> Paul Mackerras wrote:
>>> On Tue, Dec 15, 2009 at 11:31:32AM +0100, Peter Zijlstra wrote:
>>>
>>>> On Tue, 2009-12-15 at 19:40 +1100, Paul Mackerras wrote:
>>>>> It is quite legitimate for CPUs to be numbered sparsely, meaning that
>>>>> it possible for an online CPU to have a number which is greater than
>>>>> the total count of possible CPUs.
>>>>>
>>>>> Currently find_get_context() has a sanity check on the cpu number
>>>>> where it checks it against num_possible_cpus(). This test can fail
>>>>> for a legitimate cpu number if the cpu_possible_mask is sparsely
>>>>> populated.
>>>>>
>>>>> This fixes the problem by checking the CPU number against
>>>>> nr_cpumask_bits instead, since that is the appropriate check to ensure
>>>>> that the cpu number is same to pass to cpu_isset() subsequently.
>>>> Cute, do you actually have hardware that does this?
>>> Yeah, Mikey ran across this on a POWER7 box here.
>> Does the perf tool need to be fixed too? The "perf stat" tool, at
>> least, has a "-a" switch that tells the tool to count the event on
>> all cpus, and it does this by iterating over the number of cpus,
>> 0..n, assuming they are all contiguous.
>
> Yes, see patch 2/2 of this series.
>
> Ingo
Oops! missed that. Thanks!
- Corey