[PATCH] fix wrong argument to pci_dma_sync_sg in megaraid driver

[PATCH] fix wrong argument to pci_dma_sync_sg in megaraid driver

[Christoph Hellwig]

sgl is incremented for each element in the routine, but pci_dma_sync_sg
needs the original buffer.


--- 1.59/drivers/scsi/megaraid.c	Fri Jan 23 06:37:03 2004
+++ edited/drivers/scsi/megaraid.c	Sat Jan 31 07:09:25 2004
@@ -1805,7 +1805,7 @@
 	*len = (u32)cmd->request_bufflen;
 
 	if( scb->dma_direction == PCI_DMA_TODEVICE ) {
-		pci_dma_sync_sg(adapter->dev, sgl, cmd->use_sg,
+		pci_dma_sync_sg(adapter->dev, cmd->request_buffer, cmd->use_sg,
 				PCI_DMA_TODEVICE);
 	}
 

Re: [PATCH] fix wrong argument to pci_dma_sync_sg in megaraid driver

[James Bottomley]

On Sun, 2004-02-01 at 11:00, Christoph Hellwig wrote:
> sgl is incremented for each element in the routine, but pci_dma_sync_sg
> needs the original buffer.
> 
> 
> --- 1.59/drivers/scsi/megaraid.c	Fri Jan 23 06:37:03 2004
> +++ edited/drivers/scsi/megaraid.c	Sat Jan 31 07:09:25 2004
> @@ -1805,7 +1805,7 @@
>  	*len = (u32)cmd->request_bufflen;
>  
>  	if( scb->dma_direction == PCI_DMA_TODEVICE ) {
> -		pci_dma_sync_sg(adapter->dev, sgl, cmd->use_sg,
> +		pci_dma_sync_sg(adapter->dev, cmd->request_buffer, cmd->use_sg,
>  				PCI_DMA_TODEVICE);
>  	}

Hang on a minute, none of these syncs (including all the other syncs to
and from the device) looks to be necessary.

Once the buffers are mapped, their cache line ownership is transferred
automatically to the device.  Once they are unmapped, it is transferred
back.  There's no need to do additional syncs after mapping and
unmapping *unless* the driver needs to touch the actual data in between
the mapping and the unmapping.  I didn't notice the megaraid actually
doing this (but even if it does, the sync would be in the wrong place).

James