[PATCH 0/5] staging-Lustre: Fine-tuning for some function implementations

[PATCH 0/5] staging-Lustre: Fine-tuning for some function implementations

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sun, 1 Jan 2017 17:10:10 +0100

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

Markus Elfring (5):
  llite: Use seq_puts() in three functions
  mgc: Combine two seq_printf() calls into one call in lprocfs_mgc_rd_ir_state()
  obdclass: Use seq_putc() in four functions
  obdclass: Combine two seq_printf() calls into one call in lprocfs_rd_state()
  obdclass: Use seq_puts() in three functions

 drivers/staging/lustre/lustre/llite/lproc_llite.c  | 12 +++++------
 drivers/staging/lustre/lustre/mgc/mgc_request.c    |  5 +----
 drivers/staging/lustre/lustre/obdclass/cl_object.c |  8 ++++----
 .../lustre/lustre/obdclass/lprocfs_status.c        | 23 ++++++++++------------
 4 files changed, 21 insertions(+), 27 deletions(-)

-- 
2.11.0

[PATCH 1/5] staging/lustre/llite: Use seq_puts() in three functions

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sun, 1 Jan 2017 15:30:45 +0100

A string which did not contain a data format specification should be put
into a sequence. Thus use the corresponding function "seq_puts"
so that the data output will be a bit more efficient in these functions.

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/staging/lustre/lustre/llite/lproc_llite.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/staging/lustre/lustre/llite/lproc_llite.c b/drivers/staging/lustre/lustre/llite/lproc_llite.c
index 03682c10fc9e..b195b7eb2883 100644
--- a/drivers/staging/lustre/lustre/llite/lproc_llite.c
+++ b/drivers/staging/lustre/lustre/llite/lproc_llite.c
@@ -1325,8 +1325,8 @@ static int ll_rw_extents_stats_pp_seq_show(struct seq_file *seq, void *v)
 	ktime_get_real_ts64(&now);
 
 	if (!sbi->ll_rw_stats_on) {
-		seq_printf(seq, "disabled\n"
-			   "write anything in this file to activate, then 0 or \"[D/d]isabled\" to deactivate\n");
+		seq_puts(seq, "disabled\n"
+			 "write anything in this file to activate, then 0 or \"[D/d]isabled\" to deactivate\n");
 		return 0;
 	}
 	seq_printf(seq, "snapshot_time:	 %llu.%09lu (secs.usecs)\n",
@@ -1403,8 +1403,8 @@ static int ll_rw_extents_stats_seq_show(struct seq_file *seq, void *v)
 	ktime_get_real_ts64(&now);
 
 	if (!sbi->ll_rw_stats_on) {
-		seq_printf(seq, "disabled\n"
-			   "write anything in this file to activate, then 0 or \"[D/d]isabled\" to deactivate\n");
+		seq_puts(seq, "disabled\n"
+			 "write anything in this file to activate, then 0 or \"[D/d]isabled\" to deactivate\n");
 		return 0;
 	}
 	seq_printf(seq, "snapshot_time:	 %llu.%09lu (secs.usecs)\n",
@@ -1583,8 +1583,8 @@ static int ll_rw_offset_stats_seq_show(struct seq_file *seq, void *v)
 	ktime_get_real_ts64(&now);
 
 	if (!sbi->ll_rw_stats_on) {
-		seq_printf(seq, "disabled\n"
-			   "write anything in this file to activate, then 0 or \"[D/d]isabled\" to deactivate\n");
+		seq_puts(seq, "disabled\n"
+			 "write anything in this file to activate, then 0 or \"[D/d]isabled\" to deactivate\n");
 		return 0;
 	}
 	spin_lock(&sbi->ll_process_lock);
-- 
2.11.0

[PATCH 2/5] staging/lustre/mgc: Combine two seq_printf() calls into one call in lprocfs_mgc_rd_ir_state()

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sun, 1 Jan 2017 15:40:29 +0100

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>
---
 drivers/staging/lustre/lustre/mgc/mgc_request.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/staging/lustre/lustre/mgc/mgc_request.c b/drivers/staging/lustre/lustre/mgc/mgc_request.c
index b9c522a3c7a4..a6ca48d7e96b 100644
--- a/drivers/staging/lustre/lustre/mgc/mgc_request.c
+++ b/drivers/staging/lustre/lustre/mgc/mgc_request.c
@@ -460,11 +460,8 @@ int lprocfs_mgc_rd_ir_state(struct seq_file *m, void *data)
 
 	imp = obd->u.cli.cl_import;
 	ocd = &imp->imp_connect_data;
-
-	seq_printf(m, "imperative_recovery: %s\n",
+	seq_printf(m, "imperative_recovery: %s\nclient_state:\n",
 		   OCD_HAS_FLAG(ocd, IMP_RECOV) ? "ENABLED" : "DISABLED");
-	seq_printf(m, "client_state:\n");
-
 	spin_lock(&config_list_lock);
 	list_for_each_entry(cld, &config_llog_list, cld_list_chain) {
 		if (!cld->cld_recover)
-- 
2.11.0

[PATCH 3/5] staging/lustre/obdclass: Use seq_putc() in four functions

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sun, 1 Jan 2017 16:12:23 +0100

A few single characters (line breaks) 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>
---
 drivers/staging/lustre/lustre/obdclass/cl_object.c      | 4 ++--
 drivers/staging/lustre/lustre/obdclass/lprocfs_status.c | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/staging/lustre/lustre/obdclass/cl_object.c b/drivers/staging/lustre/lustre/obdclass/cl_object.c
index f5d4e23c64b7..277908d66a89 100644
--- a/drivers/staging/lustre/lustre/obdclass/cl_object.c
+++ b/drivers/staging/lustre/lustre/obdclass/cl_object.c
@@ -441,7 +441,7 @@ static int cache_stats_print(const struct cache_stats *cs,
 		seq_printf(m, "%6s", " ");
 		for (i = 0; i < CS_NR; i++)
 			seq_printf(m, "%8s", names[i]);
-		seq_printf(m, "\n");
+		seq_putc(m, '\n');
 	}
 
 	seq_printf(m, "%5.5s:", cs->cs_name);
@@ -516,7 +516,7 @@ locks: ...... ...... ...... ...... ...... [...... ...... ...... ...... ......]
 			   atomic_read(&site->cs_pages_state[i]));
 	seq_printf(m, "]\n");
 	cache_stats_print(&cl_env_stats, m, 0);
-	seq_printf(m, "\n");
+	seq_putc(m, '\n');
 	return 0;
 }
 EXPORT_SYMBOL(cl_site_stats_print);
diff --git a/drivers/staging/lustre/lustre/obdclass/lprocfs_status.c b/drivers/staging/lustre/lustre/obdclass/lprocfs_status.c
index 2c99717b0aba..3f6fcab5a1fc 100644
--- a/drivers/staging/lustre/lustre/obdclass/lprocfs_status.c
+++ b/drivers/staging/lustre/lustre/obdclass/lprocfs_status.c
@@ -878,7 +878,7 @@ int lprocfs_at_hist_helper(struct seq_file *m, struct adaptive_timeout *at)
 
 	for (i = 0; i < AT_BINS; i++)
 		seq_printf(m, "%3u ", at->at_hist[i]);
-	seq_printf(m, "\n");
+	seq_putc(m, '\n');
 	return 0;
 }
 EXPORT_SYMBOL(lprocfs_at_hist_helper);
@@ -946,7 +946,7 @@ int lprocfs_rd_connect_flags(struct seq_file *m, void *data)
 	flags = obd->u.cli.cl_import->imp_connect_data.ocd_connect_flags;
 	seq_printf(m, "flags=%#llx\n", flags);
 	obd_connect_seq_flags2str(m, flags, "\n");
-	seq_printf(m, "\n");
+	seq_putc(m, '\n');
 	up_read(&obd->u.cli.cl_sem);
 	return 0;
 }
-- 
2.11.0

[PATCH 4/5] staging/lustre/obdclass: Combine two seq_printf() calls into one call in lprocfs_rd_state()

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sun, 1 Jan 2017 16:26:36 +0100

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>
---
 drivers/staging/lustre/lustre/obdclass/lprocfs_status.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/staging/lustre/lustre/obdclass/lprocfs_status.c b/drivers/staging/lustre/lustre/obdclass/lprocfs_status.c
index 3f6fcab5a1fc..a167cbe8a50e 100644
--- a/drivers/staging/lustre/lustre/obdclass/lprocfs_status.c
+++ b/drivers/staging/lustre/lustre/obdclass/lprocfs_status.c
@@ -853,10 +853,8 @@ int lprocfs_rd_state(struct seq_file *m, void *data)
 		return rc;
 
 	imp = obd->u.cli.cl_import;
-
-	seq_printf(m, "current_state: %s\n",
+	seq_printf(m, "current_state: %s\nstate_history:\n",
 		   ptlrpc_import_state_name(imp->imp_state));
-	seq_printf(m, "state_history:\n");
 	k = imp->imp_state_hist_idx;
 	for (j = 0; j < IMP_STATE_HIST_LEN; j++) {
 		struct import_state_hist *ish =
-- 
2.11.0

[PATCH 5/5] staging/lustre/obdclass: Use seq_puts() in three functions

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sun, 1 Jan 2017 16:45:32 +0100

A string which did not contain a data format specification should be put
into a sequence. Thus use the corresponding function "seq_puts"
so that the data output will be a bit more efficient in these functions.

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/staging/lustre/lustre/obdclass/cl_object.c      |  4 ++--
 drivers/staging/lustre/lustre/obdclass/lprocfs_status.c | 15 +++++++--------
 2 files changed, 9 insertions(+), 10 deletions(-)

diff --git a/drivers/staging/lustre/lustre/obdclass/cl_object.c b/drivers/staging/lustre/lustre/obdclass/cl_object.c
index 277908d66a89..3e00aa4747b8 100644
--- a/drivers/staging/lustre/lustre/obdclass/cl_object.c
+++ b/drivers/staging/lustre/lustre/obdclass/cl_object.c
@@ -510,11 +510,11 @@ locks: ...... ...... ...... ...... ...... [...... ...... ...... ...... ......]
  */
 	lu_site_stats_print(&site->cs_lu, m);
 	cache_stats_print(&site->cs_pages, m, 1);
-	seq_printf(m, " [");
+	seq_puts(m, " [");
 	for (i = 0; i < ARRAY_SIZE(site->cs_pages_state); ++i)
 		seq_printf(m, "%s: %u ", pstate[i],
 			   atomic_read(&site->cs_pages_state[i]));
-	seq_printf(m, "]\n");
+	seq_puts(m, "]\n");
 	cache_stats_print(&cl_env_stats, m, 0);
 	seq_putc(m, '\n');
 	return 0;
diff --git a/drivers/staging/lustre/lustre/obdclass/lprocfs_status.c b/drivers/staging/lustre/lustre/obdclass/lprocfs_status.c
index a167cbe8a50e..3ce590cdd957 100644
--- a/drivers/staging/lustre/lustre/obdclass/lprocfs_status.c
+++ b/drivers/staging/lustre/lustre/obdclass/lprocfs_status.c
@@ -649,7 +649,7 @@ static int obd_import_flags2str(struct obd_import *imp, struct seq_file *m)
 	bool first = true;
 
 	if (imp->imp_obd->obd_no_recov) {
-		seq_printf(m, "no_recov");
+		seq_puts(m, "no_recov");
 		first = false;
 	}
 
@@ -715,15 +715,14 @@ int lprocfs_rd_import(struct seq_file *m, void *data)
 		   imp->imp_connect_data.ocd_instance);
 	obd_connect_seq_flags2str(m, imp->imp_connect_data.ocd_connect_flags,
 				  ", ");
-	seq_printf(m, " ]\n");
+	seq_puts(m, " ]\n");
 	obd_connect_data_seqprint(m, ocd);
-	seq_printf(m, "    import_flags: [ ");
+	seq_puts(m, "    import_flags: [ ");
 	obd_import_flags2str(imp, m);
-
-	seq_printf(m,
-		   " ]\n"
-		   "    connection:\n"
-		   "       failover_nids: [ ");
+	seq_puts(m,
+		 " ]\n"
+		 "    connection:\n"
+		 "       failover_nids: [ ");
 	spin_lock(&imp->imp_lock);
 	j = 0;
 	list_for_each_entry(conn, &imp->imp_conn_list, oic_item) {
-- 
2.11.0

Re: [PATCH 2/5] staging/lustre/mgc: Combine two seq_printf() calls into one call in lprocfs_mgc_rd_ir_state()

[Oleg Drokin]

On Jan 1, 2017, at 11:35 AM, SF Markus Elfring wrote:

> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Sun, 1 Jan 2017 15:40:29 +0100
> 
> 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>
> ---
> drivers/staging/lustre/lustre/mgc/mgc_request.c | 5 +----
> 1 file changed, 1 insertion(+), 4 deletions(-)
> 
> diff --git a/drivers/staging/lustre/lustre/mgc/mgc_request.c b/drivers/staging/lustre/lustre/mgc/mgc_request.c
> index b9c522a3c7a4..a6ca48d7e96b 100644
> --- a/drivers/staging/lustre/lustre/mgc/mgc_request.c
> +++ b/drivers/staging/lustre/lustre/mgc/mgc_request.c
> @@ -460,11 +460,8 @@ int lprocfs_mgc_rd_ir_state(struct seq_file *m, void *data)
> 
> 	imp = obd->u.cli.cl_import;
> 	ocd = &imp->imp_connect_data;
> -
> -	seq_printf(m, "imperative_recovery: %s\n",
> +	seq_printf(m, "imperative_recovery: %s\nclient_state:\n",
> 		   OCD_HAS_FLAG(ocd, IMP_RECOV) ? "ENABLED" : "DISABLED");
> -	seq_printf(m, "client_state:\n");
> -

Ugh, do we really need this?
I know it saves one call to seq_printf, but this is not a super performance-critical
code, and two calls are actually easier to read, don't you think?

> 	spin_lock(&config_list_lock);
> 	list_for_each_entry(cld, &config_llog_list, cld_list_chain) {
> 		if (!cld->cld_recover)
> -- 
> 2.11.0

Re: [PATCH 4/5] staging/lustre/obdclass: Combine two seq_printf() calls into one call in lprocfs_rd_state()

[Oleg Drokin]

On Jan 1, 2017, at 11:38 AM, SF Markus Elfring wrote:

> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Sun, 1 Jan 2017 16:26:36 +0100
> 
> 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>
> ---
> drivers/staging/lustre/lustre/obdclass/lprocfs_status.c | 4 +---
> 1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/drivers/staging/lustre/lustre/obdclass/lprocfs_status.c b/drivers/staging/lustre/lustre/obdclass/lprocfs_status.c
> index 3f6fcab5a1fc..a167cbe8a50e 100644
> --- a/drivers/staging/lustre/lustre/obdclass/lprocfs_status.c
> +++ b/drivers/staging/lustre/lustre/obdclass/lprocfs_status.c
> @@ -853,10 +853,8 @@ int lprocfs_rd_state(struct seq_file *m, void *data)
> 		return rc;
> 
> 	imp = obd->u.cli.cl_import;
> -
> -	seq_printf(m, "current_state: %s\n",
> +	seq_printf(m, "current_state: %s\nstate_history:\n",
> 		   ptlrpc_import_state_name(imp->imp_state));
> -	seq_printf(m, "state_history:\n");

same as in that other patch - this actually makes the code a bit harder to read,
what's the perceived benefit to make a change like this?

> 	k = imp->imp_state_hist_idx;
> 	for (j = 0; j < IMP_STATE_HIST_LEN; j++) {
> 		struct import_state_hist *ish =
> -- 
> 2.11.0

Re: [PATCH 2/5] staging/lustre/mgc: Combine two seq_printf() calls into one call in lprocfs_mgc_rd_ir_state()

[Greg Kroah-Hartman]

On Fri, Jan 13, 2017 at 01:58:37AM -0500, Oleg Drokin wrote:
> 
> On Jan 1, 2017, at 11:35 AM, SF Markus Elfring wrote:

<snip>

Oleg, please realize that I have blacklisted this patch author, and
don't take contributions from them, unless you, or someone else wishes
to properly review and pass them on.  I've found that personally, it's
just a waste of time working with them, but you are free to do whatever
you wish...

good luck!

greg k-h