* [PATCH 0/4] x86/smpboot: Fine-tuning for smp_init_package_map()
@ 2016-09-05 8:05 SF Markus Elfring
2016-09-05 8:10 ` [PATCH 1/4] x86/smpboot: Use kmalloc_array() in smp_init_package_map() SF Markus Elfring
` (3 more replies)
0 siblings, 4 replies; 8+ messages in thread
From: SF Markus Elfring @ 2016-09-05 8:05 UTC (permalink / raw)
To: x86, Andi Kleen, Boris Ostrovsky, Borislav Petkov,
H. Peter Anvin, Ingo Molnar, Jiri Olsa, Jürgen Groß,
Len Brown, Peter Zijlstra, Thomas Gleixner
Cc: LKML, kernel-janitors, Julia Lawall, Paolo Bonzini
From: Markus Elfring <elfring@users.sourceforge.net>
Date: Mon, 5 Sep 2016 09:50:05 +0200
A few update suggestions were taken into account
from static source code analysis.
Markus Elfring (4):
Use kmalloc_array()
Return directly after a failed kmalloc_array()
Replace one kzalloc() call by kcalloc()
Return directly after a failed kcalloc()
arch/x86/kernel/smpboot.c | 21 +++++++++++++++------
1 file changed, 15 insertions(+), 6 deletions(-)
--
2.9.3
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 1/4] x86/smpboot: Use kmalloc_array() in smp_init_package_map()
2016-09-05 8:05 [PATCH 0/4] x86/smpboot: Fine-tuning for smp_init_package_map() SF Markus Elfring
@ 2016-09-05 8:10 ` SF Markus Elfring
2016-09-05 9:19 ` Thomas Gleixner
2016-09-05 8:11 ` [PATCH 2/4] x86/smpboot: Return directly after a failed kmalloc_array() SF Markus Elfring
` (2 subsequent siblings)
3 siblings, 1 reply; 8+ messages in thread
From: SF Markus Elfring @ 2016-09-05 8:10 UTC (permalink / raw)
To: x86, Andi Kleen, Boris Ostrovsky, Borislav Petkov,
H. Peter Anvin, Ingo Molnar, Jiri Olsa, Jürgen Groß,
Len Brown, Peter Zijlstra, Thomas Gleixner
Cc: LKML, kernel-janitors, Julia Lawall, Paolo Bonzini
From: Markus Elfring <elfring@users.sourceforge.net>
Date: Mon, 5 Sep 2016 08:30:20 +0200
* A multiplication for the size determination of a memory allocation
indicated that an array data structure should be processed.
Thus use the corresponding function "kmalloc_array".
This issue was detected by using the Coccinelle software.
* Replace the specification of a data type by a pointer dereference
to make the corresponding size determination a bit safer according to
the Linux coding style convention.
* Move a calculation for one function call.
Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
arch/x86/kernel/smpboot.c | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/arch/x86/kernel/smpboot.c b/arch/x86/kernel/smpboot.c
index 3878725..36cf27e 100644
--- a/arch/x86/kernel/smpboot.c
+++ b/arch/x86/kernel/smpboot.c
@@ -349,9 +349,12 @@ static void __init smp_init_package_map(void)
* package can be smaller than the actual used apic ids.
*/
max_physical_pkg_id = DIV_ROUND_UP(MAX_LOCAL_APIC, ncpus);
- size = max_physical_pkg_id * sizeof(unsigned int);
- physical_to_logical_pkg = kmalloc(size, GFP_KERNEL);
- memset(physical_to_logical_pkg, 0xff, size);
+ physical_to_logical_pkg = kmalloc_array(max_physical_pkg_id,
+ sizeof(*physical_to_logical_pkg),
+ GFP_KERNEL);
+ memset(physical_to_logical_pkg,
+ 0xff,
+ sizeof(*physical_to_logical_pkg) * max_physical_pkg_id);
size = BITS_TO_LONGS(max_physical_pkg_id) * sizeof(unsigned long);
physical_package_map = kzalloc(size, GFP_KERNEL);
--
2.9.3
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 2/4] x86/smpboot: Return directly after a failed kmalloc_array()
2016-09-05 8:05 [PATCH 0/4] x86/smpboot: Fine-tuning for smp_init_package_map() SF Markus Elfring
2016-09-05 8:10 ` [PATCH 1/4] x86/smpboot: Use kmalloc_array() in smp_init_package_map() SF Markus Elfring
@ 2016-09-05 8:11 ` SF Markus Elfring
2016-09-05 8:12 ` [PATCH 3/4] x86/smpboot: Replace one kzalloc() call by kcalloc() SF Markus Elfring
2016-09-05 8:13 ` [PATCH 4/4] x86/smpboot: Return directly after a failed kcalloc() SF Markus Elfring
3 siblings, 0 replies; 8+ messages in thread
From: SF Markus Elfring @ 2016-09-05 8:11 UTC (permalink / raw)
To: x86, Andi Kleen, Boris Ostrovsky, Borislav Petkov,
H. Peter Anvin, Ingo Molnar, Jiri Olsa, Jürgen Groß,
Len Brown, Peter Zijlstra, Thomas Gleixner
Cc: LKML, kernel-janitors, Julia Lawall, Paolo Bonzini
From: Markus Elfring <elfring@users.sourceforge.net>
Date: Mon, 5 Sep 2016 08:48:32 +0200
Return directly after a memory allocation failed at the beginning.
Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
arch/x86/kernel/smpboot.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/arch/x86/kernel/smpboot.c b/arch/x86/kernel/smpboot.c
index 36cf27e..2ce06ef 100644
--- a/arch/x86/kernel/smpboot.c
+++ b/arch/x86/kernel/smpboot.c
@@ -352,6 +352,8 @@ static void __init smp_init_package_map(void)
physical_to_logical_pkg = kmalloc_array(max_physical_pkg_id,
sizeof(*physical_to_logical_pkg),
GFP_KERNEL);
+ if (!physical_to_logical_pkg)
+ return;
memset(physical_to_logical_pkg,
0xff,
sizeof(*physical_to_logical_pkg) * max_physical_pkg_id);
--
2.9.3
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 3/4] x86/smpboot: Replace one kzalloc() call by kcalloc()
2016-09-05 8:05 [PATCH 0/4] x86/smpboot: Fine-tuning for smp_init_package_map() SF Markus Elfring
2016-09-05 8:10 ` [PATCH 1/4] x86/smpboot: Use kmalloc_array() in smp_init_package_map() SF Markus Elfring
2016-09-05 8:11 ` [PATCH 2/4] x86/smpboot: Return directly after a failed kmalloc_array() SF Markus Elfring
@ 2016-09-05 8:12 ` SF Markus Elfring
2016-09-05 9:26 ` Paolo Bonzini
2016-09-05 8:13 ` [PATCH 4/4] x86/smpboot: Return directly after a failed kcalloc() SF Markus Elfring
3 siblings, 1 reply; 8+ messages in thread
From: SF Markus Elfring @ 2016-09-05 8:12 UTC (permalink / raw)
To: x86, Andi Kleen, Boris Ostrovsky, Borislav Petkov,
H. Peter Anvin, Ingo Molnar, Jiri Olsa, Jürgen Groß,
Len Brown, Peter Zijlstra, Thomas Gleixner
Cc: LKML, kernel-janitors, Julia Lawall, Paolo Bonzini
From: Markus Elfring <elfring@users.sourceforge.net>
Date: Mon, 5 Sep 2016 09:29:40 +0200
* The script "checkpatch.pl" can point information out like the following.
WARNING: Prefer kcalloc over kzalloc with multiply
Thus fix the affected source code place.
* Replace the specification of a data type by a pointer dereference
to make the corresponding size determination a bit safer according to
the Linux coding style convention.
* Delete the local variable "size" which became unnecessary with
this refactoring.
Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
arch/x86/kernel/smpboot.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/arch/x86/kernel/smpboot.c b/arch/x86/kernel/smpboot.c
index 2ce06ef..e3fdc44 100644
--- a/arch/x86/kernel/smpboot.c
+++ b/arch/x86/kernel/smpboot.c
@@ -311,7 +311,6 @@ EXPORT_SYMBOL(topology_phys_to_logical_pkg);
static void __init smp_init_package_map(void)
{
unsigned int ncpus, cpu;
- size_t size;
/*
* Today neither Intel nor AMD support heterogenous systems. That
@@ -357,8 +356,9 @@ static void __init smp_init_package_map(void)
memset(physical_to_logical_pkg,
0xff,
sizeof(*physical_to_logical_pkg) * max_physical_pkg_id);
- size = BITS_TO_LONGS(max_physical_pkg_id) * sizeof(unsigned long);
- physical_package_map = kzalloc(size, GFP_KERNEL);
+ physical_package_map = kcalloc(BITS_TO_LONGS(max_physical_pkg_id),
+ sizeof(*physical_package_map),
+ GFP_KERNEL);
for_each_present_cpu(cpu) {
unsigned int apicid = apic->cpu_present_to_apicid(cpu);
--
2.9.3
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 4/4] x86/smpboot: Return directly after a failed kcalloc()
2016-09-05 8:05 [PATCH 0/4] x86/smpboot: Fine-tuning for smp_init_package_map() SF Markus Elfring
` (2 preceding siblings ...)
2016-09-05 8:12 ` [PATCH 3/4] x86/smpboot: Replace one kzalloc() call by kcalloc() SF Markus Elfring
@ 2016-09-05 8:13 ` SF Markus Elfring
3 siblings, 0 replies; 8+ messages in thread
From: SF Markus Elfring @ 2016-09-05 8:13 UTC (permalink / raw)
To: x86, Andi Kleen, Boris Ostrovsky, Borislav Petkov,
H. Peter Anvin, Ingo Molnar, Jiri Olsa, Jürgen Groß,
Len Brown, Peter Zijlstra, Thomas Gleixner
Cc: LKML, kernel-janitors, Julia Lawall, Paolo Bonzini
From: Markus Elfring <elfring@users.sourceforge.net>
Date: Mon, 5 Sep 2016 09:39:05 +0200
Return directly after a memory allocation failed in this function.
Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
arch/x86/kernel/smpboot.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/arch/x86/kernel/smpboot.c b/arch/x86/kernel/smpboot.c
index e3fdc44..a30f625 100644
--- a/arch/x86/kernel/smpboot.c
+++ b/arch/x86/kernel/smpboot.c
@@ -359,6 +359,10 @@ static void __init smp_init_package_map(void)
physical_package_map = kcalloc(BITS_TO_LONGS(max_physical_pkg_id),
sizeof(*physical_package_map),
GFP_KERNEL);
+ if (!physical_package_map) {
+ kfree(physical_to_logical_pkg);
+ return;
+ }
for_each_present_cpu(cpu) {
unsigned int apicid = apic->cpu_present_to_apicid(cpu);
--
2.9.3
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 1/4] x86/smpboot: Use kmalloc_array() in smp_init_package_map()
2016-09-05 8:10 ` [PATCH 1/4] x86/smpboot: Use kmalloc_array() in smp_init_package_map() SF Markus Elfring
@ 2016-09-05 9:19 ` Thomas Gleixner
0 siblings, 0 replies; 8+ messages in thread
From: Thomas Gleixner @ 2016-09-05 9:19 UTC (permalink / raw)
To: SF Markus Elfring
Cc: x86, Andi Kleen, Boris Ostrovsky, Borislav Petkov,
H. Peter Anvin, Ingo Molnar, Jiri Olsa, Jürgen Groß,
Len Brown, Peter Zijlstra, LKML, kernel-janitors, Julia Lawall,
Paolo Bonzini
On Mon, 5 Sep 2016, SF Markus Elfring wrote:
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Mon, 5 Sep 2016 08:30:20 +0200
>
> * A multiplication for the size determination of a memory allocation
> indicated that an array data structure should be processed.
> Thus use the corresponding function "kmalloc_array".
>
> This issue was detected by using the Coccinelle software.
>
> * Replace the specification of a data type by a pointer dereference
> to make the corresponding size determination a bit safer according to
> the Linux coding style convention.
>
> * Move a calculation for one function call.
>
> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
> ---
> arch/x86/kernel/smpboot.c | 9 ++++++---
> 1 file changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/arch/x86/kernel/smpboot.c b/arch/x86/kernel/smpboot.c
> index 3878725..36cf27e 100644
> --- a/arch/x86/kernel/smpboot.c
> +++ b/arch/x86/kernel/smpboot.c
> @@ -349,9 +349,12 @@ static void __init smp_init_package_map(void)
> * package can be smaller than the actual used apic ids.
> */
> max_physical_pkg_id = DIV_ROUND_UP(MAX_LOCAL_APIC, ncpus);
> - size = max_physical_pkg_id * sizeof(unsigned int);
> - physical_to_logical_pkg = kmalloc(size, GFP_KERNEL);
> - memset(physical_to_logical_pkg, 0xff, size);
> + physical_to_logical_pkg = kmalloc_array(max_physical_pkg_id,
> + sizeof(*physical_to_logical_pkg),
> + GFP_KERNEL);
> + memset(physical_to_logical_pkg,
> + 0xff,
> + sizeof(*physical_to_logical_pkg) * max_physical_pkg_id);
Why are you replacing well readable code by this line breaked crap?
Stop acting like a shell script or we might replace you by one.
Thanks,
tglx
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 3/4] x86/smpboot: Replace one kzalloc() call by kcalloc()
2016-09-05 8:12 ` [PATCH 3/4] x86/smpboot: Replace one kzalloc() call by kcalloc() SF Markus Elfring
@ 2016-09-05 9:26 ` Paolo Bonzini
2016-09-05 9:29 ` Paolo Bonzini
0 siblings, 1 reply; 8+ messages in thread
From: Paolo Bonzini @ 2016-09-05 9:26 UTC (permalink / raw)
To: SF Markus Elfring, x86, Andi Kleen, Boris Ostrovsky,
Borislav Petkov, H. Peter Anvin, Ingo Molnar, Jiri Olsa,
Jürgen Groß,
Len Brown, Peter Zijlstra, Thomas Gleixner
Cc: LKML, kernel-janitors, Julia Lawall
On 05/09/2016 10:12, SF Markus Elfring wrote:
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Mon, 5 Sep 2016 09:29:40 +0200
>
> * The script "checkpatch.pl" can point information out like the following.
>
> WARNING: Prefer kcalloc over kzalloc with multiply
>
> Thus fix the affected source code place.
>
> * Replace the specification of a data type by a pointer dereference
> to make the corresponding size determination a bit safer according to
> the Linux coding style convention.
>
> * Delete the local variable "size" which became unnecessary with
> this refactoring.
>
> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
> ---
> arch/x86/kernel/smpboot.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/arch/x86/kernel/smpboot.c b/arch/x86/kernel/smpboot.c
> index 2ce06ef..e3fdc44 100644
> --- a/arch/x86/kernel/smpboot.c
> +++ b/arch/x86/kernel/smpboot.c
> @@ -311,7 +311,6 @@ EXPORT_SYMBOL(topology_phys_to_logical_pkg);
> static void __init smp_init_package_map(void)
> {
> unsigned int ncpus, cpu;
> - size_t size;
>
> /*
> * Today neither Intel nor AMD support heterogenous systems. That
> @@ -357,8 +356,9 @@ static void __init smp_init_package_map(void)
> memset(physical_to_logical_pkg,
> 0xff,
> sizeof(*physical_to_logical_pkg) * max_physical_pkg_id);
> - size = BITS_TO_LONGS(max_physical_pkg_id) * sizeof(unsigned long);
> - physical_package_map = kzalloc(size, GFP_KERNEL);
> + physical_package_map = kcalloc(BITS_TO_LONGS(max_physical_pkg_id),
> + sizeof(*physical_package_map),
> + GFP_KERNEL);
This is bitmap_alloc.
Paolo
> for_each_present_cpu(cpu) {
> unsigned int apicid = apic->cpu_present_to_apicid(cpu);
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 3/4] x86/smpboot: Replace one kzalloc() call by kcalloc()
2016-09-05 9:26 ` Paolo Bonzini
@ 2016-09-05 9:29 ` Paolo Bonzini
0 siblings, 0 replies; 8+ messages in thread
From: Paolo Bonzini @ 2016-09-05 9:29 UTC (permalink / raw)
To: SF Markus Elfring, x86, Andi Kleen, Boris Ostrovsky,
Borislav Petkov, H. Peter Anvin, Ingo Molnar, Jiri Olsa,
Jürgen Groß,
Len Brown, Peter Zijlstra, Thomas Gleixner
Cc: LKML, kernel-janitors, Julia Lawall
On 05/09/2016 11:26, Paolo Bonzini wrote:
>
>
> On 05/09/2016 10:12, SF Markus Elfring wrote:
>> From: Markus Elfring <elfring@users.sourceforge.net>
>> Date: Mon, 5 Sep 2016 09:29:40 +0200
>>
>> * The script "checkpatch.pl" can point information out like the following.
>>
>> WARNING: Prefer kcalloc over kzalloc with multiply
>>
>> Thus fix the affected source code place.
>>
>> * Replace the specification of a data type by a pointer dereference
>> to make the corresponding size determination a bit safer according to
>> the Linux coding style convention.
>>
>> * Delete the local variable "size" which became unnecessary with
>> this refactoring.
>>
>> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
>> ---
>> arch/x86/kernel/smpboot.c | 6 +++---
>> 1 file changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/arch/x86/kernel/smpboot.c b/arch/x86/kernel/smpboot.c
>> index 2ce06ef..e3fdc44 100644
>> --- a/arch/x86/kernel/smpboot.c
>> +++ b/arch/x86/kernel/smpboot.c
>> @@ -311,7 +311,6 @@ EXPORT_SYMBOL(topology_phys_to_logical_pkg);
>> static void __init smp_init_package_map(void)
>> {
>> unsigned int ncpus, cpu;
>> - size_t size;
>>
>> /*
>> * Today neither Intel nor AMD support heterogenous systems. That
>> @@ -357,8 +356,9 @@ static void __init smp_init_package_map(void)
>> memset(physical_to_logical_pkg,
>> 0xff,
>> sizeof(*physical_to_logical_pkg) * max_physical_pkg_id);
>> - size = BITS_TO_LONGS(max_physical_pkg_id) * sizeof(unsigned long);
>> - physical_package_map = kzalloc(size, GFP_KERNEL);
>> + physical_package_map = kcalloc(BITS_TO_LONGS(max_physical_pkg_id),
>> + sizeof(*physical_package_map),
>> + GFP_KERNEL);
>
> This is bitmap_alloc.
Doh, bitmap_alloc is only in tools/include/linux/bitmap.h, but perhaps
it makes sense to add it to include/linux/bitmap.h or
include/linux/slab.h too...
Paolo
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2016-09-05 9:29 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-09-05 8:05 [PATCH 0/4] x86/smpboot: Fine-tuning for smp_init_package_map() SF Markus Elfring
2016-09-05 8:10 ` [PATCH 1/4] x86/smpboot: Use kmalloc_array() in smp_init_package_map() SF Markus Elfring
2016-09-05 9:19 ` Thomas Gleixner
2016-09-05 8:11 ` [PATCH 2/4] x86/smpboot: Return directly after a failed kmalloc_array() SF Markus Elfring
2016-09-05 8:12 ` [PATCH 3/4] x86/smpboot: Replace one kzalloc() call by kcalloc() SF Markus Elfring
2016-09-05 9:26 ` Paolo Bonzini
2016-09-05 9:29 ` Paolo Bonzini
2016-09-05 8:13 ` [PATCH 4/4] x86/smpboot: Return directly after a failed kcalloc() SF Markus Elfring
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).