2015-12-01 16:34:15

by Josh Boyer

[permalink] [raw]
Subject: [PATCH Resend] 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]>
---

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

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

--
2.5.0


2015-12-02 08:42:49

by Johannes Thumshirn

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

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 <[email protected]>
> Cc: Josh Boyer <[email protected]>
> Reviewed-by: Thomas Hellstrom <[email protected]>
> Signed-off-by: Josh Boyer <[email protected]>
> ---
>
>  - 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

2015-12-02 13:47:53

by Josh Boyer

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

On Wed, Dec 2, 2015 at 3:42 AM, Johannes Thumshirn <[email protected]> wrote:
> 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 <[email protected]>
>> Cc: Josh Boyer <[email protected]>
>> Reviewed-by: Thomas Hellstrom <[email protected]>
>> Signed-off-by: Josh Boyer <[email protected]>
>> ---
>>
>> - 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;
> }
>

Thanks for the suggestions. They all seem reasonable to me.

Arvind, since I was originally just resending your patch, do you want
to make the changes Johannes suggests, or should I proceed with that?

josh

2015-12-02 20:04:15

by Arvind Kumar

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

The suggestions look reasonable to me too.

>Arvind, since I was originally just resending your patch, do you want
>to make the changes Johannes suggests, or should I proceed with that?
> josh

Hi Josh,

Feel free to send out the updated patch if you would like.

Thanks!
Arvind
________________________________________
From: [email protected] <[email protected]> on behalf of Josh Boyer <[email protected]>
Sent: Wednesday, December 2, 2015 5:47 AM
To: Johannes Thumshirn
Cc: Jej B; Arvind Kumar; Thomas Hellstrom; [email protected]; VMware PV-Drivers; Linux-Kernel@Vger. Kernel. Org
Subject: Re: [PATCH Resend] VMW_PVSCSI: Fix the issue of DMA-API related warnings.

On Wed, Dec 2, 2015 at 3:42 AM, Johannes Thumshirn <[email protected]> wrote:
> 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 <[email protected]>
>> Cc: Josh Boyer <[email protected]>
>> Reviewed-by: Thomas Hellstrom <[email protected]>
>> Signed-off-by: Josh Boyer <[email protected]>
>> ---
>>
>> - 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;
> }
>

Thanks for the suggestions. They all seem reasonable to me.

Arvind, since I was originally just resending your patch, do you want
to make the changes Johannes suggests, or should I proceed with that?

josh