Re: [PATCH v2] VMW_PVSCSI: Fix the issue of DMA-API related warnings.

2015-12-10 Thread Martin K. Petersen 
> "Josh" == Josh Boyer  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.

-- 
Martin K. Petersen  Oracle Linux Engineering
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2] VMW_PVSCSI: Fix the issue of DMA-API related warnings.

2015-12-04 Thread Arvind Kumar 
Thanks Josh!

The patch looks good to me.

Thanks!
Arvind

From: Johannes Thumshirn <jthumsh...@suse.de>
Sent: Thursday, December 3, 2015 5:35 AM
To: Josh Boyer; james.bottom...@hansenpartnership.com; Arvind Kumar; Thomas 
Hellstrom
Cc: linux-scsi@vger.kernel.org; pv-driv...@vmware.com; 
linux-ker...@vger.kernel.org
Subject: Re: [PATCH v2] VMW_PVSCSI: Fix the issue of DMA-API related warnings.

On Thu, 2015-12-03 at 08:27 -0500, Josh Boyer wrote:
> 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 <arvindku...@vmware.com>
> Cc: Josh Boyer <jwbo...@fedoraproject.org>
> Reviewed-by: Thomas Hellstrom <thellst...@vmware.com>
> Signed-off-by: Josh Boyer <jwbo...@fedoraproject.org>
> ---
>
>  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(-)
>
> diff --git a/drivers/scsi/vmw_pvscsi.c b/drivers/scsi/vmw_pvscsi.c
> index 0f133c1817de..6164634aff18 100644
> --- a/drivers/scsi/vmw_pvscsi.c
> +++ b/drivers/scsi/vmw_pvscsi.c
> @@ -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,
>

Re: [PATCH v2] VMW_PVSCSI: Fix the issue of DMA-API related warnings.

2015-12-03 Thread Johannes Thumshirn 
On Thu, 2015-12-03 at 08:27 -0500, Josh Boyer wrote:
> 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 
> Cc: Josh Boyer 
> Reviewed-by: Thomas Hellstrom 
> Signed-off-by: Josh Boyer 
> ---
> 
>  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(-)
> 
> diff --git a/drivers/scsi/vmw_pvscsi.c b/drivers/scsi/vmw_pvscsi.c
> index 0f133c1817de..6164634aff18 100644
> --- a/drivers/scsi/vmw_pvscsi.c
> +++ b/drivers/scsi/vmw_pvscsi.c
> @@ -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,
> +