Skip to contentSkip to conversation

VMW_PVSCSI: Fix the issue of DMA-API related warnings.

PATCH Applied Josh Boyer 4 participants 4 messages Dec 3, 2015 – Dec 10, 2015

Subsystems (2)
SCSI VMware pvscsi driver
lore
Share & tools
lore

Josh Boyer

author
#0 lore raw

PATCH v2 1/1 VMW_PVSCSI: Fix the issue of DMA-API related warnings.

The driver is missing calls to pci_dma_mapping_error() after performing the DMA mapping, which caused DMA-API warning to show up in dmesg's output. Though that happens only when DMA_API_DEBUG option is enabled. This change fixes the issue and makes pvscsi_map_buffers() function more robust.

Signed-off-by Arvind Kumar <[email protected]>
Cc Josh Boyer <[email protected]>
Reviewed-by Thomas Hellstrom <[email protected]>
Signed-off-by Josh Boyer <[email protected]>

---

v2: Use -ENOMEM instead of -1 for the error return code as suggested by
Johannes Thumshirn

 drivers/scsi/vmw_pvscsi.c | 45 +++++++++++++++++++++++++++++++++++++++------
 drivers/scsi/vmw_pvscsi.h |  2 +-
 2 files changed, 40 insertions(+), 7 deletions(-)
drivers/scsi/vmw_pvscsi.c +39 -6
index 0f133c1817de..6164634aff18 100644
@@ -349,9 +349,9 @@ static void pvscsi_create_sg(struct pvscsi_ctx *ctx,
  * Map all data buffers for a command into PCI space and
  * setup the scatter/gather list if needed.
  */
-static void pvscsi_map_buffers(struct pvscsi_adapter *adapter,
-			       struct pvscsi_ctx *ctx, struct scsi_cmnd *cmd,
-			       struct PVSCSIRingReqDesc *e)
+static int pvscsi_map_buffers(struct pvscsi_adapter *adapter,
+			      struct pvscsi_ctx *ctx, struct scsi_cmnd *cmd,
+			      struct PVSCSIRingReqDesc *e)
 {
 	unsigned count;
 	unsigned bufflen = scsi_bufflen(cmd);

@@ -360,18 +360,30 @@ static void pvscsi_map_buffers(struct pvscsi_adapter *adapter,
 	e->dataLen = bufflen;
 	e->dataAddr = 0;
 	if (bufflen == 0)
-		return;
+		return 0;
 
 	sg = scsi_sglist(cmd);
 	count = scsi_sg_count(cmd);
 	if (count != 0) {
 		int segs = scsi_dma_map(cmd);
-		if (segs > 1) {
+
+		if (segs == -ENOMEM) {
+			scmd_printk(KERN_ERR, cmd,
+				    "vmw_pvscsi: Failed to map cmd sglist for DMA.\n");
+			return -ENOMEM;
+		} else if (segs > 1) {
 			pvscsi_create_sg(ctx, sg, segs);
 
 			e->flags |= PVSCSI_FLAG_CMD_WITH_SG_LIST;
 			ctx->sglPA = pci_map_single(adapter->dev, ctx->sgl,
 						    SGL_SIZE, PCI_DMA_TODEVICE);
+			if (pci_dma_mapping_error(adapter->dev, ctx->sglPA)) {
+				scmd_printk(KERN_ERR, cmd,
+					    "vmw_pvscsi: Failed to map ctx sglist for DMA.\n");
+				scsi_dma_unmap(cmd);
+				ctx->sglPA = 0;
+				return -ENOMEM;
+			}
 			e->dataAddr = ctx->sglPA;
 		} else
 			e->dataAddr = sg_dma_address(sg);

@@ -382,8 +394,15 @@ static void pvscsi_map_buffers(struct pvscsi_adapter *adapter,
 		 */
 		ctx->dataPA = pci_map_single(adapter->dev, sg, bufflen,
 					     cmd->sc_data_direction);
+		if (pci_dma_mapping_error(adapter->dev, ctx->dataPA)) {
+			scmd_printk(KERN_ERR, cmd,
+				    "vmw_pvscsi: Failed to map direct data buffer for DMA.\n");
+			return -ENOMEM;
+		}
 		e->dataAddr = ctx->dataPA;
 	}
+
+	return 0;
 }
 
 static void pvscsi_unmap_buffers(const struct pvscsi_adapter *adapter,

@@ -690,6 +709,12 @@ static int pvscsi_queue_ring(struct pvscsi_adapter *adapter,
 		ctx->sensePA = pci_map_single(adapter->dev, cmd->sense_buffer,
 					      SCSI_SENSE_BUFFERSIZE,
 					      PCI_DMA_FROMDEVICE);
+		if (pci_dma_mapping_error(adapter->dev, ctx->sensePA)) {
+			scmd_printk(KERN_ERR, cmd,
+				    "vmw_pvscsi: Failed to map sense buffer for DMA.\n");
+			ctx->sensePA = 0;
+			return -ENOMEM;
+		}
 		e->senseAddr = ctx->sensePA;
 		e->senseLen = SCSI_SENSE_BUFFERSIZE;
 	} else {

@@ -711,7 +736,15 @@ static int pvscsi_queue_ring(struct pvscsi_adapter *adapter,
 	else
 		e->flags = 0;
 
-	pvscsi_map_buffers(adapter, ctx, cmd, e);
+	if (pvscsi_map_buffers(adapter, ctx, cmd, e) != 0) {
+		if (cmd->sense_buffer) {
+			pci_unmap_single(adapter->dev, ctx->sensePA,
+					 SCSI_SENSE_BUFFERSIZE,
+					 PCI_DMA_FROMDEVICE);
+			ctx->sensePA = 0;
+		}
+		return -ENOMEM;
+	}
 
 	e->context = pvscsi_map_context(adapter, ctx);
drivers/scsi/vmw_pvscsi.h +1 -1
index ee16f0c5c47d..12712c92f37a 100644
@@ -26,7 +26,7 @@
 
 #include <linux/types.h>
 
-#define PVSCSI_DRIVER_VERSION_STRING   "1.0.5.0-k"
+#define PVSCSI_DRIVER_VERSION_STRING   "1.0.6.0-k"
 
 #define PVSCSI_MAX_NUM_SG_ENTRIES_PER_SEGMENT 128
Signature · 1 line
2.5.0
Reviewed-by on v2 1/1
#1 lore raw
Quoted 137 lines · Josh Boyer

The driver is missing calls to pci_dma_mapping_error() after performing the DMA mapping, which caused DMA-API warning to show up in dmesg's output. Though that happens only when DMA_API_DEBUG option is enabled. This change fixes the issue and makes pvscsi_map_buffers() function more robust.

Signed-off-by Arvind Kumar <[email protected]>
Cc Josh Boyer <[email protected]>
Reviewed-by Thomas Hellstrom <[email protected]>
Signed-off-by Josh Boyer <[email protected]>

---

 v2: Use -ENOMEM instead of -1 for the error return code as suggested by Johannes Thumshirn

 drivers/scsi/vmw_pvscsi.c | 45 +++++++++++++++++++++++++++++++++++++++------ drivers/scsi/vmw_pvscsi.h |  2 +-

 2 files changed, 40 insertions(+), 7 deletions(-)
drivers/scsi/vmw_pvscsi.c +3 -3
index 0f133c1817de..6164634aff18 100644
@@ -349,9 +349,9 @@ static void pvscsi_create_sg(struct pvscsi_ctx *ctx,
   * Map all data buffers for a command into PCI space and
   * setup the scatter/gather list if needed.
   */
-static void pvscsi_map_buffers(struct pvscsi_adapter *adapter,
-			       struct pvscsi_ctx *ctx, struct scsi_cmnd
 *cmd,
-			       struct PVSCSIRingReqDesc *e)
+static int pvscsi_map_buffers(struct pvscsi_adapter *adapter,
+			      struct pvscsi_ctx *ctx, struct scsi_cmnd *cmd,
+			      struct PVSCSIRingReqDesc *e)
  {
  	unsigned count;

  unsigned bufflen = scsi_bufflen(cmd);

drivers/scsi/vmw_pvscsi.c +1 -1
@@ -360,18 +360,30 @@ static void pvscsi_map_buffers(struct pvscsi_adapter
 *adapter,
  	e->dataLen = bufflen;
  	e->dataAddr = 0;
  	if (bufflen == 0)
-		return;
+		return 0;


  sg = scsi_sglist(cmd);
  count = scsi_sg_count(cmd);
  if (count != 0) {
  int segs = scsi_dma_map(cmd);

drivers/scsi/vmw_pvscsi.c +4 -1
-		if (segs > 1) {
+
+		if (segs == -ENOMEM) {
+			scmd_printk(KERN_ERR, cmd,
+				    "vmw_pvscsi: Failed to map cmd sglist

for DMA.\n");

drivers/scsi/vmw_pvscsi.c +2 -0
+			return -ENOMEM;
+		} else if (segs > 1) {

  pvscsi_create_sg(ctx, sg, segs);

  e->flags |= PVSCSI_FLAG_CMD_WITH_SG_LIST; ctx->sglPA = pci_map_single(adapter->dev, ctx->sgl, SGL_SIZE, PCI_DMA_TODEVICE);

drivers/scsi/vmw_pvscsi.c +1 -0
+			if (pci_dma_mapping_error(adapter->dev, ctx->sglPA))

{

drivers/scsi/vmw_pvscsi.c +2 -0
+				scmd_printk(KERN_ERR, cmd,
+					    "vmw_pvscsi: Failed to map ctx

sglist for DMA.\n");

drivers/scsi/vmw_pvscsi.c +4 -0
+				scsi_dma_unmap(cmd);
+				ctx->sglPA = 0;
+				return -ENOMEM;
+			}

  e->dataAddr = ctx->sglPA; } else
  e->dataAddr = sg_dma_address(sg);

drivers/scsi/vmw_pvscsi.c +7 -0
@@ -382,8 +394,15 @@ static void pvscsi_map_buffers(struct pvscsi_adapter
 *adapter,
  		 */
  		ctx->dataPA = pci_map_single(adapter->dev, sg, bufflen,
  					     cmd->sc_data_direction);
+		if (pci_dma_mapping_error(adapter->dev, ctx->dataPA)) {
+			scmd_printk(KERN_ERR, cmd,
+				    "vmw_pvscsi: Failed to map direct data
 buffer for DMA.\n");
+			return -ENOMEM;
+		}
  		e->dataAddr = ctx->dataPA;
  	}
+
+	return 0;
  }


 static void pvscsi_unmap_buffers(const struct pvscsi_adapter *adapter,

drivers/scsi/vmw_pvscsi.c +0 -0
@@ -690,6 +709,12 @@ static int pvscsi_queue_ring(struct pvscsi_adapter
 *adapter,

  ctx->sensePA = pci_map_single(adapter->dev, cmd-

Quoted 1 line

sense_buffer,

        SCSI_SENSE_BUFFERSIZE, PCI_DMA_FROMDEVICE);

drivers/scsi/vmw_pvscsi.c +3 -0
+		if (pci_dma_mapping_error(adapter->dev, ctx->sensePA)) {
+			scmd_printk(KERN_ERR, cmd,
+				    "vmw_pvscsi: Failed to map sense buffer

for DMA.\n");

drivers/scsi/vmw_pvscsi.c +3 -0
+			ctx->sensePA = 0;
+			return -ENOMEM;
+		}

  e->senseAddr = ctx->sensePA; e->senseLen = SCSI_SENSE_BUFFERSIZE; } else {

drivers/scsi/vmw_pvscsi.c +0 -0
@@ -711,7 +736,15 @@ static int pvscsi_queue_ring(struct pvscsi_adapter
 *adapter,

  else
  e->flags = 0;

drivers/scsi/vmw_pvscsi.c +9 -1
-	pvscsi_map_buffers(adapter, ctx, cmd, e);
+	if (pvscsi_map_buffers(adapter, ctx, cmd, e) != 0) {
+		if (cmd->sense_buffer) {
+			pci_unmap_single(adapter->dev, ctx->sensePA,
+					 SCSI_SENSE_BUFFERSIZE,
+					 PCI_DMA_FROMDEVICE);
+			ctx->sensePA = 0;
+		}
+		return -ENOMEM;
+	}


  e->context = pvscsi_map_context(adapter, ctx);

drivers/scsi/vmw_pvscsi.h +0 -0
index ee16f0c5c47d..12712c92f37a 100644
@@ -26,7 +26,7 @@


 #include <linux/types.h>

drivers/scsi/vmw_pvscsi.h +1 -1
-#define PVSCSI_DRIVER_VERSION_STRING   "1.0.5.0-k"
+#define PVSCSI_DRIVER_VERSION_STRING   "1.0.6.0-k"
Quoted 3 lines


 #define PVSCSI_MAX_NUM_SG_ENTRIES_PER_SEGMENT 128

Reviewed-by Johannes Thumshirn <[email protected]>
Reviewed-by Johannes Thumshirn
#2 lore raw

Thanks Josh!

The patch looks good to me.

Thanks!
Arvind
________________________________________
From: Johannes Thumshirn <[email protected]>
Sent: Thursday, December 3, 2015 5:35 AM
To: Josh Boyer; [email protected]; Arvind Kumar; Thomas Hellstrom
Cc: [email protected]; [email protected]; [email protected]
Subject: Re: [PATCH v2] VMW_PVSCSI: Fix the issue of DMA-API related warnings.

Quoted 132 lines · Josh Boyer

The driver is missing calls to pci_dma_mapping_error() after performing the DMA mapping, which caused DMA-API warning to show up in dmesg's output. Though that happens only when DMA_API_DEBUG option is enabled. This change fixes the issue and makes pvscsi_map_buffers() function more robust.

Signed-off-by Arvind Kumar <[email protected]>
Cc Josh Boyer <[email protected]>
Reviewed-by Thomas Hellstrom <[email protected]>
Signed-off-by Josh Boyer <[email protected]>

---

v2: Use -ENOMEM instead of -1 for the error return code as suggested by
Johannes Thumshirn

 drivers/scsi/vmw_pvscsi.c | 45 +++++++++++++++++++++++++++++++++++++++------
 drivers/scsi/vmw_pvscsi.h |  2 +-
 2 files changed, 40 insertions(+), 7 deletions(-)
drivers/scsi/vmw_pvscsi.c +3 -3
index 0f133c1817de..6164634aff18 100644
@@ -349,9 +349,9 @@ static void pvscsi_create_sg(struct pvscsi_ctx *ctx,
  * Map all data buffers for a command into PCI space and
  * setup the scatter/gather list if needed.
  */
-static void pvscsi_map_buffers(struct pvscsi_adapter *adapter,
-                            struct pvscsi_ctx *ctx, struct scsi_cmnd
 *cmd,
-                            struct PVSCSIRingReqDesc *e)
+static int pvscsi_map_buffers(struct pvscsi_adapter *adapter,
+                           struct pvscsi_ctx *ctx, struct scsi_cmnd *cmd,
+                           struct PVSCSIRingReqDesc *e)
 {
      unsigned count;

      unsigned bufflen = scsi_bufflen(cmd);
drivers/scsi/vmw_pvscsi.c +8 -2
@@ -360,18 +360,30 @@ static void pvscsi_map_buffers(struct pvscsi_adapter
 *adapter,
      e->dataLen = bufflen;
      e->dataAddr = 0;
      if (bufflen == 0)
-             return;
+             return 0;
 
      sg = scsi_sglist(cmd);
      count = scsi_sg_count(cmd);
      if (count != 0) {
              int segs = scsi_dma_map(cmd);
-             if (segs > 1) {
+
+             if (segs == -ENOMEM) {
+                     scmd_printk(KERN_ERR, cmd,
+                                 "vmw_pvscsi: Failed to map cmd sglist
 for DMA.\n");
+                     return -ENOMEM;
+             } else if (segs > 1) {
                      pvscsi_create_sg(ctx, sg, segs);
 
                      e->flags |= PVSCSI_FLAG_CMD_WITH_SG_LIST;
                      ctx->sglPA = pci_map_single(adapter->dev, ctx->sgl,
                                                  SGL_SIZE,
 PCI_DMA_TODEVICE);
+                     if (pci_dma_mapping_error(adapter->dev, ctx->sglPA))

{

drivers/scsi/vmw_pvscsi.c +2 -0
+                             scmd_printk(KERN_ERR, cmd,
+                                         "vmw_pvscsi: Failed to map ctx

sglist for DMA.\n");

drivers/scsi/vmw_pvscsi.c +11 -0
+                             scsi_dma_unmap(cmd);
+                             ctx->sglPA = 0;
+                             return -ENOMEM;
+                     }
                      e->dataAddr = ctx->sglPA;
              } else
                      e->dataAddr = sg_dma_address(sg);

@@ -382,8 +394,15 @@ static void pvscsi_map_buffers(struct pvscsi_adapter
 *adapter,
               */
              ctx->dataPA = pci_map_single(adapter->dev, sg, bufflen,
                                           cmd->sc_data_direction);
+             if (pci_dma_mapping_error(adapter->dev, ctx->dataPA)) {
+                     scmd_printk(KERN_ERR, cmd,
+                                 "vmw_pvscsi: Failed to map direct data
 buffer for DMA.\n");
+                     return -ENOMEM;
+             }
              e->dataAddr = ctx->dataPA;
      }
+
+     return 0;
 }

static void pvscsi_unmap_buffers(const struct pvscsi_adapter *adapter,

drivers/scsi/vmw_pvscsi.c +6 -0
@@ -690,6 +709,12 @@ static int pvscsi_queue_ring(struct pvscsi_adapter
 *adapter,
              ctx->sensePA = pci_map_single(adapter->dev, cmd-
 >sense_buffer,
                                            SCSI_SENSE_BUFFERSIZE,
                                            PCI_DMA_FROMDEVICE);
+             if (pci_dma_mapping_error(adapter->dev, ctx->sensePA)) {
+                     scmd_printk(KERN_ERR, cmd,
+                                 "vmw_pvscsi: Failed to map sense buffer
 for DMA.\n");
+                     ctx->sensePA = 0;
+                     return -ENOMEM;
+             }

              e->senseAddr = ctx->sensePA;
              e->senseLen = SCSI_SENSE_BUFFERSIZE;
      } else {
drivers/scsi/vmw_pvscsi.c +9 -1
@@ -711,7 +736,15 @@ static int pvscsi_queue_ring(struct pvscsi_adapter
 *adapter,
      else
              e->flags = 0;
 
-     pvscsi_map_buffers(adapter, ctx, cmd, e);
+     if (pvscsi_map_buffers(adapter, ctx, cmd, e) != 0) {
+             if (cmd->sense_buffer) {
+                     pci_unmap_single(adapter->dev, ctx->sensePA,
+                                      SCSI_SENSE_BUFFERSIZE,
+                                      PCI_DMA_FROMDEVICE);
+                     ctx->sensePA = 0;
+             }
+             return -ENOMEM;
+     }
 
      e->context = pvscsi_map_context(adapter, ctx);
drivers/scsi/vmw_pvscsi.h +0 -0
index ee16f0c5c47d..12712c92f37a 100644

drivers/scsi/vmw_pvscsi.h line 32

Quoted 8 lines
drivers/scsi/vmw_pvscsi.h +1 -1
@@ -26,7 +26,7 @@
 
 #include <linux/types.h>
 
-#define PVSCSI_DRIVER_VERSION_STRING   "1.0.5.0-k"
+#define PVSCSI_DRIVER_VERSION_STRING   "1.0.6.0-k"
 
 #define PVSCSI_MAX_NUM_SG_ENTRIES_PER_SEGMENT 128
Reviewed-by Johannes Thumshirn <[email protected]>
Applied
#3 lore raw
In reply to Josh Boyer
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line

"Josh" == Josh Boyer <[email protected]> writes:

Josh> The driver is missing calls to pci_dma_mapping_error() after Josh> performing the DMA mapping, which caused DMA-API warning to show Josh> up in dmesg's output. Though that happens only when DMA_API_DEBUG Josh> option is enabled. This change fixes the issue and makes Josh> pvscsi_map_buffers() function more robust.

Applied to 4.5/scsi-queue.

Signature · 1 line
Martin K. Petersen	Oracle Linux Engineering