NVMe: Fix memory leak on retried commands
Subsystems (3)
PATCH 1/1 NVMe: Fix memory leak on retried commands
Resources are reallocated for requeued commands, so unmap and release the iod for the failed command.
---
Hi Jens,
If you're okay with this fix, it needs to get to 4.0, 4.1 and 4.2 stable, and might cause some issues merging with your 'for-next' branch. This is based on 'for-linus' since this urgently needs to get fixed in 4.3. The 3.19 kernel is okay because we reused the iod in req->special.
Thanks!
Keith
drivers/block/nvme-core.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-)
drivers/block/nvme-core.c +6 -2
@@ -603,6 +603,7 @@ static void req_completion(struct nvme_queue *nvmeq, void *ctx,struct nvme_iod *iod = ctx;struct request *req =iod_get_private (iod);struct nvme_cmd_info *cmd_rq =blk_mq_rq_to_pdu (req); +bool requeue = false;u16 status =le16_to_cpup (&cqe->status) >>1 ;
@@ -611,12 +612,13 @@ static void req_completion(struct nvme_queue *nvmeq, void *ctx, && (jiffies - req->start_time) < req->timeout) {unsigned long flags; + requeue = true;blk_mq_requeue_request (req);spin_lock_irqsave (req->q->queue_lock, flags);if (!blk_queue_stopped (req->q))blk_mq_kick_requeue_list (req->q);spin_unlock_irqrestore (req->q->queue_lock, flags); -return ; + goto release_iod; }if (req->cmd_type ==REQ_TYPE_DRV_PRIV ) {
@@ -637,6 +639,7 @@ static void req_completion(struct nvme_queue *nvmeq, void *ctx,"completing aborted command with status:%04x\n" , status); + release_iod:if (iod->nents) {dma_unmap_sg (nvmeq->dev->dev, iod->sg, iod->nents,rq_data_dir (req) ?DMA_TO_DEVICE :DMA_FROM_DEVICE );
@@ -649,7 +652,8 @@ static void req_completion(struct nvme_queue *nvmeq, void *ctx, }nvme_free_iod (nvmeq->dev, iod); -blk_mq_complete_request (req, status); +if (likely (!requeue)) +blk_mq_complete_request (req, status); }/* length is in bytes. gfp flags indicates whether we may sleep. */
Signature · 1 line
1.7.10.4
PATCH 1/1 NVMe: Fix memory leak on retried commands
Looks good,
PATCH 1/1 NVMe: Fix memory leak on retried commands
In reply to Keith BuschQuoted 2 lines · Keith Busch
Resources are reallocated for requeued commands, so unmap and release the iod for the failed command.
Hi Jens,
Since you're applying changes for 4.3, can you add this one? It's a pretty bad memory leak and causes a kernel hang if you remove a drive because of a busy dma pool. You'll get messages spewing like this:
nvme 0000:xx:xx.x: dma_pool_destroy prp list 256, ffff880420dec000 busy
and lock up pci and the driver since removal never completes while holding a lock.
I think this should go into stable back to 4.0. I'm not entirely sure how to do that. Will the Cc on the original achieve that when this is merged?
PATCH 1/1 NVMe: Fix memory leak on retried commands
Quoted 6 lines · Keith Busch
Quoted 2 lines · Keith Busch
Resources are reallocated for requeued commands, so unmap and release the iod for the failed command.
Hi Jens,
Quoted 8 lines
Since you're applying changes for 4.3, can you add this one? It's a pretty bad memory leak and causes a kernel hang if you remove a drive because of a busy dma pool. You'll get messages spewing like this:
nvme 0000:xx:xx.x: dma_pool_destroy prp list 256, ffff880420dec000 busyand lock up pci and the driver since removal never completes while holding a lock.
Yeah, it's applied. It's hand applied since it no longer applied after the error fixup from Christoph:
Quoted 2 lines
I think this should go into stable back to 4.0. I'm not entirely sure how to do that. Will the Cc on the original achieve that when this is merged?
I think your patch is fine, it's marked stable and with 4.0 and up. So should be no need to do anything else.
Signature · 1 line
Jens Axboe