[PATCH 0/6] IA64: Fine-tuning for five function implementations

[PATCH 0/6] IA64: Fine-tuning for five function implementations

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sat, 22 Oct 2016 21:27:32 +0200

A few update suggestions were taken into account
from static source code analysis.

Markus Elfring (6):
  sba_iommu: Use seq_puts() in ioc_show()
  sba_iommu: Combine four seq_printf() calls into two calls in ioc_show()
  simserial: Use seq_puts() in rs_proc_show()
  setup: Use seq_putc() in show_cpuinfo()
  sn_hwperf: Use seq_puts() in sn_topology_show()
  sn2_smp: Combine two seq_printf() calls into one call in sn2_ptc_seq_show()

 arch/ia64/hp/common/sba_iommu.c     | 16 +++++++++++-----
 arch/ia64/hp/sim/simserial.c        |  2 +-
 arch/ia64/kernel/setup.c            |  3 +--
 arch/ia64/sn/kernel/sn2/sn2_smp.c   |  9 ++++-----
 arch/ia64/sn/kernel/sn2/sn_hwperf.c |  6 +++---
 5 files changed, 20 insertions(+), 16 deletions(-)

-- 
2.10.1

[PATCH 1/6] IA64-sba_iommu: Use seq_puts() in ioc_show()

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sat, 22 Oct 2016 20:20:24 +0200

A string which did not contain a data format specification should be put
into a sequence. Thus use the corresponding function "seq_puts".

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 arch/ia64/hp/common/sba_iommu.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/ia64/hp/common/sba_iommu.c b/arch/ia64/hp/common/sba_iommu.c
index 630ee80..282f5c1 100644
--- a/arch/ia64/hp/common/sba_iommu.c
+++ b/arch/ia64/hp/common/sba_iommu.c
@@ -1930,7 +1930,7 @@ ioc_show(struct seq_file *s, void *v)
 	}
 #endif
 #ifndef ALLOW_IOV_BYPASS
-	 seq_printf(s, "IOVA bypass disabled\n");
+	seq_puts(s, "IOVA bypass disabled\n");
 #endif
 	return 0;
 }
-- 
2.10.1

[PATCH 2/6] IA64-sba_iommu: Combine four seq_printf() calls into two calls in ioc_show()

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sat, 22 Oct 2016 20:22:46 +0200

Some data were printed into a sequence by four separate function calls.
Print the same data by two function calls instead.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 arch/ia64/hp/common/sba_iommu.c | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/arch/ia64/hp/common/sba_iommu.c b/arch/ia64/hp/common/sba_iommu.c
index 282f5c1..0688441 100644
--- a/arch/ia64/hp/common/sba_iommu.c
+++ b/arch/ia64/hp/common/sba_iommu.c
@@ -1906,14 +1906,20 @@ ioc_show(struct seq_file *s, void *v)
 	if (ioc->node != NUMA_NO_NODE)
 		seq_printf(s, "NUMA node       : %d\n", ioc->node);
 #endif
-	seq_printf(s, "IOVA size       : %ld MB\n", ((ioc->pdir_size >> 3) * iovp_size)/(1024*1024));
-	seq_printf(s, "IOVA page size  : %ld kb\n", iovp_size/1024);
+	seq_printf(s,
+		   "IOVA size       : %ld MB\n"
+		   "IOVA page size  : %ld kb\n",
+		   ((ioc->pdir_size >> 3) * iovp_size) / (1024 * 1024)),
+		   iovp_size / 1024);
 
 	for (i = 0; i < (ioc->res_size / sizeof(unsigned long)); ++i, ++res_ptr)
 		used += hweight64(*res_ptr);
 
-	seq_printf(s, "PDIR size       : %d entries\n", ioc->pdir_size >> 3);
-	seq_printf(s, "PDIR used       : %d entries\n", used);
+	seq_printf(s,
+		   "PDIR size       : %d entries\n"
+		   "PDIR used       : %d entries\n",
+		   ioc->pdir_size >> 3,
+		   used);
 
 #ifdef PDIR_SEARCH_TIMING
 	{
-- 
2.10.1

[PATCH 3/6] IA64-simserial: Use seq_puts() in rs_proc_show()

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sat, 22 Oct 2016 20:35:28 +0200

A string which did not contain a data format specification should be put
into a sequence. Thus use the corresponding function "seq_puts".

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 arch/ia64/hp/sim/simserial.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/ia64/hp/sim/simserial.c b/arch/ia64/hp/sim/simserial.c
index 21fd50d..59b8d6d 100644
--- a/arch/ia64/hp/sim/simserial.c
+++ b/arch/ia64/hp/sim/simserial.c
@@ -439,7 +439,7 @@ static int rs_proc_show(struct seq_file *m, void *v)
 {
 	int i;
 
-	seq_printf(m, "simserinfo:1.0\n");
+	seq_puts(m, "simserinfo:1.0\n");
 	for (i = 0; i < NR_PORTS; i++)
 		seq_printf(m, "%d: uart:16550 port:3F8 irq:%d\n",
 		       i, rs_table[i].irq);
-- 
2.10.1

[PATCH 4/6] IA64-setup: Use seq_putc() in show_cpuinfo()

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sat, 22 Oct 2016 20:42:20 +0200

A single character (line break) should be put into a sequence.
Thus use the corresponding function "seq_putc".

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 arch/ia64/kernel/setup.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/arch/ia64/kernel/setup.c b/arch/ia64/kernel/setup.c
index 7ec7acc..28741f7 100644
--- a/arch/ia64/kernel/setup.c
+++ b/arch/ia64/kernel/setup.c
@@ -705,8 +705,7 @@ show_cpuinfo (struct seq_file *m, void *v)
 			   "thread id  : %u\n",
 			   c->core_id, c->thread_id);
 #endif
-	seq_printf(m,"\n");
-
+	seq_putc(m, '\n');
 	return 0;
 }
 
-- 
2.10.1

[PATCH 5/6] IA64-sn_hwperf: Use seq_puts() in sn_topology_show()

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sat, 22 Oct 2016 20:53:38 +0200

A string which did not contain a data format specification should be put
into a sequence. Thus use the corresponding function "seq_puts".

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 arch/ia64/sn/kernel/sn2/sn_hwperf.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/ia64/sn/kernel/sn2/sn_hwperf.c b/arch/ia64/sn/kernel/sn2/sn_hwperf.c
index b999257..435002a 100644
--- a/arch/ia64/sn/kernel/sn2/sn_hwperf.c
+++ b/arch/ia64/sn/kernel/sn2/sn_hwperf.c
@@ -400,9 +400,9 @@ static int sn_topology_show(struct seq_file *s, void *d)
 	int nasid_msb;
 
 	if (obj == objs) {
-		seq_printf(s, "# sn_topology version 2\n");
-		seq_printf(s, "# objtype ordinal location partition"
-			" [attribute value [, ...]]\n");
+		seq_puts(s,
+			 "# sn_topology version 2\n"
+			 "# objtype ordinal location partition [attribute value [, ...]]\n");
 
 		if (ia64_sn_get_sn_info(0,
 			&shubtype, &nasid_mask, &nasid_shift, &system_size,
-- 
2.10.1

[PATCH 6/6] IA64-sn2_smp: Combine two seq_printf() calls into one call in sn2_ptc_seq_show()

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sat, 22 Oct 2016 21:19:03 +0200

Some data were printed into a sequence by two separate function calls.
Print the same data by a single function call instead.

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 arch/ia64/sn/kernel/sn2/sn2_smp.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/arch/ia64/sn/kernel/sn2/sn2_smp.c b/arch/ia64/sn/kernel/sn2/sn2_smp.c
index c98dc96..1beac3c 100644
--- a/arch/ia64/sn/kernel/sn2/sn2_smp.c
+++ b/arch/ia64/sn/kernel/sn2/sn2_smp.c
@@ -494,12 +494,11 @@ static int sn2_ptc_seq_show(struct seq_file *file, void *data)
 	int cpu;
 
 	cpu = *(loff_t *) data;
-
-	if (!cpu) {
+	if (!cpu)
 		seq_printf(file,
-			   "# cpu ptc_l newrid ptc_flushes nodes_flushed deadlocks lock_nsec shub_nsec shub_nsec_max not_my_mm deadlock2 ipi_fluches ipi_nsec\n");
-		seq_printf(file, "# ptctest %d, flushopt %d\n", sn2_ptctest, sn2_flush_opt);
-	}
+			   "# cpu ptc_l newrid ptc_flushes nodes_flushed deadlocks lock_nsec shub_nsec shub_nsec_max not_my_mm deadlock2 ipi_fluches ipi_nsec\n"
+			   "# ptctest %d, flushopt %d\n",
+			   sn2_ptctest, sn2_flush_opt);
 
 	if (cpu < nr_cpu_ids && cpu_online(cpu)) {
 		stat = &per_cpu(ptcstats, cpu);
-- 
2.10.1

Re: [PATCH 6/6] IA64-sn2_smp: Combine two seq_printf() calls into one call in sn2_ptc_seq_show()

[Joe Perches]

On Sat, 2016-10-22 at 21:58 +0200, SF Markus Elfring wrote:
> Some data were printed into a sequence by two separate function calls.
> Print the same data by a single function call instead.
[]
> diff --git a/arch/ia64/sn/kernel/sn2/sn2_smp.c b/arch/ia64/sn/kernel/sn2/sn2_smp.c
[]
> @@ -494,12 +494,11 @@ static int sn2_ptc_seq_show(struct seq_file *file, void *data)
>  	int cpu;
>  
>  	cpu = *(loff_t *) data;
> -
> -	if (!cpu) {
> +	if (!cpu)
>  		seq_printf(file,
> -			   "# cpu ptc_l newrid ptc_flushes nodes_flushed deadlocks lock_nsec shub_nsec shub_nsec_max not_my_mm deadlock2 ipi_fluches ipi_nsec\n");
> -		seq_printf(file, "# ptctest %d, flushopt %d\n", sn2_ptctest, sn2_flush_opt);
> -	}
> +			   "# cpu ptc_l newrid ptc_flushes nodes_flushed deadlocks lock_nsec shub_nsec shub_nsec_max not_my_mm deadlock2 ipi_fluches ipi_nsec\n"
> +			   "# ptctest %d, flushopt %d\n",
> +			   sn2_ptctest, sn2_flush_opt);
>  
>  	if (cpu < nr_cpu_ids && cpu_online(cpu)) {
>  		stat = &per_cpu(ptcstats, cpu);

Please think more.

printf has to inspect character by character looking for
a vsprintf % character and 0 termination.

seq_puts does a strlen then memcpy.

Which is faster?
When is it better to call 2 functions?
When does readability matter more than efficiency?

Re: [PATCH 6/6] IA64-sn2_smp: Combine two seq_printf() calls into one call in sn2_ptc_seq_show()

[SF Markus Elfring]

>> @@ -494,12 +494,11 @@ static int sn2_ptc_seq_show(struct seq_file *file, void *data)
>>  	int cpu;
>>  
>>  	cpu = *(loff_t *) data;
>> -
>> -	if (!cpu) {
>> +	if (!cpu)
>>  		seq_printf(file,
>> -			   "# cpu ptc_l newrid ptc_flushes nodes_flushed deadlocks lock_nsec shub_nsec shub_nsec_max not_my_mm deadlock2 ipi_fluches ipi_nsec\n");
>> -		seq_printf(file, "# ptctest %d, flushopt %d\n", sn2_ptctest, sn2_flush_opt);
>> -	}
>> +			   "# cpu ptc_l newrid ptc_flushes nodes_flushed deadlocks lock_nsec shub_nsec shub_nsec_max not_my_mm deadlock2 ipi_fluches ipi_nsec\n"
>> +			   "# ptctest %d, flushopt %d\n",
>> +			   sn2_ptctest, sn2_flush_opt);
>>  
>>  	if (cpu < nr_cpu_ids && cpu_online(cpu)) {
>>  		stat = &per_cpu(ptcstats, cpu);
> 
> Please think more.
> 
> printf has to inspect character by character looking for
> a vsprintf % character and 0 termination.
> 
> seq_puts does a strlen then memcpy.

Would you prefer to use this function also at this source code place?

Regards,
Markus

Re: [PATCH 6/6] IA64-sn2_smp: Combine two seq_printf() calls into one call in sn2_ptc_seq_show()

[Julia Lawall]

On Sat, 22 Oct 2016, SF Markus Elfring wrote:

> >> @@ -494,12 +494,11 @@ static int sn2_ptc_seq_show(struct seq_file *file, void *data)
> >>  	int cpu;
> >>
> >>  	cpu = *(loff_t *) data;
> >> -
> >> -	if (!cpu) {
> >> +	if (!cpu)
> >>  		seq_printf(file,
> >> -			   "# cpu ptc_l newrid ptc_flushes nodes_flushed deadlocks lock_nsec shub_nsec shub_nsec_max not_my_mm deadlock2 ipi_fluches ipi_nsec\n");
> >> -		seq_printf(file, "# ptctest %d, flushopt %d\n", sn2_ptctest, sn2_flush_opt);
> >> -	}
> >> +			   "# cpu ptc_l newrid ptc_flushes nodes_flushed deadlocks lock_nsec shub_nsec shub_nsec_max not_my_mm deadlock2 ipi_fluches ipi_nsec\n"
> >> +			   "# ptctest %d, flushopt %d\n",
> >> +			   sn2_ptctest, sn2_flush_opt);
> >>
> >>  	if (cpu < nr_cpu_ids && cpu_online(cpu)) {
> >>  		stat = &per_cpu(ptcstats, cpu);
> >
> > Please think more.
> >
> > printf has to inspect character by character looking for
> > a vsprintf % character and 0 termination.
> >
> > seq_puts does a strlen then memcpy.
>
> Would you prefer to use this function also at this source code place?

I think that he is expecting that you will figure out the answer by
yourself.

julia

Re: IA64-sn2_smp: Combine two seq_printf() calls into one call in sn2_ptc_seq_show()

[SF Markus Elfring]

>> Would you prefer to use this function also at this source code place?
> 
> I think that he is expecting that you will figure out the answer by yourself.

I know some answer possibilities for the potential usage of the
function "seq_puts" also within the implementation of the function "sn2_ptc_seq_show".
I am unsure about the consensus of the involved developers for this
software module as usual.

Regards,
Markus