* [PATCH] mtd: Fix memory leak triggered by removal
@ 2012-02-25 16:57 Fubo Chen
2012-03-09 9:49 ` Artem Bityutskiy
0 siblings, 1 reply; 3+ messages in thread
From: Fubo Chen @ 2012-02-25 16:57 UTC (permalink / raw)
To: linux-mtd; +Cc: David Woodhouse
Make sure that blk_cleanup_queue() is called in del_mtd_blktrans_dev().
Signed-off-by: Fubo Chen <fubo.chen@gmail.com>
Cc: David Woodhouse <dwmw2@infradead.org>
Cc: linux-mtd@lists.infradead.org
---
drivers/mtd/mtd_blkdevs.c | 9 ++-------
1 files changed, 2 insertions(+), 7 deletions(-)
diff --git a/drivers/mtd/mtd_blkdevs.c b/drivers/mtd/mtd_blkdevs.c
index 424ca5f..8b72f24 100644
--- a/drivers/mtd/mtd_blkdevs.c
+++ b/drivers/mtd/mtd_blkdevs.c
@@ -196,7 +196,7 @@ static void mtd_blktrans_request(struct request_queue *rq)
dev = rq->queuedata;
- if (!dev)
+ if (unlikely(blk_queue_dead(rq)))
while ((req = blk_fetch_request(rq)) != NULL)
__blk_end_request_all(req, -ENODEV);
else {
@@ -469,8 +469,6 @@ error1:
int del_mtd_blktrans_dev(struct mtd_blktrans_dev *old)
{
- unsigned long flags;
-
if (mutex_trylock(&mtd_table_mutex)) {
mutex_unlock(&mtd_table_mutex);
BUG();
@@ -488,10 +486,7 @@ int del_mtd_blktrans_dev(struct mtd_blktrans_dev *old)
kthread_stop(old->thread);
/* Kill current requests */
- spin_lock_irqsave(&old->queue_lock, flags);
- old->rq->queuedata = NULL;
- blk_start_queue(old->rq);
- spin_unlock_irqrestore(&old->queue_lock, flags);
+ blk_cleanup_queue(old->rq);
/* If the device is currently open, tell trans driver to close it,
then put mtd device, and don't touch it again */
--
1.7.7
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] mtd: Fix memory leak triggered by removal
2012-03-09 9:49 ` Artem Bityutskiy
@ 2012-03-09 9:48 ` Bityutskiy, Artem
0 siblings, 0 replies; 3+ messages in thread
From: Bityutskiy, Artem @ 2012-03-09 9:48 UTC (permalink / raw)
To: Fubo Chen; +Cc: David Woodhouse, linux-mtd@lists.infradead.org
On Fri, 2012-03-09 at 11:49 +0200, Artem Bityutskiy wrote:
> Please, provide better explanation in the commit message about which
> problem you solve, why you do this, and how you tested this.
> blktrans_dev_release() also calls blk_cleanup_queue() - why we need to
> do it 2 times?
Sorry, you told which problem you solve, but still, could you please
provide more invormation about why the problem existed and when it it
was triggered, and how you fix this and why this fix is correct.
--
Best Regards,
Artem Bityutskiy
---------------------------------------------------------------------
Intel Finland Oy
Registered Address: PL 281, 00181 Helsinki
Business Identity Code: 0357606 - 4
Domiciled in Helsinki
This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] mtd: Fix memory leak triggered by removal
2012-02-25 16:57 [PATCH] mtd: Fix memory leak triggered by removal Fubo Chen
@ 2012-03-09 9:49 ` Artem Bityutskiy
2012-03-09 9:48 ` Bityutskiy, Artem
0 siblings, 1 reply; 3+ messages in thread
From: Artem Bityutskiy @ 2012-03-09 9:49 UTC (permalink / raw)
To: Fubo Chen; +Cc: David Woodhouse, linux-mtd
On Sat, 2012-02-25 at 16:57 +0000, Fubo Chen wrote:
> dev = rq->queuedata;
>
> - if (!dev)
> + if (unlikely(blk_queue_dead(rq)))
> while ((req = blk_fetch_request(rq)) != NULL)
> __blk_end_request_all(req, -ENODEV);
I think this should be a separate patch with own commit message and
explanation. I guess you can kill 'dev' as well then?
> else {
> @@ -469,8 +469,6 @@ error1:
>
> int del_mtd_blktrans_dev(struct mtd_blktrans_dev *old)
> {
> - unsigned long flags;
> -
> if (mutex_trylock(&mtd_table_mutex)) {
> mutex_unlock(&mtd_table_mutex);
> BUG();
> @@ -488,10 +486,7 @@ int del_mtd_blktrans_dev(struct mtd_blktrans_dev *old)
> kthread_stop(old->thread);
>
> /* Kill current requests */
> - spin_lock_irqsave(&old->queue_lock, flags);
> - old->rq->queuedata = NULL;
> - blk_start_queue(old->rq);
> - spin_unlock_irqrestore(&old->queue_lock, flags);
> + blk_cleanup_queue(old->rq);
Please, provide better explanation in the commit message about which
problem you solve, why you do this, and how you tested this.
blktrans_dev_release() also calls blk_cleanup_queue() - why we need to
do it 2 times?
--
Best Regards,
Artem Bityutskiy
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2012-03-09 9:49 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-02-25 16:57 [PATCH] mtd: Fix memory leak triggered by removal Fubo Chen
2012-03-09 9:49 ` Artem Bityutskiy
2012-03-09 9:48 ` Bityutskiy, Artem
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox