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

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

[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 <jg1.han@samsung.com>
---
 arch/arm64/kernel/smp.c |   10 +++++-----
 arch/arm64/mm/context.c |    2 +-
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/arch/arm64/kernel/smp.c b/arch/arm64/kernel/smp.c
index 1b7617a..09ff7d4 100644
--- a/arch/arm64/kernel/smp.c
+++ b/arch/arm64/kernel/smp.c
@@ -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);
 }
diff --git a/arch/arm64/mm/context.c b/arch/arm64/mm/context.c
index baa758d..3ef960a 100644
--- a/arch/arm64/mm/context.c
+++ b/arch/arm64/mm/context.c
@@ -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);
-- 
1.7.10.4

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

[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 <jg1.han@samsung.com>
---
 arch/arm64/kernel/setup.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm64/kernel/setup.c b/arch/arm64/kernel/setup.c
index c8e9eff..4507691 100644
--- a/arch/arm64/kernel/setup.c
+++ b/arch/arm64/kernel/setup.c
@@ -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);
-- 
1.7.10.4

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

[Sudeep Holla]

On 28/01/14 01:35, Jingoo Han wrote:
> 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 <jg1.han@samsung.com>
> ---
>  arch/arm64/kernel/smp.c |   10 +++++-----
>  arch/arm64/mm/context.c |    2 +-
>  2 files changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/arch/arm64/kernel/smp.c b/arch/arm64/kernel/smp.c
> index 1b7617a..09ff7d4 100644
> --- a/arch/arm64/kernel/smp.c
> +++ b/arch/arm64/kernel/smp.c
> @@ -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())

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.

>  			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);

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

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

[Catalin Marinas]

On Tue, Jan 28, 2014 at 01:36:18AM +0000, Jingoo Han wrote:
> 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 <jg1.han@samsung.com>
> ---
>  arch/arm64/kernel/setup.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/arm64/kernel/setup.c b/arch/arm64/kernel/setup.c
> index c8e9eff..4507691 100644
> --- a/arch/arm64/kernel/setup.c
> +++ b/arch/arm64/kernel/setup.c
> @@ -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.

-- 
Catalin

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

[Jingoo Han]

On Wednesday, January 29, 2014 12:52 AM, Catalin Marinas wrote:
> On Tue, Jan 28, 2014 at 01:36:18AM +0000, Jingoo Han wrote:
> > 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 <jg1.han@samsung.com>
> > ---
> >  arch/arm64/kernel/setup.c |    2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/arch/arm64/kernel/setup.c b/arch/arm64/kernel/setup.c
> > index c8e9eff..4507691 100644
> > --- a/arch/arm64/kernel/setup.c
> > +++ b/arch/arm64/kernel/setup.c
> > @@ -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.

(+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

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

[Joe Perches]

On Wed, 2014-01-29 at 13:54 +0900, Jingoo Han wrote:
> On Wednesday, January 29, 2014 12:52 AM, Catalin Marinas wrote:
> > On Tue, Jan 28, 2014 at 01:36:18AM +0000, Jingoo Han wrote:
> > > 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 <jg1.han@samsung.com>
> > > ---
> > >  arch/arm64/kernel/setup.c |    2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/arch/arm64/kernel/setup.c b/arch/arm64/kernel/setup.c
> > > index c8e9eff..4507691 100644
> > > --- a/arch/arm64/kernel/setup.c
> > > +++ b/arch/arm64/kernel/setup.c
> > > @@ -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.
> 
> (+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

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

[Jingoo Han]

On Tuesday, January 28, 2014 7:11 PM, Sudeep Holla wrote:
> On 28/01/14 01:35, Jingoo Han wrote:
> > 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 <jg1.han@samsung.com>
> > ---
> >  arch/arm64/kernel/smp.c |   10 +++++-----
> >  arch/arm64/mm/context.c |    2 +-
> >  2 files changed, 6 insertions(+), 6 deletions(-)
> >
> > diff --git a/arch/arm64/kernel/smp.c b/arch/arm64/kernel/smp.c
> > index 1b7617a..09ff7d4 100644
> > --- a/arch/arm64/kernel/smp.c
> > +++ b/arch/arm64/kernel/smp.c
> > @@ -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())
> 
> 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.
> 
> >  			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);
> 
> 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