[PATCH] powerpc/perf: Fix memory allocation for core-imc based on num_possible_cpus()

[PATCH] powerpc/perf: Fix memory allocation for core-imc based on num_possible_cpus()

[Anju T Sudhakar]

Currently memory is allocated for core-imc based on cpu_present_mask, which has
bit 'cpu' set iff cpu is populated. We use  (cpu number / threads per core)
as as array index to access the memory.
So in a system with guarded cores, since allocation happens based on
cpu_present_mask, (cpu number / threads per core) bounds the index and leads
to memory overflow.

The issue is exposed in a guard test.
The guard test will make some CPU's as un-available to the system during boot
time as well as at runtime. So when the cpu is unavailable to the system during
boot time, the memory allocation happens depending on the number of available
cpus. And when we access the memory using (cpu number / threads per core) as the
index the system crashes due to memory overflow.

Allocating memory for core-imc based on cpu_possible_mask, which has
bit 'cpu' set iff cpu is populatable, will fix this issue.

Reported-by: Pridhiviraj Paidipeddi <ppaidipe@linux.vnet.ibm.com>
Signed-off-by: Anju T Sudhakar <anju@linux.vnet.ibm.com>
---
 arch/powerpc/perf/imc-pmu.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/powerpc/perf/imc-pmu.c b/arch/powerpc/perf/imc-pmu.c
index d7532e7..75fb23c 100644
--- a/arch/powerpc/perf/imc-pmu.c
+++ b/arch/powerpc/perf/imc-pmu.c
@@ -1146,7 +1146,7 @@ static int init_nest_pmu_ref(void)
 
 static void cleanup_all_core_imc_memory(void)
 {
-	int i, nr_cores = DIV_ROUND_UP(num_present_cpus(), threads_per_core);
+	int i, nr_cores = DIV_ROUND_UP(num_possible_cpus(), threads_per_core);
 	struct imc_mem_info *ptr = core_imc_pmu->mem_info;
 	int size = core_imc_pmu->counter_mem_size;
 
@@ -1264,7 +1264,7 @@ static int imc_mem_init(struct imc_pmu *pmu_ptr, struct device_node *parent,
 		if (!pmu_ptr->pmu.name)
 			return -ENOMEM;
 
-		nr_cores = DIV_ROUND_UP(num_present_cpus(), threads_per_core);
+		nr_cores = DIV_ROUND_UP(num_possible_cpus(), threads_per_core);
 		pmu_ptr->mem_info = kcalloc(nr_cores, sizeof(struct imc_mem_info),
 								GFP_KERNEL);
 
-- 
2.7.4

Re: [PATCH] powerpc/perf: Fix memory allocation for core-imc based on num_possible_cpus()

[Michael Neuling]

On Fri, 2018-05-11 at 19:13 +0530, Anju T Sudhakar wrote:
> Currently memory is allocated for core-imc based on cpu_present_mask, whi=
ch
> has
> bit 'cpu' set iff cpu is populated. We use  (cpu number / threads per cor=
e)
> as as array index to access the memory.
> So in a system with guarded cores, since allocation happens based on
> cpu_present_mask, (cpu number / threads per core) bounds the index and le=
ads
> to memory overflow.
>=20
> The issue is exposed in a guard test.
> The guard test will make some CPU's as un-available to the system during =
boot
> time as well as at runtime. So when the cpu is unavailable to the system
> during
> boot time, the memory allocation happens depending on the number of avail=
able
> cpus. And when we access the memory using (cpu number / threads per core)=
 as
> the
> index the system crashes due to memory overflow.
>=20
> Allocating memory for core-imc based on cpu_possible_mask, which has
> bit 'cpu' set iff cpu is populatable, will fix this issue.
>=20
> Reported-by: Pridhiviraj Paidipeddi <ppaidipe@linux.vnet.ibm.com>
> Signed-off-by: Anju T Sudhakar <anju@linux.vnet.ibm.com>

Thanks, this should be:=20

Cc: <stable@vger.kernel.org> # 4.14

> ---
>  arch/powerpc/perf/imc-pmu.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>=20
> diff --git a/arch/powerpc/perf/imc-pmu.c b/arch/powerpc/perf/imc-pmu.c
> index d7532e7..75fb23c 100644
> --- a/arch/powerpc/perf/imc-pmu.c
> +++ b/arch/powerpc/perf/imc-pmu.c
> @@ -1146,7 +1146,7 @@ static int init_nest_pmu_ref(void)
> =20
>  static void cleanup_all_core_imc_memory(void)
>  {
> -	int i, nr_cores =3D DIV_ROUND_UP(num_present_cpus(), threads_per_core);
> +	int i, nr_cores =3D DIV_ROUND_UP(num_possible_cpus(),
> threads_per_core);
>  	struct imc_mem_info *ptr =3D core_imc_pmu->mem_info;
>  	int size =3D core_imc_pmu->counter_mem_size;
> =20
> @@ -1264,7 +1264,7 @@ static int imc_mem_init(struct imc_pmu *pmu_ptr, st=
ruct
> device_node *parent,
>  		if (!pmu_ptr->pmu.name)
>  			return -ENOMEM;
> =20
> -		nr_cores =3D DIV_ROUND_UP(num_present_cpus(),
> threads_per_core);
> +		nr_cores =3D DIV_ROUND_UP(num_possible_cpus(),
> threads_per_core);
>  		pmu_ptr->mem_info =3D kcalloc(nr_cores, sizeof(struct
> imc_mem_info),
>  								GFP_KERNEL);
> =20

Re: [PATCH] powerpc/perf: Fix memory allocation for core-imc based on num_possible_cpus()

[Balbir Singh]

On Fri, May 11, 2018 at 11:43 PM, Anju T Sudhakar
<anju@linux.vnet.ibm.com> wrote:
> Currently memory is allocated for core-imc based on cpu_present_mask, which has
> bit 'cpu' set iff cpu is populated. We use  (cpu number / threads per core)
> as as array index to access the memory.
> So in a system with guarded cores, since allocation happens based on
> cpu_present_mask, (cpu number / threads per core) bounds the index and leads
> to memory overflow.
>
> The issue is exposed in a guard test.
> The guard test will make some CPU's as un-available to the system during boot
> time as well as at runtime. So when the cpu is unavailable to the system during
> boot time, the memory allocation happens depending on the number of available
> cpus. And when we access the memory using (cpu number / threads per core) as the
> index the system crashes due to memory overflow.
>
> Allocating memory for core-imc based on cpu_possible_mask, which has
> bit 'cpu' set iff cpu is populatable, will fix this issue.
>
> Reported-by: Pridhiviraj Paidipeddi <ppaidipe@linux.vnet.ibm.com>
> Signed-off-by: Anju T Sudhakar <anju@linux.vnet.ibm.com>
> ---
>  arch/powerpc/perf/imc-pmu.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)

The changelog does not clearly call out the confusion between present
and possible.
Guarded CPUs are possible but not present, so it blows a hole when we assume the
max length of our allocation is driven by our max present cpus, where
as one of the cpus
might be online and be beyond the max present cpus, due to the hole..

Reviewed-by: Balbir Singh <bsingharora@gmail.com>

Balbir Singh.

Re: [PATCH] powerpc/perf: Fix memory allocation for core-imc based on num_possible_cpus()

[Madhavan Srinivasan]

On Saturday 12 May 2018 05:15 AM, Michael Neuling wrote:
> On Fri, 2018-05-11 at 19:13 +0530, Anju T Sudhakar wrote:
>> Currently memory is allocated for core-imc based on cpu_present_mask, which
>> has
>> bit 'cpu' set iff cpu is populated. We use  (cpu number / threads per core)
>> as as array index to access the memory.
>> So in a system with guarded cores, since allocation happens based on
>> cpu_present_mask, (cpu number / threads per core) bounds the index and leads
>> to memory overflow.
>>
>> The issue is exposed in a guard test.
>> The guard test will make some CPU's as un-available to the system during boot
>> time as well as at runtime. So when the cpu is unavailable to the system
>> during
>> boot time, the memory allocation happens depending on the number of available
>> cpus. And when we access the memory using (cpu number / threads per core) as
>> the
>> index the system crashes due to memory overflow.
>>
>> Allocating memory for core-imc based on cpu_possible_mask, which has
>> bit 'cpu' set iff cpu is populatable, will fix this issue.
>>
>> Reported-by: Pridhiviraj Paidipeddi <ppaidipe@linux.vnet.ibm.com>
>> Signed-off-by: Anju T Sudhakar <anju@linux.vnet.ibm.com>
> Thanks, this should be:
>
> Cc: <stable@vger.kernel.org> # 4.14

Thanks for marking to stable. But it should go to 4.14+ stable releases.

Maddy

>> ---
>>   arch/powerpc/perf/imc-pmu.c | 4 ++--
>>   1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/arch/powerpc/perf/imc-pmu.c b/arch/powerpc/perf/imc-pmu.c
>> index d7532e7..75fb23c 100644
>> --- a/arch/powerpc/perf/imc-pmu.c
>> +++ b/arch/powerpc/perf/imc-pmu.c
>> @@ -1146,7 +1146,7 @@ static int init_nest_pmu_ref(void)
>>   
>>   static void cleanup_all_core_imc_memory(void)
>>   {
>> -	int i, nr_cores = DIV_ROUND_UP(num_present_cpus(), threads_per_core);
>> +	int i, nr_cores = DIV_ROUND_UP(num_possible_cpus(),
>> threads_per_core);
>>   	struct imc_mem_info *ptr = core_imc_pmu->mem_info;
>>   	int size = core_imc_pmu->counter_mem_size;
>>   
>> @@ -1264,7 +1264,7 @@ static int imc_mem_init(struct imc_pmu *pmu_ptr, struct
>> device_node *parent,
>>   		if (!pmu_ptr->pmu.name)
>>   			return -ENOMEM;
>>   
>> -		nr_cores = DIV_ROUND_UP(num_present_cpus(),
>> threads_per_core);
>> +		nr_cores = DIV_ROUND_UP(num_possible_cpus(),
>> threads_per_core);
>>   		pmu_ptr->mem_info = kcalloc(nr_cores, sizeof(struct
>> imc_mem_info),
>>   								GFP_KERNEL);
>>   

Re: [PATCH] powerpc/perf: Fix memory allocation for core-imc based on num_possible_cpus()

[Anju T Sudhakar]

Hi,


On Saturday 12 May 2018 06:05 AM, Balbir Singh wrote:
> On Fri, May 11, 2018 at 11:43 PM, Anju T Sudhakar
> <anju@linux.vnet.ibm.com> wrote:
>> Currently memory is allocated for core-imc based on cpu_present_mask, which has
>> bit 'cpu' set iff cpu is populated. We use  (cpu number / threads per core)
>> as as array index to access the memory.
>> So in a system with guarded cores, since allocation happens based on
>> cpu_present_mask, (cpu number / threads per core) bounds the index and leads
>> to memory overflow.
>>
>> The issue is exposed in a guard test.
>> The guard test will make some CPU's as un-available to the system during boot
>> time as well as at runtime. So when the cpu is unavailable to the system during
>> boot time, the memory allocation happens depending on the number of available
>> cpus. And when we access the memory using (cpu number / threads per core) as the
>> index the system crashes due to memory overflow.
>>
>> Allocating memory for core-imc based on cpu_possible_mask, which has
>> bit 'cpu' set iff cpu is populatable, will fix this issue.
>>
>> Reported-by: Pridhiviraj Paidipeddi <ppaidipe@linux.vnet.ibm.com>
>> Signed-off-by: Anju T Sudhakar <anju@linux.vnet.ibm.com>
>> ---
>>   arch/powerpc/perf/imc-pmu.c | 4 ++--
>>   1 file changed, 2 insertions(+), 2 deletions(-)
> The changelog does not clearly call out the confusion between present
> and possible.
> Guarded CPUs are possible but not present, so it blows a hole when we assume the
> max length of our allocation is driven by our max present cpus, where
> as one of the cpus
> might be online and be beyond the max present cpus, due to the hole..
>
> Reviewed-by: Balbir Singh <bsingharora@gmail.com>
>
> Balbir Singh.
>

Thanks for the review.
OK. I will update the commit message here.



Regards,
Anju

Re: [PATCH] powerpc/perf: Fix memory allocation for core-imc based on num_possible_cpus()

[Anju T Sudhakar]

[-- Attachment #1: Type: text/plain, Size: 2197 bytes --]



On Friday 11 May 2018 07:13 PM, Anju T Sudhakar wrote:
> Currently memory is allocated for core-imc based on cpu_present_mask, which has
> bit 'cpu' set iff cpu is populated. We use  (cpu number / threads per core)
> as as array index to access the memory.
> So in a system with guarded cores, since allocation happens based on
> cpu_present_mask, (cpu number / threads per core) bounds the index and leads
> to memory overflow.
>
> The issue is exposed in a guard test.
> The guard test will make some CPU's as un-available to the system during boot
> time as well as at runtime. So when the cpu is unavailable to the system during
> boot time, the memory allocation happens depending on the number of available
> cpus. And when we access the memory using (cpu number / threads per core) as the
> index the system crashes due to memory overflow.
>
> Allocating memory for core-imc based on cpu_possible_mask, which has
> bit 'cpu' set iff cpu is populatable, will fix this issue.
>
> Reported-by: Pridhiviraj Paidipeddi <ppaidipe@linux.vnet.ibm.com>
> Signed-off-by: Anju T Sudhakar <anju@linux.vnet.ibm.com>

Cc: <stable@vger.kernel.org> # v4.14 +

> ---
>   arch/powerpc/perf/imc-pmu.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/arch/powerpc/perf/imc-pmu.c b/arch/powerpc/perf/imc-pmu.c
> index d7532e7..75fb23c 100644
> --- a/arch/powerpc/perf/imc-pmu.c
> +++ b/arch/powerpc/perf/imc-pmu.c
> @@ -1146,7 +1146,7 @@ static int init_nest_pmu_ref(void)
>
>   static void cleanup_all_core_imc_memory(void)
>   {
> -	int i, nr_cores = DIV_ROUND_UP(num_present_cpus(), threads_per_core);
> +	int i, nr_cores = DIV_ROUND_UP(num_possible_cpus(), threads_per_core);
>   	struct imc_mem_info *ptr = core_imc_pmu->mem_info;
>   	int size = core_imc_pmu->counter_mem_size;
>
> @@ -1264,7 +1264,7 @@ static int imc_mem_init(struct imc_pmu *pmu_ptr, struct device_node *parent,
>   		if (!pmu_ptr->pmu.name)
>   			return -ENOMEM;
>
> -		nr_cores = DIV_ROUND_UP(num_present_cpus(), threads_per_core);
> +		nr_cores = DIV_ROUND_UP(num_possible_cpus(), threads_per_core);
>   		pmu_ptr->mem_info = kcalloc(nr_cores, sizeof(struct imc_mem_info),
>   								GFP_KERNEL);
>


[-- Attachment #2: Type: text/html, Size: 2941 bytes --]

Re: [PATCH] powerpc/perf: Fix memory allocation for core-imc based on num_possible_cpus()

[Michael Ellerman]

Anju T Sudhakar <anju@linux.vnet.ibm.com> writes:
> On Saturday 12 May 2018 06:05 AM, Balbir Singh wrote:
>> On Fri, May 11, 2018 at 11:43 PM, Anju T Sudhakar
>> <anju@linux.vnet.ibm.com> wrote:
>>> Currently memory is allocated for core-imc based on cpu_present_mask, which has
>>> bit 'cpu' set iff cpu is populated. We use  (cpu number / threads per core)
>>> as as array index to access the memory.
>>> So in a system with guarded cores, since allocation happens based on
>>> cpu_present_mask, (cpu number / threads per core) bounds the index and leads
>>> to memory overflow.
>>>
>>> The issue is exposed in a guard test.
>>> The guard test will make some CPU's as un-available to the system during boot
>>> time as well as at runtime. So when the cpu is unavailable to the system during
>>> boot time, the memory allocation happens depending on the number of available
>>> cpus. And when we access the memory using (cpu number / threads per core) as the
>>> index the system crashes due to memory overflow.
>>>
>>> Allocating memory for core-imc based on cpu_possible_mask, which has
>>> bit 'cpu' set iff cpu is populatable, will fix this issue.
>>>
>>> Reported-by: Pridhiviraj Paidipeddi <ppaidipe@linux.vnet.ibm.com>
>>> Signed-off-by: Anju T Sudhakar <anju@linux.vnet.ibm.com>
>>> ---
>>>   arch/powerpc/perf/imc-pmu.c | 4 ++--
>>>   1 file changed, 2 insertions(+), 2 deletions(-)
>> The changelog does not clearly call out the confusion between present
>> and possible.
>> Guarded CPUs are possible but not present, so it blows a hole when we assume the
>> max length of our allocation is driven by our max present cpus, where
>> as one of the cpus
>> might be online and be beyond the max present cpus, due to the hole..
>>
>> Reviewed-by: Balbir Singh <bsingharora@gmail.com>
>
> Thanks for the review.
> OK. I will update the commit message here.

Yeah please do. "Guarded" CPUs is also not a well understand term, so
please explain what that means for people who don't know.

cheers