Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1494412 > unrolled thread

[PATCH 00/15] md-cluster: Fine-tuning for ten function implementations

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [PATCH 00/15] md-cluster: Fine-tuning for ten function  implementations SF Markus Elfring <elfring@users.sourceforge.net> - 2016-10-01 16:50 +0200
    [PATCH 01/15] md-cluster: Use kcalloc() in lock_all_bitmaps() SF Markus Elfring <elfring@users.sourceforge.net> - 2016-10-01 16:50 +0200
    [PATCH 04/15] md-cluster: Improve another size determination in  __sendmsg() SF Markus Elfring <elfring@users.sourceforge.net> - 2016-10-01 16:50 +0200
    [PATCH 02/15] md-cluster: Improve another size determination in  resync_info_update() SF Markus Elfring <elfring@users.sourceforge.net> - 2016-10-01 16:50 +0200
    [PATCH 03/15] md-cluster: Improve another size determination in  join() SF Markus Elfring <elfring@users.sourceforge.net> - 2016-10-01 16:50 +0200
    [PATCH 10/15] md-cluster: Delete an unnecessary variable  initialisation in lockres_init() SF Markus Elfring <elfring@users.sourceforge.net> - 2016-10-01 17:00 +0200
    [PATCH 12/15] md-cluster: Rename a jump label in area_resyncing() SF Markus Elfring <elfring@users.sourceforge.net> - 2016-10-01 17:00 +0200
    [PATCH 13/15] md-cluster: Less function calls in join() after error  detection SF Markus Elfring <elfring@users.sourceforge.net> - 2016-10-01 17:00 +0200
      [PATCH v2 13/15] md-cluster: Less function calls in join() after  error detection SF Markus Elfring <elfring@users.sourceforge.net> - 2016-10-06 17:20 +0200
    [PATCH 05/15] md-cluster: Improve another size determination in  recv_daemon() SF Markus Elfring <elfring@users.sourceforge.net> - 2016-10-01 17:00 +0200
    [PATCH 07/15] md-cluster: Improve another size determination in  process_suspend_info() SF Markus Elfring <elfring@users.sourceforge.net> - 2016-10-01 17:00 +0200
    [PATCH 14/15] md-cluster: Less function calls in lockres_init() after  error detection SF Markus Elfring <elfring@users.sourceforge.net> - 2016-10-01 17:00 +0200
    [PATCH 11/15] md-cluster: Delete four error messages for a failed  memory allocation SF Markus Elfring <elfring@users.sourceforge.net> - 2016-10-01 17:00 +0200
    [PATCH 08/15] md-cluster: Improve determination of sizes in  read_resync_info() SF Markus Elfring <elfring@users.sourceforge.net> - 2016-10-01 17:00 +0200
    [PATCH 09/15] md-cluster: Improve another size determination in  lockres_init() SF Markus Elfring <elfring@users.sourceforge.net> - 2016-10-01 17:00 +0200
    [PATCH 06/15] md-cluster: Rename a jump label in recv_daemon() SF Markus Elfring <elfring@users.sourceforge.net> - 2016-10-01 17:00 +0200
    [PATCH 15/15] md-cluster: Delete unnecessary braces in  unlock_all_bitmaps() SF Markus Elfring <elfring@users.sourceforge.net> - 2016-10-01 17:10 +0200
      Re: [PATCH 15/15] md-cluster: Delete unnecessary braces in  unlock_all_bitmaps() Dan Carpenter <dan.carpenter@oracle.com> - 2016-10-07 09:50 +0200
        Re: md-cluster: Delete unnecessary braces in unlock_all_bitmaps() SF Markus Elfring <elfring@users.sourceforge.net> - 2016-10-07 10:40 +0200
          Re: md-cluster: Delete unnecessary braces in unlock_all_bitmaps() walter harms <wharms@bfs.de> - 2016-10-07 15:30 +0200

#1494412 — [PATCH 00/15] md-cluster: Fine-tuning for ten function implementations

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sat, 1 Oct 2016 16:38:17 +0200

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

Markus Elfring (15):
  Use kcalloc() in lock_all_bitmaps()
  Improve another size determination in resync_info_update()
  Improve another size determination in join()
  Improve another size determination in __sendmsg()
  Improve another size determination in recv_daemon()
  Rename a jump label in recv_daemon()
  Improve another size determination in process_suspend_info()
  Improve determination of sizes in read_resync_info()
  Improve another size determination in lockres_init()
  Delete an unnecessary variable initialisation in lockres_init()
  Delete four error messages for a failed memory allocation
  Rename a jump label in area_resyncing()
  Less function calls in join() after error detection
  Less function calls in lockres_init() after error detection
  Delete unnecessary braces in unlock_all_bitmaps()

 drivers/md/md-cluster.c | 114 ++++++++++++++++++++++++------------------------
 1 file changed, 56 insertions(+), 58 deletions(-)

-- 
2.10.0

[toc] | [next] | [standalone]

#1494413 — [PATCH 01/15] md-cluster: Use kcalloc() in lock_all_bitmaps()

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sat, 1 Oct 2016 11:18:40 +0200

* A multiplication for the size determination of a memory allocation
  indicated that an array data structure should be processed.
  Thus reuse the corresponding function "kcalloc".

  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>
---
 drivers/md/md-cluster.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/md/md-cluster.c b/drivers/md/md-cluster.c
index 2b13117..0000e3a 100644
--- a/drivers/md/md-cluster.c
+++ b/drivers/md/md-cluster.c
@@ -1188,9 +1188,10 @@ static int lock_all_bitmaps(struct mddev *mddev)
 	char str[64];
 	struct md_cluster_info *cinfo = mddev->cluster_info;
 
-	cinfo->other_bitmap_lockres = kzalloc((mddev->bitmap_info.nodes - 1) *
-					     sizeof(struct dlm_lock_resource *),
-					     GFP_KERNEL);
+	cinfo->other_bitmap_lockres = kcalloc(mddev->bitmap_info.nodes - 1,
+					      sizeof(*cinfo
+						     ->other_bitmap_lockres),
+					      GFP_KERNEL);
 	if (!cinfo->other_bitmap_lockres) {
 		pr_err("md: can't alloc mem for other bitmap locks\n");
 		return 0;
-- 
2.10.0

[toc] | [prev] | [next] | [standalone]

#1494414 — [PATCH 04/15] md-cluster: Improve another size determination in __sendmsg()

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sat, 1 Oct 2016 12:21:48 +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>
---
 drivers/md/md-cluster.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/md/md-cluster.c b/drivers/md/md-cluster.c
index 7af27dd..c81eed4 100644
--- a/drivers/md/md-cluster.c
+++ b/drivers/md/md-cluster.c
@@ -706,7 +706,7 @@ static int __sendmsg(struct md_cluster_info *cinfo, struct cluster_msg *cmsg)
 	}
 
 	memcpy(cinfo->message_lockres->lksb.sb_lvbptr, (void *)cmsg,
-			sizeof(struct cluster_msg));
+	       sizeof(*cmsg));
 	/*down-convert EX to CW on Message*/
 	error = dlm_lock_sync(cinfo->message_lockres, DLM_LOCK_CW);
 	if (error) {
-- 
2.10.0

[toc] | [prev] | [next] | [standalone]

#1494415 — [PATCH 02/15] md-cluster: Improve another size determination in resync_info_update()

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

Replace the specification of a data structure by a variable name
as the parameter for the operator "sizeof" to make the corresponding size
determination a bit safer.

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

diff --git a/drivers/md/md-cluster.c b/drivers/md/md-cluster.c
index 0000e3a..c28f596 100644
--- a/drivers/md/md-cluster.c
+++ b/drivers/md/md-cluster.c
@@ -1057,7 +1057,7 @@ static int resync_info_update(struct mddev *mddev, sector_t lo, sector_t hi)
 
 	/* do not send zero again, if we have sent before */
 	if (hi == 0) {
-		memcpy(&ri, cinfo->bitmap_lockres->lksb.sb_lvbptr, sizeof(struct resync_info));
+		memcpy(&ri, cinfo->bitmap_lockres->lksb.sb_lvbptr, sizeof(ri));
 		if (le64_to_cpu(ri.hi) == 0)
 			return 0;
 	}
-- 
2.10.0

[toc] | [prev] | [next] | [standalone]

#1494416 — [PATCH 03/15] md-cluster: Improve another size determination in join()

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sat, 1 Oct 2016 11:42:47 +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>
---
 drivers/md/md-cluster.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/md/md-cluster.c b/drivers/md/md-cluster.c
index c28f596..7af27dd 100644
--- a/drivers/md/md-cluster.c
+++ b/drivers/md/md-cluster.c
@@ -823,7 +823,7 @@ static int join(struct mddev *mddev, int nodes)
 	int ret, ops_rv;
 	char str[64];
 
-	cinfo = kzalloc(sizeof(struct md_cluster_info), GFP_KERNEL);
+	cinfo = kzalloc(sizeof(*cinfo), GFP_KERNEL);
 	if (!cinfo)
 		return -ENOMEM;
 
-- 
2.10.0

[toc] | [prev] | [next] | [standalone]

#1494417 — [PATCH 10/15] md-cluster: Delete an unnecessary variable initialisation in lockres_init()

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sat, 1 Oct 2016 13:07:52 +0200

The local variable "res" will be set to an appropriate pointer a bit later.
Thus omit the explicit initialisation at the beginning.

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

diff --git a/drivers/md/md-cluster.c b/drivers/md/md-cluster.c
index bec8035..b91b552 100644
--- a/drivers/md/md-cluster.c
+++ b/drivers/md/md-cluster.c
@@ -186,7 +186,7 @@ static int dlm_lock_sync_interruptible(struct dlm_lock_resource *res, int mode,
 static struct dlm_lock_resource *lockres_init(struct mddev *mddev,
 		char *name, void (*bastfn)(void *arg, int mode), int with_lvb)
 {
-	struct dlm_lock_resource *res = NULL;
+	struct dlm_lock_resource *res;
 	int ret, namelen;
 	struct md_cluster_info *cinfo = mddev->cluster_info;
 
-- 
2.10.0

[toc] | [prev] | [next] | [standalone]

#1494418 — [PATCH 12/15] md-cluster: Rename a jump label in area_resyncing()

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sat, 1 Oct 2016 14:38:12 +0200

Adjust a jump label according to the Linux coding style convention

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

diff --git a/drivers/md/md-cluster.c b/drivers/md/md-cluster.c
index 7f82c6b..e1ebcc4 100644
--- a/drivers/md/md-cluster.c
+++ b/drivers/md/md-cluster.c
@@ -1085,13 +1085,13 @@ static int area_resyncing(struct mddev *mddev, int direction,
 
 	spin_lock_irq(&cinfo->suspend_lock);
 	if (list_empty(&cinfo->suspend_list))
-		goto out;
+		goto unlock;
 	list_for_each_entry(s, &cinfo->suspend_list, list)
 		if (hi > s->lo && lo < s->hi) {
 			ret = 1;
 			break;
 		}
-out:
+unlock:
 	spin_unlock_irq(&cinfo->suspend_lock);
 	return ret;
 }
-- 
2.10.0

[toc] | [prev] | [next] | [standalone]

#1494419 — [PATCH 13/15] md-cluster: Less function calls in join() after error detection

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sat, 1 Oct 2016 15:40:32 +0200

A few resource release functions were called in some cases
by the join() function during error handling
even if the passed data structure member contained a null pointer.

* Adjust jump targets according to the Linux coding style convention.

* Delete a repeated check which became unnecessary with this refactoring.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/md/md-cluster.c | 47 ++++++++++++++++++++++++++---------------------
 1 file changed, 26 insertions(+), 21 deletions(-)

diff --git a/drivers/md/md-cluster.c b/drivers/md/md-cluster.c
index e1ebcc4..36bb962 100644
--- a/drivers/md/md-cluster.c
+++ b/drivers/md/md-cluster.c
@@ -837,39 +837,39 @@ static int join(struct mddev *mddev, int nodes)
 				DLM_LSFL_FS, LVB_SIZE,
 				&md_ls_ops, mddev, &ops_rv, &cinfo->lockspace);
 	if (ret)
-		goto err;
+		goto free_cluster_info;
 	wait_for_completion(&cinfo->completion);
 	if (nodes < cinfo->slot_number) {
 		pr_err("md-cluster: Slot allotted(%d) is greater than available slots(%d).",
 			cinfo->slot_number, nodes);
 		ret = -ERANGE;
-		goto err;
+		goto release_lockspace;
 	}
 	/* Initiate the communication resources */
 	ret = -ENOMEM;
 	cinfo->recv_thread = md_register_thread(recv_daemon, mddev, "cluster_recv");
 	if (!cinfo->recv_thread)
-		goto err;
+		goto release_lockspace;
 	cinfo->message_lockres = lockres_init(mddev, "message", NULL, 1);
 	if (!cinfo->message_lockres)
-		goto err;
+		goto unregister_recv;
 	cinfo->token_lockres = lockres_init(mddev, "token", NULL, 0);
 	if (!cinfo->token_lockres)
-		goto err;
+		goto free_message;
 	cinfo->no_new_dev_lockres = lockres_init(mddev, "no-new-dev", NULL, 0);
 	if (!cinfo->no_new_dev_lockres)
-		goto err;
+		goto free_token;
 
 	ret = dlm_lock_sync(cinfo->token_lockres, DLM_LOCK_EX);
 	if (ret) {
 		ret = -EAGAIN;
 		pr_err("md-cluster: can't join cluster to avoid lock issue\n");
-		goto err;
+		goto free_no_new_dev;
 	}
 	cinfo->ack_lockres = lockres_init(mddev, "ack", ack_bast, 0);
 	if (!cinfo->ack_lockres) {
 		ret = -ENOMEM;
-		goto err;
+		goto free_no_new_dev;
 	}
 	/* get sync CR lock on ACK. */
 	if (dlm_lock_sync(cinfo->ack_lockres, DLM_LOCK_CR))
@@ -886,34 +886,39 @@ static int join(struct mddev *mddev, int nodes)
 	cinfo->bitmap_lockres = lockres_init(mddev, str, NULL, 1);
 	if (!cinfo->bitmap_lockres) {
 		ret = -ENOMEM;
-		goto err;
+		goto free_ack;
 	}
 	if (dlm_lock_sync(cinfo->bitmap_lockres, DLM_LOCK_PW)) {
 		pr_err("Failed to get bitmap lock\n");
 		ret = -EINVAL;
-		goto err;
+		goto free_bitmap;
 	}
 
 	cinfo->resync_lockres = lockres_init(mddev, "resync", NULL, 0);
 	if (!cinfo->resync_lockres) {
 		ret = -ENOMEM;
-		goto err;
+		goto free_bitmap;
 	}
 
 	return 0;
-err:
-	md_unregister_thread(&cinfo->recovery_thread);
-	md_unregister_thread(&cinfo->recv_thread);
-	lockres_free(cinfo->message_lockres);
-	lockres_free(cinfo->token_lockres);
+free_bitmap:
+	lockres_free(cinfo->bitmap_lockres);
+free_ack:
 	lockres_free(cinfo->ack_lockres);
+free_no_new_dev:
 	lockres_free(cinfo->no_new_dev_lockres);
-	lockres_free(cinfo->resync_lockres);
-	lockres_free(cinfo->bitmap_lockres);
-	if (cinfo->lockspace)
-		dlm_release_lockspace(cinfo->lockspace, 2);
-	mddev->cluster_info = NULL;
+free_token:
+	lockres_free(cinfo->token_lockres);
+free_message:
+	lockres_free(cinfo->message_lockres);
+unregister_recv:
+	md_unregister_thread(&cinfo->recv_thread);
+release_lockspace:
+	dlm_release_lockspace(cinfo->lockspace, 2);
+free_cluster_info:
 	kfree(cinfo);
+	md_unregister_thread(&cinfo->recovery_thread);
+	mddev->cluster_info = NULL;
 	return ret;
 }
 
-- 
2.10.0

[toc] | [prev] | [next] | [standalone]

#1496700 — [PATCH v2 13/15] md-cluster: Less function calls in join() after error detection

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Thu, 6 Oct 2016 16:48:51 +0200

A few resource release functions were called in some cases
by the join() function during error handling
even if the passed data structure member contained a null pointer.

* Adjust jump targets according to the Linux coding style convention.

* Delete a repeated check which became unnecessary with this refactoring.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---

v2: Julia Lawall wrote on 2016-10-01 at 19:07:
> Please check lines 919 and 920.
> 
> julia
> 
> ---------- Forwarded message ----------
> Date: Sun, 2 Oct 2016 01:03:19 +0800
> From: kbuild test robot <fengguang.wu@intel.com>
> To: kbuild@01.org
> Cc: Julia Lawall <julia.lawall@lip6.fr>
> Subject:
>     [linux-review:SF-Markus-Elfring/md-cluster-Fine-tuning-for-ten-function-impl
>     ementations/20161001-230311 13/15] drivers/md/md-cluster.c:920:23-28: ERROR:
>      reference preceded by free on line 919
> 
> CC: kbuild-all@01.org
> TO: Markus Elfring <elfring@users.sourceforge.net>
> CC: 0day robot <fengguang.wu@intel.com>
> 
> tree:   https://github.com/0day-ci/linux SF-Markus-Elfring/md-cluster-Fine-tuning-for-ten-function-implementations/20161001-230311
> head:   7b2084c76eb1c60f2ae5c2778470124b5b68e9fb
> commit: 6955234a0083a739f595e753e89ba7b5b3962f50 [13/15] md-cluster: Less function calls in join() after error detection
> :::::: branch date: 2 hours ago
> :::::: commit date: 2 hours ago
> 
>>> drivers/md/md-cluster.c:920:23-28: ERROR: reference preceded by free on line 919
> 
> git remote add linux-review https://github.com/0day-ci/linux
> git remote update linux-review
> git checkout 6955234a0083a739f595e753e89ba7b5b3962f50
> vim +920 drivers/md/md-cluster.c
> 
> 6955234a Markus Elfring    2016-10-01  913  	lockres_free(cinfo->message_lockres);
> 6955234a Markus Elfring    2016-10-01  914  unregister_recv:
> 6955234a Markus Elfring    2016-10-01  915  	md_unregister_thread(&cinfo->recv_thread);
> 6955234a Markus Elfring    2016-10-01  916  release_lockspace:
> c4ce867f Goldwyn Rodrigues 2014-03-29  917  	dlm_release_lockspace(cinfo->lockspace, 2);
> 6955234a Markus Elfring    2016-10-01  918  free_cluster_info:
> c4ce867f Goldwyn Rodrigues 2014-03-29 @919  	kfree(cinfo);
> 6955234a Markus Elfring    2016-10-01 @920  	md_unregister_thread(&cinfo->recovery_thread);
> 6955234a Markus Elfring    2016-10-01  921  	mddev->cluster_info = NULL;
> c4ce867f Goldwyn Rodrigues 2014-03-29  922  	return ret;
> edb39c9d Goldwyn Rodrigues 2014-03-29  923  }
> 
> ---
> 0-DAY kernel test infrastructure                Open Source Technology Center
> https://lists.01.org/pipermail/kbuild-all                   Intel Corporation


This automatic notification pointed out that I should have switched the order
of calls for the functions "kfree" and "md_unregister_thread".
I hope that my second approach could be integrated into another
source code repository.


 drivers/md/md-cluster.c | 47 ++++++++++++++++++++++++++---------------------
 1 file changed, 26 insertions(+), 21 deletions(-)

diff --git a/drivers/md/md-cluster.c b/drivers/md/md-cluster.c
index e1ebcc4..e60511a 100644
--- a/drivers/md/md-cluster.c
+++ b/drivers/md/md-cluster.c
@@ -837,39 +837,39 @@ static int join(struct mddev *mddev, int nodes)
 				DLM_LSFL_FS, LVB_SIZE,
 				&md_ls_ops, mddev, &ops_rv, &cinfo->lockspace);
 	if (ret)
-		goto err;
+		goto unregister_recovery_thread;
 	wait_for_completion(&cinfo->completion);
 	if (nodes < cinfo->slot_number) {
 		pr_err("md-cluster: Slot allotted(%d) is greater than available slots(%d).",
 			cinfo->slot_number, nodes);
 		ret = -ERANGE;
-		goto err;
+		goto release_lockspace;
 	}
 	/* Initiate the communication resources */
 	ret = -ENOMEM;
 	cinfo->recv_thread = md_register_thread(recv_daemon, mddev, "cluster_recv");
 	if (!cinfo->recv_thread)
-		goto err;
+		goto release_lockspace;
 	cinfo->message_lockres = lockres_init(mddev, "message", NULL, 1);
 	if (!cinfo->message_lockres)
-		goto err;
+		goto unregister_recv;
 	cinfo->token_lockres = lockres_init(mddev, "token", NULL, 0);
 	if (!cinfo->token_lockres)
-		goto err;
+		goto free_message;
 	cinfo->no_new_dev_lockres = lockres_init(mddev, "no-new-dev", NULL, 0);
 	if (!cinfo->no_new_dev_lockres)
-		goto err;
+		goto free_token;
 
 	ret = dlm_lock_sync(cinfo->token_lockres, DLM_LOCK_EX);
 	if (ret) {
 		ret = -EAGAIN;
 		pr_err("md-cluster: can't join cluster to avoid lock issue\n");
-		goto err;
+		goto free_no_new_dev;
 	}
 	cinfo->ack_lockres = lockres_init(mddev, "ack", ack_bast, 0);
 	if (!cinfo->ack_lockres) {
 		ret = -ENOMEM;
-		goto err;
+		goto free_no_new_dev;
 	}
 	/* get sync CR lock on ACK. */
 	if (dlm_lock_sync(cinfo->ack_lockres, DLM_LOCK_CR))
@@ -886,34 +886,39 @@ static int join(struct mddev *mddev, int nodes)
 	cinfo->bitmap_lockres = lockres_init(mddev, str, NULL, 1);
 	if (!cinfo->bitmap_lockres) {
 		ret = -ENOMEM;
-		goto err;
+		goto free_ack;
 	}
 	if (dlm_lock_sync(cinfo->bitmap_lockres, DLM_LOCK_PW)) {
 		pr_err("Failed to get bitmap lock\n");
 		ret = -EINVAL;
-		goto err;
+		goto free_bitmap;
 	}
 
 	cinfo->resync_lockres = lockres_init(mddev, "resync", NULL, 0);
 	if (!cinfo->resync_lockres) {
 		ret = -ENOMEM;
-		goto err;
+		goto free_bitmap;
 	}
 
 	return 0;
-err:
-	md_unregister_thread(&cinfo->recovery_thread);
-	md_unregister_thread(&cinfo->recv_thread);
-	lockres_free(cinfo->message_lockres);
-	lockres_free(cinfo->token_lockres);
+free_bitmap:
+	lockres_free(cinfo->bitmap_lockres);
+free_ack:
 	lockres_free(cinfo->ack_lockres);
+free_no_new_dev:
 	lockres_free(cinfo->no_new_dev_lockres);
-	lockres_free(cinfo->resync_lockres);
-	lockres_free(cinfo->bitmap_lockres);
-	if (cinfo->lockspace)
-		dlm_release_lockspace(cinfo->lockspace, 2);
-	mddev->cluster_info = NULL;
+free_token:
+	lockres_free(cinfo->token_lockres);
+free_message:
+	lockres_free(cinfo->message_lockres);
+unregister_recv:
+	md_unregister_thread(&cinfo->recv_thread);
+release_lockspace:
+	dlm_release_lockspace(cinfo->lockspace, 2);
+unregister_recovery_thread:
+	md_unregister_thread(&cinfo->recovery_thread);
 	kfree(cinfo);
+	mddev->cluster_info = NULL;
 	return ret;
 }
 
-- 
2.10.1

[toc] | [prev] | [next] | [standalone]

#1494420 — [PATCH 05/15] md-cluster: Improve another size determination in recv_daemon()

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sat, 1 Oct 2016 12:28:19 +0200

Replace the specification of a data structure by a variable name
as the parameter for the operator "sizeof" to make the corresponding size
determination a bit safer.

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

diff --git a/drivers/md/md-cluster.c b/drivers/md/md-cluster.c
index c81eed4..adf9555 100644
--- a/drivers/md/md-cluster.c
+++ b/drivers/md/md-cluster.c
@@ -617,7 +617,7 @@ static void recv_daemon(struct md_thread *thread)
 	}
 
 	/* read lvb and wake up thread to process this message_lockres */
-	memcpy(&msg, message_lockres->lksb.sb_lvbptr, sizeof(struct cluster_msg));
+	memcpy(&msg, message_lockres->lksb.sb_lvbptr, sizeof(msg));
 	ret = process_recvd_msg(thread->mddev, &msg);
 	if (ret)
 		goto out;
-- 
2.10.0

[toc] | [prev] | [next] | [standalone]

#1494421 — [PATCH 07/15] md-cluster: Improve another size determination in process_suspend_info()

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sat, 1 Oct 2016 12:42:50 +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>
---
 drivers/md/md-cluster.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/md/md-cluster.c b/drivers/md/md-cluster.c
index 94835b5..e965e78 100644
--- a/drivers/md/md-cluster.c
+++ b/drivers/md/md-cluster.c
@@ -482,8 +482,7 @@ static void process_suspend_info(struct mddev *mddev,
 					lo, hi);
 	cinfo->sync_low = lo;
 	cinfo->sync_hi = hi;
-
-	s = kzalloc(sizeof(struct suspend_info), GFP_KERNEL);
+	s = kzalloc(sizeof(*s), GFP_KERNEL);
 	if (!s)
 		return;
 	s->slot = slot;
-- 
2.10.0

[toc] | [prev] | [next] | [standalone]

#1494422 — [PATCH 14/15] md-cluster: Less function calls in lockres_init() after error detection

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sat, 1 Oct 2016 16:01:05 +0200

The kfree() function was called in up to three cases
by the lockres_init() function during error handling even if
the passed data structure member (or variable) contained a null pointer.

Adjust jump targets according to the Linux coding style convention.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/md/md-cluster.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/md/md-cluster.c b/drivers/md/md-cluster.c
index 36bb962..0cade1a 100644
--- a/drivers/md/md-cluster.c
+++ b/drivers/md/md-cluster.c
@@ -201,12 +201,12 @@ static struct dlm_lock_resource *lockres_init(struct mddev *mddev,
 	namelen = strlen(name);
 	res->name = kzalloc(namelen + 1, GFP_KERNEL);
 	if (!res->name)
-		goto out_err;
+		goto free_resource;
 	strlcpy(res->name, name, namelen + 1);
 	if (with_lvb) {
 		res->lksb.sb_lvbptr = kzalloc(LVB_SIZE, GFP_KERNEL);
 		if (!res->lksb.sb_lvbptr)
-			goto out_err;
+			goto free_name;
 		res->flags = DLM_LKF_VALBLK;
 	}
 
@@ -218,15 +218,17 @@ static struct dlm_lock_resource *lockres_init(struct mddev *mddev,
 	ret = dlm_lock_sync(res, DLM_LOCK_NL);
 	if (ret) {
 		pr_err("md-cluster: Unable to lock NL on new lock resource %s\n", name);
-		goto out_err;
+		goto free_lvb;
 	}
 	res->flags &= ~DLM_LKF_EXPEDITE;
 	res->flags |= DLM_LKF_CONVERT;
 
 	return res;
-out_err:
+free_lvb:
 	kfree(res->lksb.sb_lvbptr);
+free_name:
 	kfree(res->name);
+free_resource:
 	kfree(res);
 	return NULL;
 }
-- 
2.10.0

[toc] | [prev] | [next] | [standalone]

#1494423 — [PATCH 11/15] md-cluster: Delete four error messages for a failed memory allocation

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

Omit extra messages for a memory allocation failure in three functions.

Link: http://events.linuxfoundation.org/sites/events/files/slides/LCJ16-Refactor_Strings-WSang_0.pdf
Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/md/md-cluster.c | 16 ++++------------
 1 file changed, 4 insertions(+), 12 deletions(-)

diff --git a/drivers/md/md-cluster.c b/drivers/md/md-cluster.c
index b91b552..7f82c6b 100644
--- a/drivers/md/md-cluster.c
+++ b/drivers/md/md-cluster.c
@@ -200,17 +200,13 @@ static struct dlm_lock_resource *lockres_init(struct mddev *mddev,
 	res->mode = DLM_LOCK_IV;
 	namelen = strlen(name);
 	res->name = kzalloc(namelen + 1, GFP_KERNEL);
-	if (!res->name) {
-		pr_err("md-cluster: Unable to allocate resource name for resource %s\n", name);
+	if (!res->name)
 		goto out_err;
-	}
 	strlcpy(res->name, name, namelen + 1);
 	if (with_lvb) {
 		res->lksb.sb_lvbptr = kzalloc(LVB_SIZE, GFP_KERNEL);
-		if (!res->lksb.sb_lvbptr) {
-			pr_err("md-cluster: Unable to allocate LVB for resource %s\n", name);
+		if (!res->lksb.sb_lvbptr)
 			goto out_err;
-		}
 		res->flags = DLM_LKF_VALBLK;
 	}
 
@@ -852,10 +848,8 @@ static int join(struct mddev *mddev, int nodes)
 	/* Initiate the communication resources */
 	ret = -ENOMEM;
 	cinfo->recv_thread = md_register_thread(recv_daemon, mddev, "cluster_recv");
-	if (!cinfo->recv_thread) {
-		pr_err("md-cluster: cannot allocate memory for recv_thread!\n");
+	if (!cinfo->recv_thread)
 		goto err;
-	}
 	cinfo->message_lockres = lockres_init(mddev, "message", NULL, 1);
 	if (!cinfo->message_lockres)
 		goto err;
@@ -1191,10 +1185,8 @@ static int lock_all_bitmaps(struct mddev *mddev)
 					      sizeof(*cinfo
 						     ->other_bitmap_lockres),
 					      GFP_KERNEL);
-	if (!cinfo->other_bitmap_lockres) {
-		pr_err("md: can't alloc mem for other bitmap locks\n");
+	if (!cinfo->other_bitmap_lockres)
 		return 0;
-	}
 
 	my_slot = slot_number(mddev);
 	for (slot = 0; slot < mddev->bitmap_info.nodes; slot++) {
-- 
2.10.0

[toc] | [prev] | [next] | [standalone]

#1494424 — [PATCH 08/15] md-cluster: Improve determination of sizes in read_resync_info()

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sat, 1 Oct 2016 12:52:58 +0200

Replace the specification of data structures by either a pointer
dereference or a variable name 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>
---
 drivers/md/md-cluster.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/md/md-cluster.c b/drivers/md/md-cluster.c
index e965e78..0918108 100644
--- a/drivers/md/md-cluster.c
+++ b/drivers/md/md-cluster.c
@@ -275,10 +275,10 @@ static struct suspend_info *read_resync_info(struct mddev *mddev, struct dlm_loc
 	sector_t hi = 0;
 
 	dlm_lock_sync(lockres, DLM_LOCK_CR);
-	memcpy(&ri, lockres->lksb.sb_lvbptr, sizeof(struct resync_info));
+	memcpy(&ri, lockres->lksb.sb_lvbptr, sizeof(ri));
 	hi = le64_to_cpu(ri.hi);
 	if (hi > 0) {
-		s = kzalloc(sizeof(struct suspend_info), GFP_KERNEL);
+		s = kzalloc(sizeof(*s), GFP_KERNEL);
 		if (!s)
 			goto out;
 		s->hi = hi;
-- 
2.10.0

[toc] | [prev] | [next] | [standalone]

#1494425 — [PATCH 09/15] md-cluster: Improve another size determination in lockres_init()

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sat, 1 Oct 2016 13:02:18 +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>
---
 drivers/md/md-cluster.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/md/md-cluster.c b/drivers/md/md-cluster.c
index 0918108..bec8035 100644
--- a/drivers/md/md-cluster.c
+++ b/drivers/md/md-cluster.c
@@ -190,7 +190,7 @@ static struct dlm_lock_resource *lockres_init(struct mddev *mddev,
 	int ret, namelen;
 	struct md_cluster_info *cinfo = mddev->cluster_info;
 
-	res = kzalloc(sizeof(struct dlm_lock_resource), GFP_KERNEL);
+	res = kzalloc(sizeof(*res), GFP_KERNEL);
 	if (!res)
 		return NULL;
 	init_waitqueue_head(&res->sync_locking);
-- 
2.10.0

[toc] | [prev] | [next] | [standalone]

#1494426 — [PATCH 06/15] md-cluster: Rename a jump label in recv_daemon()

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sat, 1 Oct 2016 12:36:30 +0200

Adjust a jump label according to the Linux coding style convention.

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

diff --git a/drivers/md/md-cluster.c b/drivers/md/md-cluster.c
index adf9555..94835b5 100644
--- a/drivers/md/md-cluster.c
+++ b/drivers/md/md-cluster.c
@@ -620,7 +620,7 @@ static void recv_daemon(struct md_thread *thread)
 	memcpy(&msg, message_lockres->lksb.sb_lvbptr, sizeof(msg));
 	ret = process_recvd_msg(thread->mddev, &msg);
 	if (ret)
-		goto out;
+		goto unlock;
 
 	/*release CR on ack_lockres*/
 	ret = dlm_unlock_sync(ack_lockres);
@@ -634,7 +634,7 @@ static void recv_daemon(struct md_thread *thread)
 	ret = dlm_lock_sync(ack_lockres, DLM_LOCK_CR);
 	if (unlikely(ret != 0))
 		pr_info("lock CR on ack failed return %d\n", ret);
-out:
+unlock:
 	/*release CR on message_lockres*/
 	ret = dlm_unlock_sync(message_lockres);
 	if (unlikely(ret != 0))
-- 
2.10.0

[toc] | [prev] | [next] | [standalone]

#1494428 — [PATCH 15/15] md-cluster: Delete unnecessary braces in unlock_all_bitmaps()

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sat, 1 Oct 2016 16:15:55 +0200

Do not use curly brackets at one source code place
where a single statement should be sufficient.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/md/md-cluster.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/md/md-cluster.c b/drivers/md/md-cluster.c
index 0cade1a..a9bf13d 100644
--- a/drivers/md/md-cluster.c
+++ b/drivers/md/md-cluster.c
@@ -1223,11 +1223,10 @@ static void unlock_all_bitmaps(struct mddev *mddev)
 
 	/* release other node's bitmap lock if they are existed */
 	if (cinfo->other_bitmap_lockres) {
-		for (i = 0; i < mddev->bitmap_info.nodes - 1; i++) {
-			if (cinfo->other_bitmap_lockres[i]) {
+		for (i = 0; i < mddev->bitmap_info.nodes - 1; i++)
+			if (cinfo->other_bitmap_lockres[i])
 				lockres_free(cinfo->other_bitmap_lockres[i]);
-			}
-		}
+
 		kfree(cinfo->other_bitmap_lockres);
 	}
 }
-- 
2.10.0

[toc] | [prev] | [next] | [standalone]

#1497048 — Re: [PATCH 15/15] md-cluster: Delete unnecessary braces in unlock_all_bitmaps()

On Sat, Oct 01, 2016 at 05:00:07PM +0200, SF Markus Elfring wrote:
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Sat, 1 Oct 2016 16:15:55 +0200
> 
> Do not use curly brackets at one source code place
> where a single statement should be sufficient.
> 

The original style was correct and this is wrong.  I have explained this
before.

regards,
dan carpenter

[toc] | [prev] | [next] | [standalone]

#1497070 — Re: md-cluster: Delete unnecessary braces in unlock_all_bitmaps()

>> Do not use curly brackets at one source code place
>> where a single statement should be sufficient.
> 
> The original style was correct and this is wrong.  I have explained this before.

Did I change a bit too much in the proposed step according to the following
update suggestion?

elfring@Sonne:~/Projekte/Linux/next-patched> git checkout d6385db94196b253ae5eb3678fa95cdf1f839fcc && scripts/checkpatch.pl --types BRACES -f drivers/md/md-cluster.c
…
WARNING: braces {} are not necessary for single statement blocks
#1228: FILE: drivers/md/md-cluster.c:1228:
+			if (cinfo->other_bitmap_lockres[i]) {
+				lockres_free(cinfo->other_bitmap_lockres[i]);
+			}
…


How do you think about to adjust this source code place a bit?

Regards,
Markus

[toc] | [prev] | [next] | [standalone]

#1497156 — Re: md-cluster: Delete unnecessary braces in unlock_all_bitmaps()

Am 07.10.2016 10:37, schrieb SF Markus Elfring:
>>> Do not use curly brackets at one source code place
>>> where a single statement should be sufficient.
>>
>> The original style was correct and this is wrong.  I have explained this before.
> 
> Did I change a bit too much in the proposed step according to the following
> update suggestion?
> 
> elfring@Sonne:~/Projekte/Linux/next-patched> git checkout d6385db94196b253ae5eb3678fa95cdf1f839fcc && scripts/checkpatch.pl --types BRACES -f drivers/md/md-cluster.c
> …
> WARNING: braces {} are not necessary for single statement blocks
> #1228: FILE: drivers/md/md-cluster.c:1228:
> +			if (cinfo->other_bitmap_lockres[i]) {
> +				lockres_free(cinfo->other_bitmap_lockres[i]);
> +			}
> …
> 
> 
> How do you think about to adjust this source code place a bit?
> 

perhaps we can agree to delete the if() block ?

static void lockres_free(struct dlm_lock_resource *res)
{
        int ret;

       if (!res)
                return;

....


@marcus: is can not send mail to you sf.net adresse because sf.net
         mark my domain als spam (note: nobody else does)

re,
 wh

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web