Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1496700
| From | SF Markus Elfring <elfring@users.sourceforge.net> |
|---|---|
| Newsgroups | linux.kernel |
| Subject | [PATCH v2 13/15] md-cluster: Less function calls in join() after error detection |
| Date | 2016-10-06 17:20 +0200 |
| Message-ID | <spiCJ-1Up-7@gated-at.bofh.it> (permalink) |
| References | <qEuGl-43C-5@gated-at.bofh.it> <sntM1-1qz-135@gated-at.bofh.it> <sntVD-1vj-7@gated-at.bofh.it> |
| Organization | linux.* mail to news gateway |
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
Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread
[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
csiph-web