[PATCH] gdth buggy page mapping

[Jens Axboe <axboe@suse.de>]

Hi,

Just tripped over a bug report for the SUSE kernel where gdth would
crash on a 32G opteron, turned out that the gdth_internal_copy() sg
handling was really buggy. After fixing this I wanted to do the same for
mainline, but I can see that a vain attempt was already made to fix it.
Unfortunately it wasn't complete, and on top of that there's room for
improvement.

The current code is buggy on highmem, as page_address() will not yield a
valid kernel address causing a NULL pointer dereference. The current
code also doesn't unmap the sg list if it sees a NULL sl->page. In fact,
the whole sg mapping looks really strange, why on earth would you be
mapping the sglist for dma when you are only going to copy from it?

This patch corrects both errors - correctly maps in the page, and kills
the pci_map_sg/pci_unmap_sg calls completely. If someone could test
this, that would be great.

Signed-off-by: Jens Axboe <axboe@suse.de>

===== drivers/scsi/gdth.c 1.44 vs edited =====
--- 1.44/drivers/scsi/gdth.c	2004-10-01 04:44:10 +02:00
+++ edited/drivers/scsi/gdth.c	2004-11-23 19:05:51 +01:00
@@ -2708,7 +2708,6 @@
     ushort cpsum,cpnow;
     struct scatterlist *sl;
     gdth_ha_str *ha;
-    int sgcnt;
     char *address;
 
     cpcount = count<=(ushort)scp->bufflen ? count:(ushort)scp->bufflen;
@@ -2717,8 +2716,8 @@
     if (scp->use_sg) {
         sl = (struct scatterlist *)scp->request_buffer;
 #if LINUX_VERSION_CODE >= KERNEL_VERSION(2,4,13)
-        sgcnt = pci_map_sg(ha->pdev,sl,scp->use_sg,PCI_DMA_FROMDEVICE);
-        for (i=0,cpsum=0; i<sgcnt; ++i,++sl) {
+        for (i=0,cpsum=0; i<scp->use_sg; ++i,++sl) {
+	    unsigned long flags;
             cpnow = (ushort)sg_dma_len(sl);
             TRACE(("copy_internal() now %d sum %d count %d %d\n",
                           cpnow,cpsum,cpcount,(ushort)scp->bufflen));
@@ -2730,17 +2729,18 @@
                        hanum);
                 return;
             }
-            address = (char *)(page_address(sl->page) + sl->offset);
+	    local_irq_save(flags);
+	    address = kmap_atomic(sl->page, KM_BIO_SRC_IRQ) + sl->offset;
             memcpy(address,buffer,cpnow);
+	    flush_dcache_page(sl->page);
+	    kunmap_atomic(address, KM_BIO_SRC_IRQ);
+	    local_irq_restore(flags);
             if (cpsum == cpcount)
                 break;
             buffer += cpnow;
         }
-        pci_unmap_sg(ha->pdev,scp->request_buffer,
-                     scp->use_sg,PCI_DMA_FROMDEVICE);
 #else
-        sgcnt = scp->use_sg;
-        for (i=0,cpsum=0; i<sgcnt; ++i,++sl) {
+        for (i=0,cpsum=0; i<scp->use_sg; ++i,++sl) {
             cpnow = (ushort)sl->length;
             TRACE(("copy_internal() now %d sum %d count %d %d\n",
                           cpnow,cpsum,cpcount,(ushort)scp->bufflen));

-- 
Jens Axboe