2019-05-02 19:08:34

by Raul Rangel

[permalink] [raw]
Subject: [PATCH] mmc: Fix tag set memory leak

The tag set is allocated in mmc_init_queue but never freed. This results
in a memory leak. This change makes sure we free the tag set when the
queue is also freed.

Signed-off-by: Raul E Rangel <[email protected]>
---
I found this using kmemleak and plugging and unplugging an SD card in a
few times.

Here is an example of the output of kmemleak:
unreferenced object 0xffff888125be4ce8 (size 8):
comm "kworker/1:0", pid 17, jiffies 4294901575 (age 204.773s)
hex dump (first 8 bytes):
00 00 00 00 00 00 00 00 ........
backtrace:
[<0000000061cb8887>] blk_mq_alloc_tag_set+0xe9/0x234
[<00000000cf532a0f>] mmc_init_queue+0xa9/0x2f0
[<000000001e085171>] mmc_blk_alloc_req+0x125/0x2f9
[<00000000eae1bd01>] mmc_blk_probe+0x1e2/0x6c1
[<00000000a0b4a87d>] really_probe+0x1bd/0x3b0
[<00000000e58f3eb9>] driver_probe_device+0xe1/0x115
[<00000000358f3b3c>] bus_for_each_drv+0x89/0xac
[<00000000ef52ccbe>] __device_attach+0xb0/0x14a
[<00000000c9daafa7>] bus_probe_device+0x33/0x9f
[<0000000008ac5779>] device_add+0x34b/0x5e2
[<00000000b42623cc>] mmc_add_card+0x1f5/0x20d
[<00000000f114ebc3>] mmc_attach_sd+0xc5/0x14b
[<000000006e915e0d>] mmc_rescan+0x261/0x2b6
[<00000000e5b49c26>] process_one_work+0x1d3/0x31f
[<0000000068c8cd3c>] worker_thread+0x1cd/0x2bf
[<00000000326e2e22>] kthread+0x14f/0x157

Once I applied this patch the leak went away.

p.s., I included a small white space fix. Hope that's ok.

drivers/mmc/core/queue.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/mmc/core/queue.c b/drivers/mmc/core/queue.c
index 7c364a9c4eeb..176a08748cf1 100644
--- a/drivers/mmc/core/queue.c
+++ b/drivers/mmc/core/queue.c
@@ -402,7 +402,7 @@ int mmc_init_queue(struct mmc_queue *mq, struct mmc_card *card)

mq->card = card;
mq->use_cqe = host->cqe_enabled;
-
+
spin_lock_init(&mq->lock);

memset(&mq->tag_set, 0, sizeof(mq->tag_set));
@@ -472,6 +472,7 @@ void mmc_cleanup_queue(struct mmc_queue *mq)
blk_mq_unquiesce_queue(q);

blk_cleanup_queue(q);
+ blk_mq_free_tag_set(&mq->tag_set);

/*
* A request can be completed before the next request, potentially
--
2.21.0.593.g511ec345e18-goog


2019-05-02 20:14:21

by Jens Axboe

[permalink] [raw]
Subject: Re: [PATCH] mmc: Fix tag set memory leak

On 5/2/19 1:07 PM, Raul E Rangel wrote:
> The tag set is allocated in mmc_init_queue but never freed. This results
> in a memory leak. This change makes sure we free the tag set when the
> queue is also freed.

Reviewed-by: Jens Axboe <[email protected]>

--
Jens Axboe

2019-05-03 07:25:53

by Adrian Hunter

[permalink] [raw]
Subject: Re: [PATCH] mmc: Fix tag set memory leak

On 2/05/19 10:07 PM, Raul E Rangel wrote:
> The tag set is allocated in mmc_init_queue but never freed. This results
> in a memory leak. This change makes sure we free the tag set when the
> queue is also freed.
>
> Signed-off-by: Raul E Rangel <[email protected]>

One comment below, otherwise:

Fixes: 81196976ed94 ("mmc: block: Add blk-mq support")
Acked-by: Adrian Hunter <[email protected]>

> ---
> I found this using kmemleak and plugging and unplugging an SD card in a
> few times.
>
> Here is an example of the output of kmemleak:
> unreferenced object 0xffff888125be4ce8 (size 8):
> comm "kworker/1:0", pid 17, jiffies 4294901575 (age 204.773s)
> hex dump (first 8 bytes):
> 00 00 00 00 00 00 00 00 ........
> backtrace:
> [<0000000061cb8887>] blk_mq_alloc_tag_set+0xe9/0x234
> [<00000000cf532a0f>] mmc_init_queue+0xa9/0x2f0
> [<000000001e085171>] mmc_blk_alloc_req+0x125/0x2f9
> [<00000000eae1bd01>] mmc_blk_probe+0x1e2/0x6c1
> [<00000000a0b4a87d>] really_probe+0x1bd/0x3b0
> [<00000000e58f3eb9>] driver_probe_device+0xe1/0x115
> [<00000000358f3b3c>] bus_for_each_drv+0x89/0xac
> [<00000000ef52ccbe>] __device_attach+0xb0/0x14a
> [<00000000c9daafa7>] bus_probe_device+0x33/0x9f
> [<0000000008ac5779>] device_add+0x34b/0x5e2
> [<00000000b42623cc>] mmc_add_card+0x1f5/0x20d
> [<00000000f114ebc3>] mmc_attach_sd+0xc5/0x14b
> [<000000006e915e0d>] mmc_rescan+0x261/0x2b6
> [<00000000e5b49c26>] process_one_work+0x1d3/0x31f
> [<0000000068c8cd3c>] worker_thread+0x1cd/0x2bf
> [<00000000326e2e22>] kthread+0x14f/0x157
>
> Once I applied this patch the leak went away.
>
> p.s., I included a small white space fix. Hope that's ok.

Not really. For example, the patch does not apply cleanly to stable trees
only because of that chunk.

>
> drivers/mmc/core/queue.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/mmc/core/queue.c b/drivers/mmc/core/queue.c
> index 7c364a9c4eeb..176a08748cf1 100644
> --- a/drivers/mmc/core/queue.c
> +++ b/drivers/mmc/core/queue.c
> @@ -402,7 +402,7 @@ int mmc_init_queue(struct mmc_queue *mq, struct mmc_card *card)
>
> mq->card = card;
> mq->use_cqe = host->cqe_enabled;
> -
> +
> spin_lock_init(&mq->lock);
>
> memset(&mq->tag_set, 0, sizeof(mq->tag_set));
> @@ -472,6 +472,7 @@ void mmc_cleanup_queue(struct mmc_queue *mq)
> blk_mq_unquiesce_queue(q);
>
> blk_cleanup_queue(q);
> + blk_mq_free_tag_set(&mq->tag_set);
>
> /*
> * A request can be completed before the next request, potentially
>

2019-05-03 10:45:09

by Ulf Hansson

[permalink] [raw]
Subject: Re: [PATCH] mmc: Fix tag set memory leak

On Thu, 2 May 2019 at 21:07, Raul E Rangel <[email protected]> wrote:
>
> The tag set is allocated in mmc_init_queue but never freed. This results
> in a memory leak. This change makes sure we free the tag set when the
> queue is also freed.
>
> Signed-off-by: Raul E Rangel <[email protected]>

Applied for fixes (it may not make it for v5.1 unless there is a rc8), thanks!

I also dropped the white space fixup, for the reasons explained by Adrian.

Kind regards
Uffe

> ---
> I found this using kmemleak and plugging and unplugging an SD card in a
> few times.
>
> Here is an example of the output of kmemleak:
> unreferenced object 0xffff888125be4ce8 (size 8):
> comm "kworker/1:0", pid 17, jiffies 4294901575 (age 204.773s)
> hex dump (first 8 bytes):
> 00 00 00 00 00 00 00 00 ........
> backtrace:
> [<0000000061cb8887>] blk_mq_alloc_tag_set+0xe9/0x234
> [<00000000cf532a0f>] mmc_init_queue+0xa9/0x2f0
> [<000000001e085171>] mmc_blk_alloc_req+0x125/0x2f9
> [<00000000eae1bd01>] mmc_blk_probe+0x1e2/0x6c1
> [<00000000a0b4a87d>] really_probe+0x1bd/0x3b0
> [<00000000e58f3eb9>] driver_probe_device+0xe1/0x115
> [<00000000358f3b3c>] bus_for_each_drv+0x89/0xac
> [<00000000ef52ccbe>] __device_attach+0xb0/0x14a
> [<00000000c9daafa7>] bus_probe_device+0x33/0x9f
> [<0000000008ac5779>] device_add+0x34b/0x5e2
> [<00000000b42623cc>] mmc_add_card+0x1f5/0x20d
> [<00000000f114ebc3>] mmc_attach_sd+0xc5/0x14b
> [<000000006e915e0d>] mmc_rescan+0x261/0x2b6
> [<00000000e5b49c26>] process_one_work+0x1d3/0x31f
> [<0000000068c8cd3c>] worker_thread+0x1cd/0x2bf
> [<00000000326e2e22>] kthread+0x14f/0x157
>
> Once I applied this patch the leak went away.
>
> p.s., I included a small white space fix. Hope that's ok.
>
> drivers/mmc/core/queue.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/mmc/core/queue.c b/drivers/mmc/core/queue.c
> index 7c364a9c4eeb..176a08748cf1 100644
> --- a/drivers/mmc/core/queue.c
> +++ b/drivers/mmc/core/queue.c
> @@ -402,7 +402,7 @@ int mmc_init_queue(struct mmc_queue *mq, struct mmc_card *card)
>
> mq->card = card;
> mq->use_cqe = host->cqe_enabled;
> -
> +
> spin_lock_init(&mq->lock);
>
> memset(&mq->tag_set, 0, sizeof(mq->tag_set));
> @@ -472,6 +472,7 @@ void mmc_cleanup_queue(struct mmc_queue *mq)
> blk_mq_unquiesce_queue(q);
>
> blk_cleanup_queue(q);
> + blk_mq_free_tag_set(&mq->tag_set);
>
> /*
> * A request can be completed before the next request, potentially
> --
> 2.21.0.593.g511ec345e18-goog
>