2015-04-24 02:09:25

by Chao Yu

[permalink] [raw]
Subject: [PATCH] elevator: fix memory leak in ->elevator_init_fn

In ->elevator_init_fn, if we fail to call kzalloc_node, we should release
elevator queue space which is allocated previously, otherwise it will cause
memory leak.

Signed-off-by: Chao Yu <[email protected]>
---
block/cfq-iosched.c | 1 +
block/deadline-iosched.c | 1 +
block/noop-iosched.c | 1 +
3 files changed, 3 insertions(+)

diff --git a/block/cfq-iosched.c b/block/cfq-iosched.c
index 5da8e6e..2793fb7 100644
--- a/block/cfq-iosched.c
+++ b/block/cfq-iosched.c
@@ -4391,6 +4391,7 @@ static int cfq_init_queue(struct request_queue *q, struct elevator_type *e)
cfqd = kzalloc_node(sizeof(*cfqd), GFP_KERNEL, q->node);
if (!cfqd) {
kobject_put(&eq->kobj);
+ kfree(eq);
return -ENOMEM;
}
eq->elevator_data = cfqd;
diff --git a/block/deadline-iosched.c b/block/deadline-iosched.c
index a753df2..bc6d7d9 100644
--- a/block/deadline-iosched.c
+++ b/block/deadline-iosched.c
@@ -349,6 +349,7 @@ static int deadline_init_queue(struct request_queue *q, struct elevator_type *e)
dd = kzalloc_node(sizeof(*dd), GFP_KERNEL, q->node);
if (!dd) {
kobject_put(&eq->kobj);
+ kfree(eq);
return -ENOMEM;
}
eq->elevator_data = dd;
diff --git a/block/noop-iosched.c b/block/noop-iosched.c
index 3de89d4..1399c78 100644
--- a/block/noop-iosched.c
+++ b/block/noop-iosched.c
@@ -71,6 +71,7 @@ static int noop_init_queue(struct request_queue *q, struct elevator_type *e)
nd = kmalloc_node(sizeof(*nd), GFP_KERNEL, q->node);
if (!nd) {
kobject_put(&eq->kobj);
+ kfree(eq);
return -ENOMEM;
}
eq->elevator_data = nd;
--
2.3.3


2015-04-24 13:03:13

by Jeff Moyer

[permalink] [raw]
Subject: Re: [PATCH] elevator: fix memory leak in ->elevator_init_fn

Chao Yu <[email protected]> writes:

> In ->elevator_init_fn, if we fail to call kzalloc_node, we should release
> elevator queue space which is allocated previously, otherwise it will cause
> memory leak.
>
> Signed-off-by: Chao Yu <[email protected]>

Acked-by: Jeff Moyer <[email protected]>

2015-04-27 05:31:10

by Chao Yu

[permalink] [raw]
Subject: RE: [PATCH] elevator: fix memory leak in ->elevator_init_fn

Hi all,

I found that when we release eq->kobj by invoking kobject_put() we had gave
misson to workqueue, so that workqueue will release the elevator queue
space in elevator_alloc() which is registered in ->release of elv_ktype object.
We should not release it again.

So, this fix is wrong, please ignore this patch, sorry for the noisy!

Regards,

> -----Original Message-----
> From: Chao Yu [mailto:[email protected]]
> Sent: Friday, April 24, 2015 10:09 AM
> To: '[email protected]'
> Cc: '[email protected]'
> Subject: [PATCH] elevator: fix memory leak in ->elevator_init_fn
>
> In ->elevator_init_fn, if we fail to call kzalloc_node, we should release
> elevator queue space which is allocated previously, otherwise it will cause
> memory leak.
>
> Signed-off-by: Chao Yu <[email protected]>
> ---
> block/cfq-iosched.c | 1 +
> block/deadline-iosched.c | 1 +
> block/noop-iosched.c | 1 +
> 3 files changed, 3 insertions(+)
>
> diff --git a/block/cfq-iosched.c b/block/cfq-iosched.c
> index 5da8e6e..2793fb7 100644
> --- a/block/cfq-iosched.c
> +++ b/block/cfq-iosched.c
> @@ -4391,6 +4391,7 @@ static int cfq_init_queue(struct request_queue *q, struct elevator_type
> *e)
> cfqd = kzalloc_node(sizeof(*cfqd), GFP_KERNEL, q->node);
> if (!cfqd) {
> kobject_put(&eq->kobj);
> + kfree(eq);
> return -ENOMEM;
> }
> eq->elevator_data = cfqd;
> diff --git a/block/deadline-iosched.c b/block/deadline-iosched.c
> index a753df2..bc6d7d9 100644
> --- a/block/deadline-iosched.c
> +++ b/block/deadline-iosched.c
> @@ -349,6 +349,7 @@ static int deadline_init_queue(struct request_queue *q, struct elevator_type
> *e)
> dd = kzalloc_node(sizeof(*dd), GFP_KERNEL, q->node);
> if (!dd) {
> kobject_put(&eq->kobj);
> + kfree(eq);
> return -ENOMEM;
> }
> eq->elevator_data = dd;
> diff --git a/block/noop-iosched.c b/block/noop-iosched.c
> index 3de89d4..1399c78 100644
> --- a/block/noop-iosched.c
> +++ b/block/noop-iosched.c
> @@ -71,6 +71,7 @@ static int noop_init_queue(struct request_queue *q, struct elevator_type *e)
> nd = kmalloc_node(sizeof(*nd), GFP_KERNEL, q->node);
> if (!nd) {
> kobject_put(&eq->kobj);
> + kfree(eq);
> return -ENOMEM;
> }
> eq->elevator_data = nd;
> --
> 2.3.3