On 12/10/21 9:18 AM, Xiao Ni wrote:
On Fri, Dec 10, 2021 at 2:02 AM Song Liu <song@xxxxxxxxxx> wrote:
On Wed, Dec 8, 2021 at 9:55 PM Xiao Ni <xni@xxxxxxxxxx> wrote:
It doesn't free memory when register integrity failed. And move
free conf codes into a single function.
Signed-off-by: Xiao Ni <xni@xxxxxxxxxx>
---
drivers/md/raid0.c | 18 +++++++++++++++---
1 file changed, 15 insertions(+), 3 deletions(-)
diff --git a/drivers/md/raid0.c b/drivers/md/raid0.c
index 62c8b6adac70..3fa47df1c60e 100644
--- a/drivers/md/raid0.c
+++ b/drivers/md/raid0.c
@@ -356,6 +356,7 @@ static sector_t raid0_size(struct mddev *mddev, sector_t sectors, int raid_disks
return array_sectors;
}
+static void free_conf(struct r0conf *conf);
static void raid0_free(struct mddev *mddev, void *priv);
static int raid0_run(struct mddev *mddev)
@@ -413,19 +414,30 @@ static int raid0_run(struct mddev *mddev)
dump_zones(mddev);
ret = md_integrity_register(mddev);
+ if (ret)
+ goto free;
return ret;
+
+free:
+ free_conf(conf);
Can we just use raid0_free() here? Also, after freeing conf,
At first I used raid0_free too. But it looks strange after adding
acct_bioset_exit
in raid0_free. Because if creating stripe zones failed, it doesn't
need to free conf,
although it doesn't have problems that passes NULL to kfree.
Maybe something like this, just FYI.
diff --git a/drivers/md/raid0.c b/drivers/md/raid0.c
index 62c8b6adac70..c24bec49b36f 100644
--- a/drivers/md/raid0.c
+++ b/drivers/md/raid0.c
@@ -377,6 +377,12 @@ static int raid0_run(struct mddev *mddev)
return ret;
mddev->private = conf;
}
+
+ if (acct_bioset_init(mddev)) {
+ pr_err("md/raid0:%s: alloc acct bioset failed.\n",
mdname(mddev));
+ return -ENOMEM;
+ }
+
conf = mddev->private;
if (mddev->queue) {
struct md_rdev *rdev;
@@ -413,6 +419,11 @@ static int raid0_run(struct mddev *mddev)
dump_zones(mddev);
ret = md_integrity_register(mddev);
+ if (ret) {
+ raid0_free(mddev, mddev->private);
+ mddev->private = NULL;
+ acct_bioset_exit(mddev);
+ }
return ret;
}
Thanks,
Guoqing
