* [PATCH] md/bitmap: call bitmap_destroy in case bitmap_create failed
@ 2016-03-25 11:00 Guoqing Jiang
2016-03-25 22:02 ` Shaohua Li
2016-04-01 9:08 ` [V2 PATCH] md/bitmap: clear bitmap if " Guoqing Jiang
0 siblings, 2 replies; 6+ messages in thread
From: Guoqing Jiang @ 2016-03-25 11:00 UTC (permalink / raw)
To: shli; +Cc: linux-raid, neilb, Guoqing Jiang
If bitmap_create returns an error, bitmap_destroy must be
called to do clean up.
Signed-off-by: Guoqing Jiang <gqjiang@suse.com>
---
drivers/md/bitmap.c | 14 ++++++++------
1 file changed, 8 insertions(+), 6 deletions(-)
diff --git a/drivers/md/bitmap.c b/drivers/md/bitmap.c
index 7df6b4f..ba908e7 100644
--- a/drivers/md/bitmap.c
+++ b/drivers/md/bitmap.c
@@ -1865,8 +1865,10 @@ int bitmap_copy_from_slot(struct mddev *mddev, int slot,
struct bitmap_counts *counts;
struct bitmap *bitmap = bitmap_create(mddev, slot);
- if (IS_ERR(bitmap))
+ if (IS_ERR(bitmap)) {
+ bitmap_destroy(mddev);
return PTR_ERR(bitmap);
+ }
rv = bitmap_init_from_disk(bitmap, 0);
if (rv)
@@ -1898,7 +1900,7 @@ int bitmap_copy_from_slot(struct mddev *mddev, int slot,
*low = lo;
*high = hi;
err:
- bitmap_free(bitmap);
+ bitmap_destroy(mddev);
return rv;
}
EXPORT_SYMBOL_GPL(bitmap_copy_from_slot);
@@ -2170,14 +2172,14 @@ location_store(struct mddev *mddev, const char *buf, size_t len)
else {
mddev->bitmap = bitmap;
rv = bitmap_load(mddev);
- if (rv) {
- bitmap_destroy(mddev);
+ if (rv)
mddev->bitmap_info.offset = 0;
- }
}
mddev->pers->quiesce(mddev, 0);
- if (rv)
+ if (rv) {
+ bitmap_destroy(mddev);
return rv;
+ }
}
}
}
--
2.1.4
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH] md/bitmap: call bitmap_destroy in case bitmap_create failed
2016-03-25 11:00 [PATCH] md/bitmap: call bitmap_destroy in case bitmap_create failed Guoqing Jiang
@ 2016-03-25 22:02 ` Shaohua Li
2016-03-28 3:27 ` Guoqing Jiang
2016-04-01 9:08 ` [V2 PATCH] md/bitmap: clear bitmap if " Guoqing Jiang
1 sibling, 1 reply; 6+ messages in thread
From: Shaohua Li @ 2016-03-25 22:02 UTC (permalink / raw)
To: Guoqing Jiang; +Cc: linux-raid, neilb
On Fri, Mar 25, 2016 at 07:00:14PM +0800, Guoqing Jiang wrote:
> If bitmap_create returns an error, bitmap_destroy must be
> called to do clean up.
>
> Signed-off-by: Guoqing Jiang <gqjiang@suse.com>
> ---
> drivers/md/bitmap.c | 14 ++++++++------
> 1 file changed, 8 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/md/bitmap.c b/drivers/md/bitmap.c
> index 7df6b4f..ba908e7 100644
> --- a/drivers/md/bitmap.c
> +++ b/drivers/md/bitmap.c
> @@ -1865,8 +1865,10 @@ int bitmap_copy_from_slot(struct mddev *mddev, int slot,
> struct bitmap_counts *counts;
> struct bitmap *bitmap = bitmap_create(mddev, slot);
>
> - if (IS_ERR(bitmap))
> + if (IS_ERR(bitmap)) {
> + bitmap_destroy(mddev);
> return PTR_ERR(bitmap);
> + }
bitmap_create doesn't set mddev->bitmap, so bitmap_destroy will do nothing.
is this because of reference leak of sysfs_can_clear? can we move
sysfs_put(bitmap->sysfs_can_clear); to bitmap_free?
Thanks,
Shaohua
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] md/bitmap: call bitmap_destroy in case bitmap_create failed
2016-03-25 22:02 ` Shaohua Li
@ 2016-03-28 3:27 ` Guoqing Jiang
2016-03-28 18:15 ` Shaohua Li
0 siblings, 1 reply; 6+ messages in thread
From: Guoqing Jiang @ 2016-03-28 3:27 UTC (permalink / raw)
To: Shaohua Li; +Cc: linux-raid, neilb
On 03/26/2016 06:02 AM, Shaohua Li wrote:
> On Fri, Mar 25, 2016 at 07:00:14PM +0800, Guoqing Jiang wrote:
>> If bitmap_create returns an error, bitmap_destroy must be
>> called to do clean up.
>>
>> Signed-off-by: Guoqing Jiang <gqjiang@suse.com>
>> ---
>> drivers/md/bitmap.c | 14 ++++++++------
>> 1 file changed, 8 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/md/bitmap.c b/drivers/md/bitmap.c
>> index 7df6b4f..ba908e7 100644
>> --- a/drivers/md/bitmap.c
>> +++ b/drivers/md/bitmap.c
>> @@ -1865,8 +1865,10 @@ int bitmap_copy_from_slot(struct mddev *mddev, int slot,
>> struct bitmap_counts *counts;
>> struct bitmap *bitmap = bitmap_create(mddev, slot);
>>
>> - if (IS_ERR(bitmap))
>> + if (IS_ERR(bitmap)) {
>> + bitmap_destroy(mddev);
>> return PTR_ERR(bitmap);
>> + }
> bitmap_create doesn't set mddev->bitmap, so bitmap_destroy will do nothing.
> is this because of reference leak of sysfs_can_clear?
Yes, use bitmap_free should be ok here since mddev->bitmap is not set.
> can we move sysfs_put(bitmap->sysfs_can_clear); to bitmap_free?
Do you mean move sysfs_put from bitmap_destroy to bitmap_free?
I guess it could be ok. Also we need make minor change to the comment.
/*
* initialize the bitmap structure
* if this returns an error, bitmap_destroy must be called to do clean up
+ * once mddev->bitmap is set
*/
struct bitmap *bitmap_create(struct mddev *mddev, int slot)
Thanks,
Guoqing
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] md/bitmap: call bitmap_destroy in case bitmap_create failed
2016-03-28 3:27 ` Guoqing Jiang
@ 2016-03-28 18:15 ` Shaohua Li
0 siblings, 0 replies; 6+ messages in thread
From: Shaohua Li @ 2016-03-28 18:15 UTC (permalink / raw)
To: Guoqing Jiang; +Cc: linux-raid, neilb
On Mon, Mar 28, 2016 at 11:27:26AM +0800, Guoqing Jiang wrote:
>
>
> On 03/26/2016 06:02 AM, Shaohua Li wrote:
> >On Fri, Mar 25, 2016 at 07:00:14PM +0800, Guoqing Jiang wrote:
> >>If bitmap_create returns an error, bitmap_destroy must be
> >>called to do clean up.
> >>
> >>Signed-off-by: Guoqing Jiang <gqjiang@suse.com>
> >>---
> >> drivers/md/bitmap.c | 14 ++++++++------
> >> 1 file changed, 8 insertions(+), 6 deletions(-)
> >>
> >>diff --git a/drivers/md/bitmap.c b/drivers/md/bitmap.c
> >>index 7df6b4f..ba908e7 100644
> >>--- a/drivers/md/bitmap.c
> >>+++ b/drivers/md/bitmap.c
> >>@@ -1865,8 +1865,10 @@ int bitmap_copy_from_slot(struct mddev *mddev, int slot,
> >> struct bitmap_counts *counts;
> >> struct bitmap *bitmap = bitmap_create(mddev, slot);
> >>- if (IS_ERR(bitmap))
> >>+ if (IS_ERR(bitmap)) {
> >>+ bitmap_destroy(mddev);
> >> return PTR_ERR(bitmap);
> >>+ }
> >bitmap_create doesn't set mddev->bitmap, so bitmap_destroy will do nothing.
> >is this because of reference leak of sysfs_can_clear?
>
> Yes, use bitmap_free should be ok here since mddev->bitmap is not set.
>
> >can we move sysfs_put(bitmap->sysfs_can_clear); to bitmap_free?
>
> Do you mean move sysfs_put from bitmap_destroy to bitmap_free?
> I guess it could be ok. Also we need make minor change to the comment.
right. Just free the reference of sysfs_can_clear and set it to NULL. I think
that will fix your issue.
> /*
> * initialize the bitmap structure
> * if this returns an error, bitmap_destroy must be called to do clean up
> + * once mddev->bitmap is set
> */
> struct bitmap *bitmap_create(struct mddev *mddev, int slot)
please also fix the comment.
Thanks,
Shaohua
^ permalink raw reply [flat|nested] 6+ messages in thread
* [V2 PATCH] md/bitmap: clear bitmap if bitmap_create failed
2016-03-25 11:00 [PATCH] md/bitmap: call bitmap_destroy in case bitmap_create failed Guoqing Jiang
2016-03-25 22:02 ` Shaohua Li
@ 2016-04-01 9:08 ` Guoqing Jiang
2016-04-01 20:04 ` Shaohua Li
1 sibling, 1 reply; 6+ messages in thread
From: Guoqing Jiang @ 2016-04-01 9:08 UTC (permalink / raw)
To: shli; +Cc: neilb, linux-raid, Guoqing Jiang
If bitmap_create returns an error, we need to call
either bitmap_destroy or bitmap_free to do clean up,
and the selection is based on mddev->bitmap is set
or not.
And the sysfs_put(bitmap->sysfs_can_clear) is moved
from bitmap_destroy to bitmap_free, and the comment
of bitmap_create is changed as well.
Signed-off-by: Guoqing Jiang <gqjiang@suse.com>
---
drivers/md/bitmap.c | 19 +++++++++++--------
1 file changed, 11 insertions(+), 8 deletions(-)
diff --git a/drivers/md/bitmap.c b/drivers/md/bitmap.c
index a624792..062ac99 100644
--- a/drivers/md/bitmap.c
+++ b/drivers/md/bitmap.c
@@ -1697,6 +1697,9 @@ static void bitmap_free(struct bitmap *bitmap)
if (!bitmap) /* there was no bitmap */
return;
+ if (bitmap->sysfs_can_clear)
+ sysfs_put(bitmap->sysfs_can_clear);
+
if (mddev_is_clustered(bitmap->mddev) && bitmap->mddev->cluster_info &&
bitmap->cluster_slot == md_cluster_ops->slot_number(bitmap->mddev))
md_cluster_stop(bitmap->mddev);
@@ -1736,15 +1739,13 @@ void bitmap_destroy(struct mddev *mddev)
if (mddev->thread)
mddev->thread->timeout = MAX_SCHEDULE_TIMEOUT;
- if (bitmap->sysfs_can_clear)
- sysfs_put(bitmap->sysfs_can_clear);
-
bitmap_free(bitmap);
}
/*
* initialize the bitmap structure
* if this returns an error, bitmap_destroy must be called to do clean up
+ * once mddev->bitmap is set
*/
struct bitmap *bitmap_create(struct mddev *mddev, int slot)
{
@@ -1889,8 +1890,10 @@ int bitmap_copy_from_slot(struct mddev *mddev, int slot,
struct bitmap_counts *counts;
struct bitmap *bitmap = bitmap_create(mddev, slot);
- if (IS_ERR(bitmap))
+ if (IS_ERR(bitmap)) {
+ bitmap_free(bitmap);
return PTR_ERR(bitmap);
+ }
rv = bitmap_init_from_disk(bitmap, 0);
if (rv)
@@ -2223,14 +2226,14 @@ location_store(struct mddev *mddev, const char *buf, size_t len)
else {
mddev->bitmap = bitmap;
rv = bitmap_load(mddev);
- if (rv) {
- bitmap_destroy(mddev);
+ if (rv)
mddev->bitmap_info.offset = 0;
- }
}
mddev->pers->quiesce(mddev, 0);
- if (rv)
+ if (rv) {
+ bitmap_destroy(mddev);
return rv;
+ }
}
}
}
--
2.6.2
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [V2 PATCH] md/bitmap: clear bitmap if bitmap_create failed
2016-04-01 9:08 ` [V2 PATCH] md/bitmap: clear bitmap if " Guoqing Jiang
@ 2016-04-01 20:04 ` Shaohua Li
0 siblings, 0 replies; 6+ messages in thread
From: Shaohua Li @ 2016-04-01 20:04 UTC (permalink / raw)
To: Guoqing Jiang; +Cc: neilb, linux-raid
On Fri, Apr 01, 2016 at 05:08:49PM +0800, Guoqing Jiang wrote:
> If bitmap_create returns an error, we need to call
> either bitmap_destroy or bitmap_free to do clean up,
> and the selection is based on mddev->bitmap is set
> or not.
>
> And the sysfs_put(bitmap->sysfs_can_clear) is moved
> from bitmap_destroy to bitmap_free, and the comment
> of bitmap_create is changed as well.
Applied, thanks!
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2016-04-01 20:04 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-03-25 11:00 [PATCH] md/bitmap: call bitmap_destroy in case bitmap_create failed Guoqing Jiang
2016-03-25 22:02 ` Shaohua Li
2016-03-28 3:27 ` Guoqing Jiang
2016-03-28 18:15 ` Shaohua Li
2016-04-01 9:08 ` [V2 PATCH] md/bitmap: clear bitmap if " Guoqing Jiang
2016-04-01 20:04 ` Shaohua Li
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).