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

[Johannes Thumshirn]

Hi Josh,

On Tue, 2015-12-01 at 11:34 -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 <arvindkumar@xxxxxxxxxx>
> Cc: Josh Boyer <jwboyer@xxxxxxxxxxxxxxxxx>
> Reviewed-by: Thomas Hellstrom <thellstrom@xxxxxxxxxx>
> Signed-off-by: Josh Boyer <jwboyer@xxxxxxxxxxxxxxxxx>
> ---
> 
> Â- Resend of patch that was never committed for some reason
> 
> Â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..19734494f9ec 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 -1;

Please return -ENOMEM instead of -1

> +		} 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 -1;

Same here.

> +			}
> Â			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 -1;

And here.

> +		}
> Â		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 -1;

And here.

> +		}
> Â		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 -1;

As pvscsi_map_buffers() only returns 0 or -ENOMEM please return -ENOMEM here as
well, or do

	int err;
	[...]
	err = pvscsi_map_buffers(adapter, ctx, cmd, e);
	if (err) {
		[...]
		return err;
	}

> +	}
> Â
> Â	e->context = pvscsi_map_context(adapter, ctx);
> Â
> diff --git a/drivers/scsi/vmw_pvscsi.h b/drivers/scsi/vmw_pvscsi.h
> index ee16f0c5c47d..12712c92f37a 100644
> --- a/drivers/scsi/vmw_pvscsi.h
> +++ b/drivers/scsi/vmw_pvscsi.h
> @@ -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
> Â

Thanks,
	Johannes
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/