Re: [PATCH] null_blk: fix memory leak on cleanup

[Matias Bjorling]

Den 02-09-2015 kl. 20:39 skrev Ross Zwisler:

On Mon, Aug 31, 2015 at 02:17:18PM +0200, Matias Bjørling wrote:

From: Matias Bjørling 

Driver was not freeing the memory allocated for internal nullb queues.
This patch frees the memory during driver unload.


You may want to consider devm_* style allocations instead.  These are freed
automatically on driver unload, simplifying your error paths and your unload
logic as well as preventing leaks.

See this patch for an example:

https://lkml.org/lkml/2015/8/10/992



Thanks for looking through the patches. I'll look into it.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] null_blk: fix memory leak on cleanup

[Ross Zwisler]

On Mon, Aug 31, 2015 at 02:17:18PM +0200, Matias Bjørling wrote:
> From: Matias Bjørling 
> 
> Driver was not freeing the memory allocated for internal nullb queues.
> This patch frees the memory during driver unload.

You may want to consider devm_* style allocations instead.  These are freed
automatically on driver unload, simplifying your error paths and your unload
logic as well as preventing leaks.

See this patch for an example:

https://lkml.org/lkml/2015/8/10/992
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] null_blk: fix memory leak on cleanup

[Ross Zwisler]

On Mon, Aug 31, 2015 at 02:17:18PM +0200, Matias Bjørling wrote:
> From: Matias Bjørling 
> 
> Driver was not freeing the memory allocated for internal nullb queues.
> This patch frees the memory during driver unload.

You may want to consider devm_* style allocations instead.  These are freed
automatically on driver unload, simplifying your error paths and your unload
logic as well as preventing leaks.

See this patch for an example:

https://lkml.org/lkml/2015/8/10/992
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] null_blk: fix memory leak on cleanup

[Matias Bjorling]

Den 02-09-2015 kl. 20:39 skrev Ross Zwisler:

On Mon, Aug 31, 2015 at 02:17:18PM +0200, Matias Bjørling wrote:

From: Matias Bjørling 

Driver was not freeing the memory allocated for internal nullb queues.
This patch frees the memory during driver unload.


You may want to consider devm_* style allocations instead.  These are freed
automatically on driver unload, simplifying your error paths and your unload
logic as well as preventing leaks.

See this patch for an example:

https://lkml.org/lkml/2015/8/10/992



Thanks for looking through the patches. I'll look into it.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH] null_blk: fix memory leak on cleanup

[Matias Bjørling]

From: Matias Bjørling 

Driver was not freeing the memory allocated for internal nullb queues.
This patch frees the memory during driver unload.

Signed-off-by: Matias Bjørling 
---
 drivers/block/null_blk.c | 33 +
 1 file changed, 17 insertions(+), 16 deletions(-)

diff --git a/drivers/block/null_blk.c b/drivers/block/null_blk.c
index 69de41a..90f6781 100644
--- a/drivers/block/null_blk.c
+++ b/drivers/block/null_blk.c
@@ -406,6 +406,22 @@ static struct blk_mq_ops null_mq_ops = {
.complete   = null_softirq_done_fn,
 };
 
+static void cleanup_queue(struct nullb_queue *nq)
+{
+   kfree(nq->tag_map);
+   kfree(nq->cmds);
+}
+
+static void cleanup_queues(struct nullb *nullb)
+{
+   int i;
+
+   for (i = 0; i < nullb->nr_queues; i++)
+   cleanup_queue(>queues[i]);
+
+   kfree(nullb->queues);
+}
+
 static void null_del_dev(struct nullb *nullb)
 {
list_del_init(>list);
@@ -415,6 +431,7 @@ static void null_del_dev(struct nullb *nullb)
if (queue_mode == NULL_Q_MQ)
blk_mq_free_tag_set(>tag_set);
put_disk(nullb->disk);
+   cleanup_queues(nullb);
kfree(nullb);
 }
 
@@ -459,22 +476,6 @@ static int setup_commands(struct nullb_queue *nq)
return 0;
 }
 
-static void cleanup_queue(struct nullb_queue *nq)
-{
-   kfree(nq->tag_map);
-   kfree(nq->cmds);
-}
-
-static void cleanup_queues(struct nullb *nullb)
-{
-   int i;
-
-   for (i = 0; i < nullb->nr_queues; i++)
-   cleanup_queue(>queues[i]);
-
-   kfree(nullb->queues);
-}
-
 static int setup_queues(struct nullb *nullb)
 {
nullb->queues = kzalloc(submit_queues * sizeof(struct nullb_queue),
-- 
2.1.4

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH] null_blk: fix memory leak on cleanup

[Matias Bjørling]

From: Matias Bjørling 

Driver was not freeing the memory allocated for internal nullb queues.
This patch frees the memory during driver unload.

Signed-off-by: Matias Bjørling 
---
 drivers/block/null_blk.c | 33 +
 1 file changed, 17 insertions(+), 16 deletions(-)

diff --git a/drivers/block/null_blk.c b/drivers/block/null_blk.c
index 69de41a..90f6781 100644
--- a/drivers/block/null_blk.c
+++ b/drivers/block/null_blk.c
@@ -406,6 +406,22 @@ static struct blk_mq_ops null_mq_ops = {
.complete   = null_softirq_done_fn,
 };
 
+static void cleanup_queue(struct nullb_queue *nq)
+{
+   kfree(nq->tag_map);
+   kfree(nq->cmds);
+}
+
+static void cleanup_queues(struct nullb *nullb)
+{
+   int i;
+
+   for (i = 0; i < nullb->nr_queues; i++)
+   cleanup_queue(>queues[i]);
+
+   kfree(nullb->queues);
+}
+
 static void null_del_dev(struct nullb *nullb)
 {
list_del_init(>list);
@@ -415,6 +431,7 @@ static void null_del_dev(struct nullb *nullb)
if (queue_mode == NULL_Q_MQ)
blk_mq_free_tag_set(>tag_set);
put_disk(nullb->disk);
+   cleanup_queues(nullb);
kfree(nullb);
 }
 
@@ -459,22 +476,6 @@ static int setup_commands(struct nullb_queue *nq)
return 0;
 }
 
-static void cleanup_queue(struct nullb_queue *nq)
-{
-   kfree(nq->tag_map);
-   kfree(nq->cmds);
-}
-
-static void cleanup_queues(struct nullb *nullb)
-{
-   int i;
-
-   for (i = 0; i < nullb->nr_queues; i++)
-   cleanup_queue(>queues[i]);
-
-   kfree(nullb->queues);
-}
-
 static int setup_queues(struct nullb *nullb)
 {
nullb->queues = kzalloc(submit_queues * sizeof(struct nullb_queue),
-- 
2.1.4

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH] null_blk: fix memory leak on cleanup

[Matias Bjørling]

Driver was not freeing the memory allocated for internal nullb queues.
This patch frees the memory during driver unload.

Signed-off-by: Matias Bjørling 
---
 drivers/block/null_blk.c | 33 +
 1 file changed, 17 insertions(+), 16 deletions(-)

diff --git a/drivers/block/null_blk.c b/drivers/block/null_blk.c
index 69de41a..90f6781 100644
--- a/drivers/block/null_blk.c
+++ b/drivers/block/null_blk.c
@@ -406,6 +406,22 @@ static struct blk_mq_ops null_mq_ops = {
.complete   = null_softirq_done_fn,
 };
 
+static void cleanup_queue(struct nullb_queue *nq)
+{
+   kfree(nq->tag_map);
+   kfree(nq->cmds);
+}
+
+static void cleanup_queues(struct nullb *nullb)
+{
+   int i;
+
+   for (i = 0; i < nullb->nr_queues; i++)
+   cleanup_queue(>queues[i]);
+
+   kfree(nullb->queues);
+}
+
 static void null_del_dev(struct nullb *nullb)
 {
list_del_init(>list);
@@ -415,6 +431,7 @@ static void null_del_dev(struct nullb *nullb)
if (queue_mode == NULL_Q_MQ)
blk_mq_free_tag_set(>tag_set);
put_disk(nullb->disk);
+   cleanup_queues(nullb);
kfree(nullb);
 }
 
@@ -459,22 +476,6 @@ static int setup_commands(struct nullb_queue *nq)
return 0;
 }
 
-static void cleanup_queue(struct nullb_queue *nq)
-{
-   kfree(nq->tag_map);
-   kfree(nq->cmds);
-}
-
-static void cleanup_queues(struct nullb *nullb)
-{
-   int i;
-
-   for (i = 0; i < nullb->nr_queues; i++)
-   cleanup_queue(>queues[i]);
-
-   kfree(nullb->queues);
-}
-
 static int setup_queues(struct nullb *nullb)
 {
nullb->queues = kzalloc(submit_queues * sizeof(struct nullb_queue),
-- 
2.1.4

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH] null_blk: fix memory leak on cleanup

[Matias Bjørling]

Driver was not freeing the memory allocated for internal nullb queues.
This patch frees the memory during driver unload.

Signed-off-by: Matias Bjørling m...@lightnvm.io
---
 drivers/block/null_blk.c | 33 +
 1 file changed, 17 insertions(+), 16 deletions(-)

diff --git a/drivers/block/null_blk.c b/drivers/block/null_blk.c
index 69de41a..90f6781 100644
--- a/drivers/block/null_blk.c
+++ b/drivers/block/null_blk.c
@@ -406,6 +406,22 @@ static struct blk_mq_ops null_mq_ops = {
.complete   = null_softirq_done_fn,
 };
 
+static void cleanup_queue(struct nullb_queue *nq)
+{
+   kfree(nq-tag_map);
+   kfree(nq-cmds);
+}
+
+static void cleanup_queues(struct nullb *nullb)
+{
+   int i;
+
+   for (i = 0; i  nullb-nr_queues; i++)
+   cleanup_queue(nullb-queues[i]);
+
+   kfree(nullb-queues);
+}
+
 static void null_del_dev(struct nullb *nullb)
 {
list_del_init(nullb-list);
@@ -415,6 +431,7 @@ static void null_del_dev(struct nullb *nullb)
if (queue_mode == NULL_Q_MQ)
blk_mq_free_tag_set(nullb-tag_set);
put_disk(nullb-disk);
+   cleanup_queues(nullb);
kfree(nullb);
 }
 
@@ -459,22 +476,6 @@ static int setup_commands(struct nullb_queue *nq)
return 0;
 }
 
-static void cleanup_queue(struct nullb_queue *nq)
-{
-   kfree(nq-tag_map);
-   kfree(nq-cmds);
-}
-
-static void cleanup_queues(struct nullb *nullb)
-{
-   int i;
-
-   for (i = 0; i  nullb-nr_queues; i++)
-   cleanup_queue(nullb-queues[i]);
-
-   kfree(nullb-queues);
-}
-
 static int setup_queues(struct nullb *nullb)
 {
nullb-queues = kzalloc(submit_queues * sizeof(struct nullb_queue),
-- 
2.1.4

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/