[PATCH 3/5] aacraid: update dma mapping

[PATCH 3/5] aacraid: update dma mapping

[Mark Haverkamp]

Received from Mark Salyzyn.

This patch changes map_single to map_page in compliance with
recommendations made in the IO-Perf howto document to prevent bounce
buffering of DMA requests.

Signed-off-by: Mark Haverkamp <markh@osdl.org>
---
Applies to the scsi-misc-2.6 git tree.


Index: scsi-misc-aac/drivers/scsi/aacraid/aachba.c
===================================================================
--- scsi-misc-aac.orig/drivers/scsi/aacraid/aachba.c	2005-11-28 15:15:02.000000000 -0800
+++ scsi-misc-aac/drivers/scsi/aacraid/aachba.c	2005-11-28 15:15:13.000000000 -0800
@@ -955,9 +955,9 @@
 			scsicmd->use_sg,
 			scsicmd->sc_data_direction);
 	else if(scsicmd->request_bufflen)
-		pci_unmap_single(dev->pdev, scsicmd->SCp.dma_handle,
-				 scsicmd->request_bufflen,
-				 scsicmd->sc_data_direction);
+		pci_unmap_page(dev->pdev, scsicmd->SCp.dma_handle,
+			 scsicmd->request_bufflen,
+			 scsicmd->sc_data_direction);
 	readreply = (struct aac_read_reply *)fib_data(fibptr);
 	if (le32_to_cpu(readreply->status) == ST_OK)
 		scsicmd->result = DID_OK << 16 | COMMAND_COMPLETE << 8 | SAM_STAT_GOOD;
@@ -1936,7 +1936,8 @@
 			scsicmd->use_sg,
 			scsicmd->sc_data_direction);
 	else if(scsicmd->request_bufflen)
-		pci_unmap_single(dev->pdev, scsicmd->SCp.dma_handle, scsicmd->request_bufflen,
+		pci_unmap_page(dev->pdev, scsicmd->SCp.dma_handle,
+			scsicmd->request_bufflen,
 			scsicmd->sc_data_direction);
 
 	/*
@@ -2239,15 +2240,16 @@
 		}
 	}
 	else if(scsicmd->request_bufflen) {
-		dma_addr_t addr; 
-		addr = pci_map_single(dev->pdev,
-				scsicmd->request_buffer,
+		u32 addr;
+		scsicmd->SCp.dma_handle = pci_map_page(dev->pdev,
+				virt_to_page(scsicmd->request_buffer),
+				offset_in_page(scsicmd->request_buffer),
 				scsicmd->request_bufflen,
 				scsicmd->sc_data_direction);
+		addr = scsicmd->SCp.dma_handle;
 		psg->count = cpu_to_le32(1);
 		psg->sg[0].addr = cpu_to_le32(addr);
 		psg->sg[0].count = cpu_to_le32(scsicmd->request_bufflen);  
-		scsicmd->SCp.dma_handle = addr;
 		byte_count = scsicmd->request_bufflen;
 	}
 	return byte_count;
@@ -2300,16 +2302,17 @@
 		}
 	}
 	else if(scsicmd->request_bufflen) {
-		u64 addr; 
-		addr = pci_map_single(dev->pdev,
-				scsicmd->request_buffer,
+		u64 addr;
+		scsicmd->SCp.dma_handle = pci_map_page(dev->pdev,
+				virt_to_page(scsicmd->request_buffer),
+				offset_in_page(scsicmd->request_buffer),
 				scsicmd->request_bufflen,
 				scsicmd->sc_data_direction);
+		addr = scsicmd->SCp.dma_handle;
 		psg->count = cpu_to_le32(1);
 		psg->sg[0].addr[0] = cpu_to_le32(addr & 0xffffffff);
 		psg->sg[0].addr[1] = cpu_to_le32(addr >> 32);
 		psg->sg[0].count = cpu_to_le32(scsicmd->request_bufflen);  
-		scsicmd->SCp.dma_handle = addr;
 		byte_count = scsicmd->request_bufflen;
 	}
 	return byte_count;
@@ -2367,8 +2370,9 @@
 	else if(scsicmd->request_bufflen) {
 		int count;
 		u64 addr;
-		scsicmd->SCp.dma_handle = pci_map_single(dev->pdev,
-				scsicmd->request_buffer,
+		scsicmd->SCp.dma_handle = pci_map_page(dev->pdev,
+				virt_to_page(scsicmd->request_buffer),
+				offset_in_page(scsicmd->request_buffer),
 				scsicmd->request_bufflen,
 				scsicmd->sc_data_direction);
 		addr = scsicmd->SCp.dma_handle;
Index: scsi-misc-aac/drivers/scsi/aacraid/commctrl.c
===================================================================
--- scsi-misc-aac.orig/drivers/scsi/aacraid/commctrl.c	2005-11-28 15:15:09.000000000 -0800
+++ scsi-misc-aac/drivers/scsi/aacraid/commctrl.c	2005-11-28 15:15:13.000000000 -0800
@@ -590,7 +590,9 @@
 					goto cleanup;
 				}
 			}
-			addr = pci_map_single(dev->pdev, p, usg->sg[i].count, data_dir);
+			addr = pci_map_page(dev->pdev, virt_to_page(p),
+			  	offset_in_page(p),
+				usg->sg[i].count, data_dir);
 
 			psg->sg[i].addr[0] = cpu_to_le32(addr & 0xffffffff);
 			psg->sg[i].addr[1] = cpu_to_le32(addr>>32);
@@ -640,7 +642,8 @@
 					goto cleanup;
 				}
 			}
-			addr = pci_map_single(dev->pdev, p,
+			addr = pci_map_page(dev->pdev, virt_to_page(p),
+				offset_in_page(p),
 				upsg->sg[i].count, data_dir);
 
 			psg->sg[i].addr = cpu_to_le32(addr);

-- 
Mark Haverkamp <markh@osdl.org>

Re: [PATCH 3/5] aacraid: update dma mapping

[James Bottomley]

On Wed, 2005-11-30 at 10:36 -0800, Mark Haverkamp wrote:
> This patch changes map_single to map_page in compliance with
> recommendations made in the IO-Perf howto document to prevent bounce
> buffering of DMA requests.

Erm, there's a misunderstanding somewhere.  You're replacing correct
pci_map_single calls with its more difficult to use pci_map_page.  This
has absolutely zero performance impact and adds the risk that because
pci_map_page is more complex to use, someone gets it wrong somewhere.
Really, for block devices pci_map_single is the absolutely correct
interface.  pci_map_page should really only be used by the net stack for
packet fragment mapping.

James

Re: [PATCH 3/5] aacraid: update dma mapping

[Christoph Hellwig]

On Wed, Nov 30, 2005 at 12:45:27PM -0600, James Bottomley wrote:
> On Wed, 2005-11-30 at 10:36 -0800, Mark Haverkamp wrote:
> > This patch changes map_single to map_page in compliance with
> > recommendations made in the IO-Perf howto document to prevent bounce
> > buffering of DMA requests.
> 
> Erm, there's a misunderstanding somewhere.  You're replacing correct
> pci_map_single calls with its more difficult to use pci_map_page.  This
> has absolutely zero performance impact and adds the risk that because
> pci_map_page is more complex to use, someone gets it wrong somewhere.
> Really, for block devices pci_map_single is the absolutely correct
> interface.  pci_map_page should really only be used by the net stack for
> packet fragment mapping.

And the really nice thing is that this whole non-S/G I/O path will go away
completely for 2.6.16 :)

Re: [PATCH 3/5] aacraid: update dma mapping

[Mark Haverkamp]

On Wed, 2005-11-30 at 12:45 -0600, James Bottomley wrote:
> On Wed, 2005-11-30 at 10:36 -0800, Mark Haverkamp wrote:
> > This patch changes map_single to map_page in compliance with
> > recommendations made in the IO-Perf howto document to prevent bounce
> > buffering of DMA requests.
> 
> Erm, there's a misunderstanding somewhere.  You're replacing correct
> pci_map_single calls with its more difficult to use pci_map_page.  This
> has absolutely zero performance impact and adds the risk that because
> pci_map_page is more complex to use, someone gets it wrong somewhere.
> Really, for block devices pci_map_single is the absolutely correct
> interface.  pci_map_page should really only be used by the net stack for
> packet fragment mapping.

Thanks,  The remaining patches still apply cleanly without it.

Mark.


> 
> James
> 
-- 
Mark Haverkamp <markh@osdl.org>

RE: [PATCH 3/5] aacraid: update dma mapping

[Salyzyn, Mark]

Cool ... Simpler is *always* better! We got a code inspection that
indicated that memory above 4G would be accessed inefficiently using
pci_map_single. Sounds like the IO-Perf howto needs modernization.

Sincerely -- Mark Salyzyn

> -----Original Message-----
> From: James Bottomley [mailto:James.Bottomley@SteelEye.com] 
> Sent: Wednesday, November 30, 2005 1:45 PM
> To: Mark Haverkamp
> Cc: linux-scsi; Salyzyn, Mark
> Subject: Re: [PATCH 3/5] aacraid: update dma mapping
> 
> 
> On Wed, 2005-11-30 at 10:36 -0800, Mark Haverkamp wrote:
> > This patch changes map_single to map_page in compliance with
> > recommendations made in the IO-Perf howto document to prevent bounce
> > buffering of DMA requests.
> 
> Erm, there's a misunderstanding somewhere.  You're replacing correct
> pci_map_single calls with its more difficult to use 
> pci_map_page.  This
> has absolutely zero performance impact and adds the risk that because
> pci_map_page is more complex to use, someone gets it wrong somewhere.
> Really, for block devices pci_map_single is the absolutely correct
> interface.  pci_map_page should really only be used by the 
> net stack for
> packet fragment mapping.
> 
> James
> 
> 
>