nvme/pci: Remove nvme_setup_prps BUG_ON
PATCH 1/1 nvme/pci: Remove nvme_setup_prps BUG_ON
This patch replaces the invalid nvme SGL kernel panic with a warning, and returns an appropriate error. The warning will occur only on the first occurance, and sgl details will be printed to help debug how the request was allowed to form.
--- drivers/nvme/host/pci.c | 33 +++++++++++++++++++++++++-------- 1 file changed, 25 insertions(+), 8 deletions(-)
drivers/nvme/host/pci.c +25 -8
@@ -533,7 +533,7 @@ static void nvme_dif_complete(u32 p, u32 v, struct t10_pi_tuple *pi) }#endif -static bool nvme_setup_prps (struct nvme_dev *dev,struct request *req) +static blk_status_t nvme_setup_prps (struct nvme_dev *dev,struct request *req) {struct nvme_iod *iod =blk_mq_rq_to_pdu (req);struct dma_pool *pool;
@@ -550,7 +550,7 @@ static bool nvme_setup_prps(struct nvme_dev *dev, struct request *req) length -= (page_size - offset);if (length <=0 ) -return true; +return BLK_STS_OK ; dma_len -= (page_size - offset);if (dma_len) {
@@ -563,7 +563,7 @@ static bool nvme_setup_prps(struct nvme_dev *dev, struct request *req)if (length <= page_size) { iod->first_dma = dma_addr; -return true; +return BLK_STS_OK ; } nprps =DIV_ROUND_UP (length, page_size);
@@ -579,7 +579,7 @@ static bool nvme_setup_prps(struct nvme_dev *dev, struct request *req)if (!prp_list) { iod->first_dma = dma_addr; iod->npages =-1 ; -return false; +return BLK_STS_RESOURCE ; } list[0 ] = prp_list; iod->first_dma = prp_dma;
@@ -589,7 +589,7 @@ static bool nvme_setup_prps(struct nvme_dev *dev, struct request *req)__le64 *old_prp_list = prp_list; prp_list =dma_pool_alloc (pool,GFP_ATOMIC , &prp_dma);if (!prp_list) -return false; +return BLK_STS_RESOURCE ; list[iod->npages++] = prp_list; prp_list[0 ] = old_prp_list[i -1 ]; old_prp_list[i -1 ] =cpu_to_le64 (prp_dma);
@@ -603,13 +603,29 @@ static bool nvme_setup_prps(struct nvme_dev *dev, struct request *req)break ;if (dma_len >0 )continue ; -BUG_ON (dma_len <0 ); +if (unlikely (dma_len <0 )) + goto bad_sgl; sg =sg_next (sg); dma_addr =sg_dma_address (sg); dma_len =sg_dma_len (sg); } -return true; +return BLK_STS_OK ; + + bad_sgl: +if (WARN_ONCE (1 ,"Invalid SGL for payload:%d nents:%d\n" , +blk_rq_payload_bytes (req), iod->nents)) { +for_each_sg (iod->sg, sg, iod->nents, i) { +dma_addr_t phys =sg_phys (sg); +pr_warn ("sg[%d] phys_addr:%pad offset:%d length:%d " +"dma_address:%pad dma_length:%d\n" , i, &phys, + sg->offset, sg->length, + &sg_dma_address (sg), +sg_dma_len (sg)); + } + } +return BLK_STS_IOERR ; + } static blk_status_t nvme_map_data(struct nvme_dev *dev, struct request *req,
@@ -631,7 +647,8 @@ static blk_status_t nvme_map_data(struct nvme_dev *dev, struct request *req, DMA_ATTR_NO_WARN)) goto out; -if (!nvme_setup_prps (dev, req)) + ret =nvme_setup_prps (dev, req); +if (ret !=BLK_STS_OK ) goto out_unmap; ret =BLK_STS_IOERR ;
Signature · 1 line
2.5.5
Sagi Grimberg
maintainerPATCH 1/1 nvme/pci: Remove nvme_setup_prps BUG_ON
Quoted 4 lines
This patch replaces the invalid nvme SGL kernel panic with a warning, and returns an appropriate error. The warning will occur only on the first occurance, and sgl details will be printed to help debug how the request was allowed to form.
That's definitely needed! Looks great,
Christoph Hellwig
maintainerPATCH 1/1 nvme/pci: Remove nvme_setup_prps BUG_ON
In reply to Keith BuschLooks fine,
PATCH 1/1 nvme/pci: Remove nvme_setup_prps BUG_ON
In reply to Keith BuschHighly appreciated,
Reviewed-by: Johannes Thumshirn <jthumshirn at suse.de>
Signature · 6 lines
Johannes Thumshirn Storage jthumshirn at suse.de +49 911 74053 689 SUSE LINUX GmbH, Maxfeldstr. 5, 90409 N?rnberg GF: Felix Imend?rffer, Jane Smithard, Graham Norton HRB 21284 (AG N?rnberg) Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850