Skip to contentSkip to conversation

mtd: Fix memory leak triggered by removal

PATCH In review Fubo Chen 3 participants 3 messages Feb 25, 2012 – Mar 9, 2012 Memory technology devices (MTD)

lore
Share & tools
lore

Fubo Chen

author
#0 lore raw

PATCH 1/1 mtd: Fix memory leak triggered by removal

Make sure that blk_cleanup_queue() is called in del_mtd_blktrans_dev().

Signed-off-by Fubo Chen <[email protected]>
Cc David Woodhouse <[email protected]>
Cc [email protected]
---
 drivers/mtd/mtd_blkdevs.c |    9 ++-------
 1 files changed, 2 insertions(+), 7 deletions(-)
drivers/mtd/mtd_blkdevs.c +2 -7
index 424ca5f..8b72f24 100644
@@ -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 */
Signature · 1 line
1.7.7
#1 lore raw

drivers/mtd/mtd_blkdevs.c static void mtd_blktrans_request(struct request_queue *rq) · line 201

Quoted 6 lines · Fubo Chen
drivers/mtd/mtd_blkdevs.c +1 -1
 	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?

Quoted 11 lines
drivers/mtd/mtd_blkdevs.c +0 -2
 	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();

drivers/mtd/mtd_blkdevs.c int del_mtd_blktrans_dev(struct mtd_blktrans_dev *old) · line 489

Quoted 8 lines
drivers/mtd/mtd_blkdevs.c +1 -4
@@ -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?

Signature · 2 lines
Best Regards,
Artem Bityutskiy
#2 lore raw
Quoted 4 lines · Artem Bityutskiy

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.

Signature · 12 lines
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.