arm64: use num_possible_cpus() instead of NR_CPUS

PATCH New Jingoo Han 4 participants 7 messages Jan 28, 2014 – Jan 29, 2014 arm64

lore

Share & tools

lore

Jingoo Han

author

2014-01-28 01:35 UTC #0 lore raw

PATCH 1/2 arm64: use num_possible_cpus() instead of NR_CPUS

Use num_possible_cpus() instead of direct use of NR_CPUS. Also, it fixes the following checkpatch warning.

WARNING: usage of NR_CPUS is often wrong - consider using cpu_possible(), num_possible_cpus(), for_each_possible_cpu(), etc

Signed-off-by Jingoo Han <[email protected]>

---
 arch/arm64/kernel/smp.c |   10 +++++-----
 arch/arm64/mm/context.c |    2 +-
 2 files changed, 6 insertions(+), 6 deletions(-)

arch/arm64/kernel/smp.c +5 -5

index 1b7617a..09ff7d4 100644
@@ -320,7 +320,7 @@ void __init smp_init_cpus(void)
 		 * cpu_logical_map was initialized to INVALID_HWID to
 		 * avoid matching valid MPIDR values.
 		 */
-		for (i = 1; (i < cpu) && (i < NR_CPUS); i++) {
+		for (i = 1; (i < cpu) && (i < num_possible_cpus()); i++) {
 			if (cpu_logical_map(i) == hwid) {
 				pr_err("%s: duplicate cpu reg properties in the DT\n",
 					dn->full_name);

@@ -352,7 +352,7 @@ void __init smp_init_cpus(void)
 			continue;
 		}
 
-		if (cpu >= NR_CPUS)
+		if (cpu >= num_possible_cpus())
 			goto next;
 
 		if (cpu_read_ops(dn, cpu) != 0)

@@ -368,9 +368,9 @@ next:
 	}
 
 	/* sanity check */
-	if (cpu > NR_CPUS)
+	if (cpu > num_possible_cpus())
 		pr_warning("no. of cores (%d) greater than configured maximum of %d - clipping\n",
-			   cpu, NR_CPUS);
+			   cpu, num_possible_cpus());
 
 	if (!bootcpu_valid) {
 		pr_err("DT missing boot CPU MPIDR, not enabling secondaries\n");

@@ -381,7 +381,7 @@ next:
 	 * All the cpus that made it to the cpu_logical_map have been
 	 * validated so set them as possible cpus.
 	 */
-	for (i = 0; i < NR_CPUS; i++)
+	for (i = 0; i < num_possible_cpus(); i++)
 		if (cpu_logical_map(i) != INVALID_HWID)
 			set_cpu_possible(i, true);
 }

arch/arm64/mm/context.c +1 -1

index baa758d..3ef960a 100644
@@ -151,7 +151,7 @@ void __new_context(struct mm_struct *mm)
 		smp_wmb();
 		smp_call_function(reset_context, NULL, 1);
 #endif
-		cpu_last_asid += NR_CPUS - 1;
+		cpu_last_asid += num_possible_cpus() - 1;
 	}
 
 	set_mm_context(mm, asid);

Signature · 1 line

1.7.10.4

Jingoo Han

author

2014-01-28 01:36 UTC #1 lore raw

PATCH 2/2 arm64: kernel: use seq_puts() instead of seq_printf()

For a constant format without additional arguments, use seq_puts() instead of seq_printf(). Also, it fixes the following checkpatch warning.

WARNING: Prefer seq_puts to seq_printf

Signed-off-by Jingoo Han <[email protected]>

---
 arch/arm64/kernel/setup.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

arch/arm64/kernel/setup.c +1 -1

index c8e9eff..4507691 100644
@@ -416,7 +416,7 @@ static int c_show(struct seq_file *m, void *v)
 			seq_printf(m, "%s ", hwcap_str[i]);
 
 	seq_printf(m, "\nCPU implementer\t: 0x%02x\n", read_cpuid_id() >> 24);
-	seq_printf(m, "CPU architecture: AArch64\n");
+	seq_puts(m, "CPU architecture: AArch64\n");
 	seq_printf(m, "CPU variant\t: 0x%x\n", (read_cpuid_id() >> 20) & 15);
 	seq_printf(m, "CPU part\t: 0x%03x\n", (read_cpuid_id() >> 4) & 0xfff);
 	seq_printf(m, "CPU revision\t: %d\n", read_cpuid_id() & 15);

Signature · 1 line

1.7.10.4

Catalin Marinas

maintainer

2014-01-28 15:51 UTC #2 lore raw

PATCH 2/2 arm64: kernel: use seq_puts() instead of seq_printf()

Quoted 16 lines · Jingoo Han

For a constant format without additional arguments, use seq_puts() instead of seq_printf(). Also, it fixes the following checkpatch warning.
WARNING: Prefer seq_puts to seq_printf
Signed-off-by Jingoo Han <[email protected]>
---
 arch/arm64/kernel/setup.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
arch/arm64/kernel/setup.c +0 -0
index c8e9eff..4507691 100644

arch/arm64/kernel/setup.c static int c_show(struct seq_file *m, void *v) · line 422

Quoted 8 lines

arch/arm64/kernel/setup.c +1 -1

@@ -416,7 +416,7 @@ static int c_show(struct seq_file *m, void *v)
 			seq_printf(m, "%s ", hwcap_str[i]);
 
 	seq_printf(m, "\nCPU implementer\t: 0x%02x\n", read_cpuid_id() >> 24);
-	seq_printf(m, "CPU architecture: AArch64\n");
+	seq_puts(m, "CPU architecture: AArch64\n");
 	seq_printf(m, "CPU variant\t: 0x%x\n", (read_cpuid_id() >> 20) & 15);
 	seq_printf(m, "CPU part\t: 0x%03x\n", (read_cpuid_id() >> 4) & 0xfff);
 	seq_printf(m, "CPU revision\t: %d\n", read_cpuid_id() & 15);

Just ignore the checkpatch warning. I prefer the consistency of seq_printf() in this function.

Signature · 1 line

Catalin

Jingoo Han

author

2014-01-29 04:54 UTC #3 lore raw

PATCH 2/2 arm64: kernel: use seq_puts() instead of seq_printf()

Quoted 19 lines · Catalin Marinas

Quoted 18 lines · Jingoo Han
For a constant format without additional arguments, use seq_puts() instead of seq_printf(). Also, it fixes the following checkpatch warning.
WARNING: Prefer seq_puts to seq_printf
Signed-off-by Jingoo Han <[email protected]>
---
 arch/arm64/kernel/setup.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
arch/arm64/kernel/setup.c +0 -0
index c8e9eff..4507691 100644
@@ -416,7 +416,7 @@ static int c_show(struct seq_file *m, void *v)
 			seq_printf(m, "%s ", hwcap_str[i]);
 

Quoted 9 lines

arch/arm64/kernel/setup.c static int c_show(struct seq_file *m, void *v) · line 422
Quoted 6 lines
arch/arm64/kernel/setup.c +1 -1
 	seq_printf(m, "\nCPU implementer\t: 0x%02x\n", read_cpuid_id() >> 24);
-	seq_printf(m, "CPU architecture: AArch64\n");
+	seq_puts(m, "CPU architecture: AArch64\n");
 	seq_printf(m, "CPU variant\t: 0x%x\n", (read_cpuid_id() >> 20) & 15);
 	seq_printf(m, "CPU part\t: 0x%03x\n", (read_cpuid_id() >> 4) & 0xfff);
 	seq_printf(m, "CPU revision\t: %d\n", read_cpuid_id() & 15);
Just ignore the checkpatch warning. I prefer the consistency of seq_printf() in this function.

(+cc Joe Perches, Dan Carpenter)

Personally, I don't like the checkpatch warning. However, I respect your opinion on the consistency. Thank you for your comment.

Best regards,
Jingoo Han

Joe Perches

2014-01-29 05:00 UTC #4 lore raw

PATCH 2/2 arm64: kernel: use seq_puts() instead of seq_printf()

Quoted 30 lines · Jingoo Han

Quoted 19 lines · Catalin Marinas
Quoted 18 lines · Jingoo Han
For a constant format without additional arguments, use seq_puts() instead of seq_printf(). Also, it fixes the following checkpatch warning.
WARNING: Prefer seq_puts to seq_printf
Signed-off-by Jingoo Han <[email protected]>
---
 arch/arm64/kernel/setup.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
arch/arm64/kernel/setup.c +0 -0
index c8e9eff..4507691 100644
@@ -416,7 +416,7 @@ static int c_show(struct seq_file *m, void *v)
 			seq_printf(m, "%s ", hwcap_str[i]);
 
Quoted 9 lines
arch/arm64/kernel/setup.c static int c_show(struct seq_file *m, void *v) · line 422
Quoted 6 lines
arch/arm64/kernel/setup.c +1 -1
 	seq_printf(m, "\nCPU implementer\t: 0x%02x\n", read_cpuid_id() >> 24);
-	seq_printf(m, "CPU architecture: AArch64\n");
+	seq_puts(m, "CPU architecture: AArch64\n");
 	seq_printf(m, "CPU variant\t: 0x%x\n", (read_cpuid_id() >> 20) & 15);
 	seq_printf(m, "CPU part\t: 0x%03x\n", (read_cpuid_id() >> 4) & 0xfff);
 	seq_printf(m, "CPU revision\t: %d\n", read_cpuid_id() & 15);
Just ignore the checkpatch warning. I prefer the consistency of seq_printf() in this function.

Quoted 5 lines

(+cc Joe Perches, Dan Carpenter)
Personally, I don't like the checkpatch warning. However, I respect your opinion on the consistency. Thank you for your comment.

No worries from me.

I'm happy you can ignore checkpatch bleatings
you don't agree with.

It's a stupid little checker.
People are much smarter.

cheers, Joe

Sudeep Holla

2014-01-28 10:11 UTC #5 lore raw

PATCH 1/2 arm64: use num_possible_cpus() instead of NR_CPUS

In reply to Jingoo Han

Quoted 25 lines · Jingoo Han

Use num_possible_cpus() instead of direct use of NR_CPUS. Also, it fixes the following checkpatch warning.
WARNING: usage of NR_CPUS is often wrong - consider using cpu_possible(), num_possible_cpus(), for_each_possible_cpu(), etc
Signed-off-by Jingoo Han <[email protected]>
---
 arch/arm64/kernel/smp.c |   10 +++++-----
 arch/arm64/mm/context.c |    2 +-
 2 files changed, 6 insertions(+), 6 deletions(-)
arch/arm64/kernel/smp.c +1 -1
index 1b7617a..09ff7d4 100644
@@ -320,7 +320,7 @@ void __init smp_init_cpus(void)
 		 * cpu_logical_map was initialized to INVALID_HWID to
 		 * avoid matching valid MPIDR values.
 		 */
-		for (i = 1; (i < cpu) && (i < NR_CPUS); i++) {
+		for (i = 1; (i < cpu) && (i < num_possible_cpus()); i++) {
 			if (cpu_logical_map(i) == hwid) {
 				pr_err("%s: duplicate cpu reg properties in the DT\n",
 					dn->full_name);

arch/arm64/kernel/smp.c void __init smp_init_cpus(void) · line 355

Quoted 5 lines

arch/arm64/kernel/smp.c +1 -1

@@ -352,7 +352,7 @@ void __init smp_init_cpus(void)
 			continue;
 		}
 
-		if (cpu >= NR_CPUS)
+		if (cpu >= num_possible_cpus())

Have you tested this patch ? IIUC this will not work as cpu_possible mask is populated completely and correctly only at the end of this function.

Quoted 16 lines

arch/arm64/kernel/smp.c +2 -2

 			goto next;
 
 		if (cpu_read_ops(dn, cpu) != 0)

@@ -368,9 +368,9 @@ next:
 	}
 
 	/* sanity check */
-	if (cpu > NR_CPUS)
+	if (cpu > num_possible_cpus())
 		pr_warning("no. of cores (%d) greater than configured maximum of %d - clipping\n",
-			   cpu, NR_CPUS);
+			   cpu, num_possible_cpus());
 
 	if (!bootcpu_valid) {
 		pr_err("DT missing boot CPU MPIDR, not enabling secondaries\n");

arch/arm64/kernel/smp.c next: · line 386

Quoted 7 lines

arch/arm64/kernel/smp.c +1 -1

@@ -381,7 +381,7 @@ next:
 	 * All the cpus that made it to the cpu_logical_map have been
 	 * validated so set them as possible cpus.
 	 */
-	for (i = 0; i < NR_CPUS; i++)
+	for (i = 0; i < num_possible_cpus(); i++)
 		if (cpu_logical_map(i) != INVALID_HWID)
 			set_cpu_possible(i, true);

This is what I am referring above, where is possible mask set before this. If it's already populated correctly then we can remove this completely.

Regards,
Sudeep

Jingoo Han

author

2014-01-29 05:31 UTC #6 lore raw

PATCH 1/2 arm64: use num_possible_cpus() instead of NR_CPUS

Quoted 49 lines · Sudeep Holla

Quoted 4 lines · Jingoo Han
Use num_possible_cpus() instead of direct use of NR_CPUS. Also, it fixes the following checkpatch warning.
WARNING: usage of NR_CPUS is often wrong - consider using cpu_possible(), num_possible_cpus(),
for_each_possible_cpu(), etc
Quoted 21 lines
Signed-off-by Jingoo Han <[email protected]>
---
 arch/arm64/kernel/smp.c |   10 +++++-----
 arch/arm64/mm/context.c |    2 +-
 2 files changed, 6 insertions(+), 6 deletions(-)
arch/arm64/kernel/smp.c +1 -1
index 1b7617a..09ff7d4 100644
@@ -320,7 +320,7 @@ void __init smp_init_cpus(void)
 		 * cpu_logical_map was initialized to INVALID_HWID to
 		 * avoid matching valid MPIDR values.
 		 */
-		for (i = 1; (i < cpu) && (i < NR_CPUS); i++) {
+		for (i = 1; (i < cpu) && (i < num_possible_cpus()); i++) {
 			if (cpu_logical_map(i) == hwid) {
 				pr_err("%s: duplicate cpu reg properties in the DT\n",
 					dn->full_name);
arch/arm64/kernel/smp.c void __init smp_init_cpus(void) · line 355
Quoted 5 lines
arch/arm64/kernel/smp.c +1 -1
@@ -352,7 +352,7 @@ void __init smp_init_cpus(void)
 			continue;
 		}
 
-		if (cpu >= NR_CPUS)
+		if (cpu >= num_possible_cpus())
Have you tested this patch ? IIUC this will not work as cpu_possible mask is populated completely and correctly only at the end of this function.
Quoted 13 lines
arch/arm64/kernel/smp.c +2 -2
 			goto next;
 
 		if (cpu_read_ops(dn, cpu) != 0)
@@ -368,9 +368,9 @@ next:
 	}
 
 	/* sanity check */
-	if (cpu > NR_CPUS)
+	if (cpu > num_possible_cpus())
 		pr_warning("no. of cores (%d) greater than configured maximum of %d - clipping\n",
-			   cpu, NR_CPUS);
+			   cpu, num_possible_cpus());
 

Quoted 13 lines

arch/arm64/kernel/smp.c next: · line 386
Quoted 10 lines
arch/arm64/kernel/smp.c +1 -1
 	if (!bootcpu_valid) {
 		pr_err("DT missing boot CPU MPIDR, not enabling secondaries\n");
@@ -381,7 +381,7 @@ next:
 	 * All the cpus that made it to the cpu_logical_map have been
 	 * validated so set them as possible cpus.
 	 */
-	for (i = 0; i < NR_CPUS; i++)
+	for (i = 0; i < num_possible_cpus(); i++)
 		if (cpu_logical_map(i) != INVALID_HWID)
 			set_cpu_possible(i, true);
This is what I am referring above, where is possible mask set before this. If it's already populated correctly then we can remove this completely.

OK, you're right.
Please, ignore this patch.
I really appreciate your comment. :-)

Best regards,
Jingoo Han