Re: [PATCH] megaraid_mbox: remove bogus use of pci_dma_sync_sg_* APIs

2018-10-15 Thread Martin K. Petersen


Christoph,

> The dma_map_sg / dma_unmap_sg APIs called from scsi_dma_map /
> scsi_dma_unmap already transfer memory ownership to the device or cpu
> respectively.  Adding additional calls to pci_dma_sync_sg_* will in
> fact lead to data corruption if we end up using swiotlb for some
> reason.
>
> Also remove the now pointless megaraid_mbox_sync_scb function.

Applied to 4.20/scsi-queue, thanks!

-- 
Martin K. Petersen  Oracle Linux Engineering


Re: [PATCH] megaraid_mbox: remove bogus use of pci_dma_sync_sg_* APIs

2018-10-11 Thread Johannes Thumshirn
Looks good,
Reviewed-by: Johannes Thumshirn 
-- 
Johannes ThumshirnSUSE Labs 
jthumsh...@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


[PATCH] megaraid_mbox: remove bogus use of pci_dma_sync_sg_* APIs

2018-10-11 Thread Christoph Hellwig
The dma_map_sg / dma_unmap_sg APIs called from scsi_dma_map /
scsi_dma_unmap already transfer memory ownership to the device or
cpu respectively.  Adding additional calls to pci_dma_sync_sg_*
will in fact lead to data corruption if we end up using swiotlb
for some reason.

Also remove the now pointless megaraid_mbox_sync_scb function.

Signed-off-by: Christoph Hellwig 
---
 drivers/scsi/megaraid/megaraid_mbox.c | 35 +--
 1 file changed, 1 insertion(+), 34 deletions(-)

diff --git a/drivers/scsi/megaraid/megaraid_mbox.c 
b/drivers/scsi/megaraid/megaraid_mbox.c
index 2013523605c5..89c85a5a47af 100644
--- a/drivers/scsi/megaraid/megaraid_mbox.c
+++ b/drivers/scsi/megaraid/megaraid_mbox.c
@@ -1428,12 +1428,6 @@ mbox_post_cmd(adapter_t *adapter, scb_t *scb)
 
adapter->outstanding_cmds++;
 
-   if (scb->dma_direction == PCI_DMA_TODEVICE)
-   pci_dma_sync_sg_for_device(adapter->pdev,
-  scsi_sglist(scb->scp),
-  scsi_sg_count(scb->scp),
-  PCI_DMA_TODEVICE);
-
mbox->busy  = 1;// Set busy
mbox->poll  = 0;
mbox->ack   = 0;
@@ -2180,31 +2174,6 @@ megaraid_isr(int irq, void *devp)
 }
 
 
-/**
- * megaraid_mbox_sync_scb - sync kernel buffers
- * @adapter: controller's soft state
- * @scb: pointer to the resource packet
- *
- * DMA sync if required.
- */
-static void
-megaraid_mbox_sync_scb(adapter_t *adapter, scb_t *scb)
-{
-   mbox_ccb_t  *ccb;
-
-   ccb = (mbox_ccb_t *)scb->ccb;
-
-   if (scb->dma_direction == PCI_DMA_FROMDEVICE)
-   pci_dma_sync_sg_for_cpu(adapter->pdev,
-   scsi_sglist(scb->scp),
-   scsi_sg_count(scb->scp),
-   PCI_DMA_FROMDEVICE);
-
-   scsi_dma_unmap(scb->scp);
-   return;
-}
-
-
 /**
  * megaraid_mbox_dpc - the tasklet to complete the commands from completed list
  * @devp   : pointer to HBA soft state
@@ -2403,9 +2372,7 @@ megaraid_mbox_dpc(unsigned long devp)
megaraid_mbox_display_scb(adapter, scb);
}
 
-   // Free our internal resources and call the mid-layer callback
-   // routine
-   megaraid_mbox_sync_scb(adapter, scb);
+   scsi_dma_unmap(scp);
 
// remove from local clist
list_del_init(>list);
-- 
2.19.1