[PATCH 00/10] fs-DLM: Fine-tuning for several function implementations

[PATCH 00/10] fs-DLM: Fine-tuning for several function implementations

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sat, 6 May 2017 10:15:25 +0200

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

Markus Elfring (10):
  Replace six seq_puts() calls by seq_putc()
  Add spaces for better code readability
  Improve a size determination in table_seq_start()
  Use kcalloc() in dlm_scan_waiters()
  Improve a size determination in dlm_recover_waiters_pre()
  Delete an error message for a failed memory allocation in dlm_recover_waiters_pre()
  Use kmalloc_array() in make_member_array()
  Use kcalloc() in two functions
  Improve a size determination in two functions
  Delete an unnecessary variable initialisation in dlm_ls_start()

 fs/dlm/debug_fs.c | 25 ++++++++++++-------------
 fs/dlm/lock.c     |  8 +++-----
 fs/dlm/member.c   | 15 ++++++---------
 3 files changed, 21 insertions(+), 27 deletions(-)

-- 
2.12.2

[PATCH 01/10] dlm: Replace six seq_puts() calls by seq_putc()

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sat, 6 May 2017 08:12:31 +0200

Six 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>
---
 fs/dlm/debug_fs.c | 13 ++++++-------
 1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/fs/dlm/debug_fs.c b/fs/dlm/debug_fs.c
index ca7089aeadab..464218c6b502 100644
--- a/fs/dlm/debug_fs.c
+++ b/fs/dlm/debug_fs.c
@@ -68,7 +68,7 @@ static void print_format1_lock(struct seq_file *s, struct dlm_lkb *lkb,
 	if (lkb->lkb_wait_type)
 		seq_printf(s, " wait_type: %d", lkb->lkb_wait_type);
 
-	seq_puts(s, "\n");
+	seq_putc(s, '\n');
 }
 
 static void print_format1(struct dlm_rsb *res, struct seq_file *s)
@@ -111,7 +111,7 @@ static void print_format1(struct dlm_rsb *res, struct seq_file *s)
 		}
 		if (rsb_flag(res, RSB_VALNOTVALID))
 			seq_puts(s, " (INVALID)");
-		seq_puts(s, "\n");
+		seq_putc(s, '\n');
 		if (seq_has_overflowed(s))
 			goto out;
 	}
@@ -156,7 +156,7 @@ static void print_format1(struct dlm_rsb *res, struct seq_file *s)
 			   lkb->lkb_id, print_lockmode(lkb->lkb_rqmode));
 		if (lkb->lkb_wait_type)
 			seq_printf(s, " wait_type: %d", lkb->lkb_wait_type);
-		seq_puts(s, "\n");
+		seq_putc(s, '\n');
 		if (seq_has_overflowed(s))
 			goto out;
 	}
@@ -287,7 +287,7 @@ static void print_format3(struct dlm_rsb *r, struct seq_file *s)
 		else
 			seq_printf(s, " %02x", (unsigned char)r->res_name[i]);
 	}
-	seq_puts(s, "\n");
+	seq_putc(s, '\n');
 	if (seq_has_overflowed(s))
 		goto out;
 
@@ -298,7 +298,7 @@ static void print_format3(struct dlm_rsb *r, struct seq_file *s)
 
 	for (i = 0; i < lvblen; i++)
 		seq_printf(s, " %02x", (unsigned char)r->res_lvbptr[i]);
-	seq_puts(s, "\n");
+	seq_putc(s, '\n');
 	if (seq_has_overflowed(s))
 		goto out;
 
@@ -361,8 +361,7 @@ static void print_format4(struct dlm_rsb *r, struct seq_file *s)
 		else
 			seq_printf(s, " %02x", (unsigned char)r->res_name[i]);
 	}
-	seq_puts(s, "\n");
-
+	seq_putc(s, '\n');
 	unlock_rsb(r);
 }
 
-- 
2.12.2

[PATCH 02/10] dlm: Add spaces for better code readability

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sat, 6 May 2017 08:22:35 +0200

The script "checkpatch.pl" pointed information out like the following.

CHECK: spaces preferred around that '+' (ctx:VxV)

Thus fix the affected source code places.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 fs/dlm/debug_fs.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/fs/dlm/debug_fs.c b/fs/dlm/debug_fs.c
index 464218c6b502..551e0f8dbe0d 100644
--- a/fs/dlm/debug_fs.c
+++ b/fs/dlm/debug_fs.c
@@ -741,7 +741,7 @@ void dlm_delete_debug_file(struct dlm_ls *ls)
 
 int dlm_create_debug_file(struct dlm_ls *ls)
 {
-	char name[DLM_LOCKSPACE_LEN+8];
+	char name[DLM_LOCKSPACE_LEN + 8];
 
 	/* format 1 */
 
@@ -756,7 +756,7 @@ int dlm_create_debug_file(struct dlm_ls *ls)
 	/* format 2 */
 
 	memset(name, 0, sizeof(name));
-	snprintf(name, DLM_LOCKSPACE_LEN+8, "%s_locks", ls->ls_name);
+	snprintf(name, DLM_LOCKSPACE_LEN + 8, "%s_locks", ls->ls_name);
 
 	ls->ls_debug_locks_dentry = debugfs_create_file(name,
 							S_IFREG | S_IRUGO,
@@ -769,7 +769,7 @@ int dlm_create_debug_file(struct dlm_ls *ls)
 	/* format 3 */
 
 	memset(name, 0, sizeof(name));
-	snprintf(name, DLM_LOCKSPACE_LEN+8, "%s_all", ls->ls_name);
+	snprintf(name, DLM_LOCKSPACE_LEN + 8, "%s_all", ls->ls_name);
 
 	ls->ls_debug_all_dentry = debugfs_create_file(name,
 						      S_IFREG | S_IRUGO,
@@ -782,7 +782,7 @@ int dlm_create_debug_file(struct dlm_ls *ls)
 	/* format 4 */
 
 	memset(name, 0, sizeof(name));
-	snprintf(name, DLM_LOCKSPACE_LEN+8, "%s_toss", ls->ls_name);
+	snprintf(name, DLM_LOCKSPACE_LEN + 8, "%s_toss", ls->ls_name);
 
 	ls->ls_debug_toss_dentry = debugfs_create_file(name,
 						       S_IFREG | S_IRUGO,
@@ -793,7 +793,7 @@ int dlm_create_debug_file(struct dlm_ls *ls)
 		goto fail;
 
 	memset(name, 0, sizeof(name));
-	snprintf(name, DLM_LOCKSPACE_LEN+8, "%s_waiters", ls->ls_name);
+	snprintf(name, DLM_LOCKSPACE_LEN + 8, "%s_waiters", ls->ls_name);
 
 	ls->ls_debug_waiters_dentry = debugfs_create_file(name,
 							  S_IFREG | S_IRUGO,
-- 
2.12.2

[PATCH 03/10] dlm: Improve a size determination in table_seq_start()

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sat, 6 May 2017 08:34:27 +0200

Replace the specification of a data structure by a pointer dereference
as the parameter for the operator "sizeof" to make the corresponding size
determination a bit safer according to the Linux coding style convention.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 fs/dlm/debug_fs.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/dlm/debug_fs.c b/fs/dlm/debug_fs.c
index 551e0f8dbe0d..fa08448e35dd 100644
--- a/fs/dlm/debug_fs.c
+++ b/fs/dlm/debug_fs.c
@@ -435,7 +435,7 @@ static void *table_seq_start(struct seq_file *seq, loff_t *pos)
 	if (bucket >= ls->ls_rsbtbl_size)
 		return NULL;
 
-	ri = kzalloc(sizeof(struct rsbtbl_iter), GFP_NOFS);
+	ri = kzalloc(sizeof(*ri), GFP_NOFS);
 	if (!ri)
 		return NULL;
 	if (n == 0)
-- 
2.12.2

[PATCH 04/10] dlm: Use kcalloc() in dlm_scan_waiters()

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sat, 6 May 2017 08:38:49 +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 "kcalloc".

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 fs/dlm/lock.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/dlm/lock.c b/fs/dlm/lock.c
index 6df332296c66..fd6fe55bedae 100644
--- a/fs/dlm/lock.c
+++ b/fs/dlm/lock.c
@@ -1426,7 +1426,7 @@ void dlm_scan_waiters(struct dlm_ls *ls)
 
 		if (!num_nodes) {
 			num_nodes = ls->ls_num_nodes;
-			warned = kzalloc(num_nodes * sizeof(int), GFP_KERNEL);
+			warned = kcalloc(num_nodes, sizeof(int), GFP_KERNEL);
 		}
 		if (!warned)
 			continue;
-- 
2.12.2

[PATCH 05/10] dlm: Improve a size determination in dlm_recover_waiters_pre()

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sat, 6 May 2017 08:48:29 +0200

Replace the specification of a data structure by a pointer dereference
as the parameter for the operator "sizeof" to make the corresponding size
determination a bit safer according to the Linux coding style convention.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 fs/dlm/lock.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/dlm/lock.c b/fs/dlm/lock.c
index fd6fe55bedae..ffadb817ad39 100644
--- a/fs/dlm/lock.c
+++ b/fs/dlm/lock.c
@@ -5119,7 +5119,7 @@ void dlm_recover_waiters_pre(struct dlm_ls *ls)
 	int wait_type, stub_unlock_result, stub_cancel_result;
 	int dir_nodeid;
 
-	ms_stub = kmalloc(sizeof(struct dlm_message), GFP_KERNEL);
+	ms_stub = kmalloc(sizeof(*ms_stub), GFP_KERNEL);
 	if (!ms_stub) {
 		log_error(ls, "dlm_recover_waiters_pre no mem");
 		return;
-- 
2.12.2

[PATCH 06/10] dlm: Delete an error message for a failed memory allocation in dlm_recover_waiters_pre()

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sat, 6 May 2017 08:55:43 +0200

Omit an extra message for a memory allocation failure in this function.

Link: http://events.linuxfoundation.org/sites/events/files/slides/LCJ16-Refactor_Strings-WSang_0.pdf
Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 fs/dlm/lock.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/fs/dlm/lock.c b/fs/dlm/lock.c
index ffadb817ad39..d4aaddec1b16 100644
--- a/fs/dlm/lock.c
+++ b/fs/dlm/lock.c
@@ -5120,10 +5120,8 @@ void dlm_recover_waiters_pre(struct dlm_ls *ls)
 	int dir_nodeid;
 
 	ms_stub = kmalloc(sizeof(*ms_stub), GFP_KERNEL);
-	if (!ms_stub) {
-		log_error(ls, "dlm_recover_waiters_pre no mem");
+	if (!ms_stub)
 		return;
-	}
 
 	mutex_lock(&ls->ls_waiters_mutex);
 
-- 
2.12.2

[PATCH 07/10] dlm: Use kmalloc_array() in make_member_array()

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sat, 6 May 2017 09:19:17 +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.

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

diff --git a/fs/dlm/member.c b/fs/dlm/member.c
index 9c47f1c14a8b..89257699d4e4 100644
--- a/fs/dlm/member.c
+++ b/fs/dlm/member.c
@@ -405,8 +405,7 @@ static void make_member_array(struct dlm_ls *ls)
 	}
 
 	ls->ls_total_weight = total;
-
-	array = kmalloc(sizeof(int) * total, GFP_NOFS);
+	array = kmalloc_array(total, sizeof(*array), GFP_NOFS);
 	if (!array)
 		return;
 
-- 
2.12.2

[PATCH 08/10] dlm: Use kcalloc() in two functions

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sat, 6 May 2017 09:34:53 +0200

* Multiplications for the size determination of memory allocations
  indicated that array data structures should be processed.
  Thus reuse the corresponding function "kcalloc".

  This issue was detected by using the Coccinelle software.

* Replace the specification of data structures by pointer dereferences
  to make the corresponding size determinations a bit safer according to
  the Linux coding style convention.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 fs/dlm/member.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/fs/dlm/member.c b/fs/dlm/member.c
index 89257699d4e4..92c601a11e38 100644
--- a/fs/dlm/member.c
+++ b/fs/dlm/member.c
@@ -217,8 +217,7 @@ int dlm_slots_assign(struct dlm_ls *ls, int *num_slots, int *slots_size,
 	}
 
 	array_size = max + need;
-
-	array = kzalloc(array_size * sizeof(struct dlm_slot), GFP_NOFS);
+	array = kcalloc(array_size, sizeof(*array), GFP_NOFS);
 	if (!array)
 		return -ENOMEM;
 
@@ -491,8 +490,7 @@ void dlm_lsop_recover_done(struct dlm_ls *ls)
 		return;
 
 	num = ls->ls_num_nodes;
-
-	slots = kzalloc(num * sizeof(struct dlm_slot), GFP_KERNEL);
+	slots = kcalloc(num, sizeof(*slots), GFP_KERNEL);
 	if (!slots)
 		return;
 
-- 
2.12.2

[PATCH 09/10] dlm: Improve a size determination in two functions

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sat, 6 May 2017 09:45:59 +0200

Replace the specification of two data structures by pointer dereferences
as the parameter for the operator "sizeof" to make the corresponding size
determination a bit safer according to the Linux coding style convention.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 fs/dlm/member.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/fs/dlm/member.c b/fs/dlm/member.c
index 92c601a11e38..3e565034ff2e 100644
--- a/fs/dlm/member.c
+++ b/fs/dlm/member.c
@@ -318,7 +318,7 @@ static int dlm_add_member(struct dlm_ls *ls, struct dlm_config_node *node)
 	struct dlm_member *memb;
 	int error;
 
-	memb = kzalloc(sizeof(struct dlm_member), GFP_NOFS);
+	memb = kzalloc(sizeof(*memb), GFP_NOFS);
 	if (!memb)
 		return -ENOMEM;
 
@@ -674,7 +674,7 @@ int dlm_ls_start(struct dlm_ls *ls)
 	struct dlm_config_node *nodes;
 	int error, count;
 
-	rv = kzalloc(sizeof(struct dlm_recover), GFP_NOFS);
+	rv = kzalloc(sizeof(*rv), GFP_NOFS);
 	if (!rv)
 		return -ENOMEM;
 
-- 
2.12.2

[PATCH 10/10] dlm: Delete an unnecessary variable initialisation in dlm_ls_start()

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sat, 6 May 2017 09:56:55 +0200

The local variable "rv" is reassigned by a statement at the beginning.
Thus omit the explicit initialisation.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 fs/dlm/member.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/dlm/member.c b/fs/dlm/member.c
index 3e565034ff2e..3fda3832cf6a 100644
--- a/fs/dlm/member.c
+++ b/fs/dlm/member.c
@@ -670,7 +670,7 @@ int dlm_ls_stop(struct dlm_ls *ls)
 
 int dlm_ls_start(struct dlm_ls *ls)
 {
-	struct dlm_recover *rv = NULL, *rv_old;
+	struct dlm_recover *rv, *rv_old;
 	struct dlm_config_node *nodes;
 	int error, count;
 
-- 
2.12.2

Re: [Cluster-devel] [PATCH 00/10] fs-DLM: Fine-tuning for several function implementations

[Bob Peterson]

----- Original Message -----
| From: Markus Elfring <elfring@users.sourceforge.net>
| Date: Sat, 6 May 2017 10:15:25 +0200
| 
| Some update suggestions were taken into account
| from static source code analysis.
| 
| Markus Elfring (10):
|   Replace six seq_puts() calls by seq_putc()
|   Add spaces for better code readability
|   Improve a size determination in table_seq_start()
|   Use kcalloc() in dlm_scan_waiters()
|   Improve a size determination in dlm_recover_waiters_pre()
|   Delete an error message for a failed memory allocation in
|   dlm_recover_waiters_pre()
|   Use kmalloc_array() in make_member_array()
|   Use kcalloc() in two functions
|   Improve a size determination in two functions
|   Delete an unnecessary variable initialisation in dlm_ls_start()
| 
|  fs/dlm/debug_fs.c | 25 ++++++++++++-------------
|  fs/dlm/lock.c     |  8 +++-----
|  fs/dlm/member.c   | 15 ++++++---------
|  3 files changed, 21 insertions(+), 27 deletions(-)
| 
| --
| 2.12.2
| 
| 
Hi Markus,

I'm not a DLM maintainer, but I've reviewed your patch set and ACK them.
They seem to fit the spirit of:

https://www.kernel.org/doc/html/v4.10/process/coding-style.html#allocating-memory

I've actually got a bunch of similar cleanups myself, but I don't think
there's any overlap.

Regards,

Bob Peterson
Red Hat File Systems