linux-scsi.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCHSET] block: fix PIO cache coherency bug, take 2
@ 2006-06-04  3:41 Tejun Heo
  2006-06-04  3:41 ` [PATCH 1/5] arm: implement flush_kernel_dcache_page() Tejun Heo
                   ` (5 more replies)
  0 siblings, 6 replies; 28+ messages in thread
From: Tejun Heo @ 2006-06-04  3:41 UTC (permalink / raw)
  To: Jens Axboe, James Bottomley, Dave Miller, bzolnier, james.steward,
	jgarzik, mattjreimer, Guennadi Liakhovetski, rmk, lkml, linux-ide,
	linux-scsi, htejun

Hello, all.

Here's another round of block PIO cache coherency fix patchset.  The
previous try[1] was rejected because flush_dcache_page() was excessive
and couldn't be called from irq context.  A new cachetlb interface has
been introduced - flush_kernel_dcache_page(), which is only
responsible for flushing the kernel mapping and safe to call from irq
context.  The function is implemented only for parisc.  This patchset
adds implementation for arm.

blk kmap wrappers have been dropped and calls to
flush_kernel_dcache_page() have been directly added.  Because
flush_kernel_dcache_page() hasn't been implemented on many
architectures, converting to such wrappers breaks cache coherency for
such architectures.  kmap should be updated after all archtectures
with aliasing caches implement flush_kernel_dcache_page().

Russell, can you please verify arm's flush_kernel_dcache_page()?  I
tried to implement flush_anon_page() too but didn't know what to do
with anon_vma object.  It seems that a call to
__cpuc_flush_user_range() should do the job but it requires
vma->vm_flags to see whether it's an executable page.  To access vma
from anon mapped page, page->mapping:anon_vma->lock should be grabbed
and probably the first vma on the list can be used, which is kind of
complex.  I think the options here are...

* adding vma argument to flush_anon_page()
* always flush for the worst vm_flags

I have only compile tested.  Please verify this fixes the coherency
problem on arm.

Jens, if everyone is happy with this, can you push this patchset
through blk tree?  As this change only adds calls to
flush_kernel_dcache_page() which is currently implement only on parisc
and arm, I think including this fix into 2.6.17 shouldn't cause too
much trouble.

Thanks.

--
tejun

[1] http://article.gmane.org/gmane.linux.kernel/367509



^ permalink raw reply	[flat|nested] 28+ messages in thread
* Re: [PATCH 4/5] SCSI: add cpu cache flushes after kmapping and modifying a page
@ 2007-05-29 16:53 Salyzyn, Mark
  0 siblings, 0 replies; 28+ messages in thread
From: Salyzyn, Mark @ 2007-05-29 16:53 UTC (permalink / raw)
  To: James Bottomley, Tejun Heo; +Cc: linux-scsi

[-- Attachment #1: Type: text/plain, Size: 6557 bytes --]

What ever became of the following patch? I have enclosed the incremental
aacraid version of this patch to permit closure if the following was
rejected because of another portion.

This attached aacraid specific portion of the patch is against current
scsi-misc-2.6.

ObligatoryDisclaimer: Please accept my condolences regarding Outlook's
handling of patch content.

Signed-off-by: Mark Salyzyn <aacraid@adaptec.com>

Sincerely -- Mark Salyzyn

-----Original Message-----
From: linux-scsi-owner@vger.kernel.org
[mailto:linux-scsi-owner@vger.kernel.org] On Behalf Of Tejun Heo
Sent: Saturday, June 03, 2006 11:41 PM
To: Jens Axboe; James Bottomley; Dave Miller; bzolnier@gmail.com;
james.steward@dynamicratings.com; jgarzik@pobox.com;
mattjreimer@gmail.com; Guennadi Liakhovetski; rmk@arm.linux.org.uk;
lkml; linux-ide@vger.kernel.org; linux-scsi@vger.kernel.org;
htejun@gmail.com
Cc: Tejun Heo
Subject: [PATCH 4/5] SCSI: add cpu cache flushes after kmapping and
modifying a page


Add calls to flush_kernel_dcache_page() after CPU has kmapped and
modified a page.  This fixes PIO cache coherency bugs on architectures
with aliased caches.

Signed-off-by: Tejun Heo <htejun@gmail.com>

---

 drivers/scsi/3w-9xxx.c        |    1 +
 drivers/scsi/3w-xxxx.c        |    1 +
 drivers/scsi/aacraid/aachba.c |    4 +++-
 drivers/scsi/ide-scsi.c       |    1 +
 drivers/scsi/ips.c            |    2 ++
 drivers/scsi/iscsi_tcp.c      |    1 +
 drivers/scsi/megaraid.c       |    2 ++
 drivers/scsi/qlogicpti.c      |    1 +
 drivers/scsi/scsi_debug.c     |    1 +
 drivers/scsi/scsi_lib.c       |    1 +
 10 files changed, 14 insertions(+), 1 deletions(-)

9b4bdd1409efb726d4a6561a4f7e2aff878ab4f4
diff --git a/drivers/scsi/3w-9xxx.c b/drivers/scsi/3w-9xxx.c
index caeb6d2..172f16b 100644
--- a/drivers/scsi/3w-9xxx.c
+++ b/drivers/scsi/3w-9xxx.c
@@ -1948,6 +1948,7 @@ static void twa_scsiop_execute_scsi_comp
 			local_irq_save(flags);
 			buf = kmap_atomic(sg->page, KM_IRQ0) +
sg->offset;
 			memcpy(buf,
tw_dev->generic_buffer_virt[request_id], sg->length);
+			flush_kernel_dcache_page(kmap_atomic_to_page(buf
- sg->offset));
 			kunmap_atomic(buf - sg->offset, KM_IRQ0);
 			local_irq_restore(flags);
 		}
diff --git a/drivers/scsi/3w-xxxx.c b/drivers/scsi/3w-xxxx.c
index e8e41e6..8449551 100644
--- a/drivers/scsi/3w-xxxx.c
+++ b/drivers/scsi/3w-xxxx.c
@@ -1527,6 +1527,7 @@ static void tw_transfer_internal(TW_Devi
 		struct scatterlist *sg;
 
 		sg = (struct scatterlist *)cmd->request_buffer;
+		flush_kernel_dcache_page(kmap_atomic_to_page(buf -
sg->offset));
 		kunmap_atomic(buf - sg->offset, KM_IRQ0);
 		local_irq_restore(flags);
 	}
diff --git a/drivers/scsi/aacraid/aachba.c
b/drivers/scsi/aacraid/aachba.c
index 642a3b4..b7c00b8 100644
--- a/drivers/scsi/aacraid/aachba.c
+++ b/drivers/scsi/aacraid/aachba.c
@@ -376,8 +376,10 @@ static void aac_internal_transfer(struct
 
 	memcpy(buf + offset, data, transfer_len - offset);
 
-	if (scsicmd->use_sg) 
+	if (scsicmd->use_sg) {
+		flush_kernel_dcache_page(kmap_atomic_to_page(buf -
sg->offset));
 		kunmap_atomic(buf - sg->offset, KM_IRQ0);
+	}
 
 }
 
diff --git a/drivers/scsi/ide-scsi.c b/drivers/scsi/ide-scsi.c
index 39b760a..9c28b95 100644
--- a/drivers/scsi/ide-scsi.c
+++ b/drivers/scsi/ide-scsi.c
@@ -189,6 +189,7 @@ static void idescsi_input_buffers (ide_d
 					pc->sg->offset;
 			drive->hwif->atapi_input_bytes(drive,
 						buf + pc->b_count,
count);
+			flush_kernel_dcache_page(kmap_atomic_to_page(buf
- pc->sg->offset));
 			kunmap_atomic(buf - pc->sg->offset, KM_IRQ0);
 			local_irq_restore(flags);
 		} else {
diff --git a/drivers/scsi/ips.c b/drivers/scsi/ips.c
index a4c0b04..29eb3f0 100644
--- a/drivers/scsi/ips.c
+++ b/drivers/scsi/ips.c
@@ -3682,6 +3682,8 @@ ips_scmd_buf_write(Scsi_Cmnd * scmd, voi
 			local_irq_save(flags);
 			buffer = kmap_atomic(sg[i].page, KM_IRQ0) +
sg[i].offset;
 			memcpy(buffer, &cdata[xfer_cnt], min_cnt);
+                        flush_kernel_dcache_page(
+                                kmap_atomic_to_page(buffer -
sg[i].offset));
 			kunmap_atomic(buffer - sg[i].offset, KM_IRQ0);
 			local_irq_restore(flags);
 
diff --git a/drivers/scsi/iscsi_tcp.c b/drivers/scsi/iscsi_tcp.c
index 2068b66..ae9784c 100644
--- a/drivers/scsi/iscsi_tcp.c
+++ b/drivers/scsi/iscsi_tcp.c
@@ -945,6 +945,7 @@ static int iscsi_scsi_data_in(struct isc
 		dest = kmap_atomic(sg[i].page, KM_SOFTIRQ0);
 		rc = iscsi_ctask_copy(conn, ctask, dest + sg[i].offset,
 				      sg[i].length, offset);
+		flush_kernel_dcache_page(kmap_atomic_to_page(dest));
 		kunmap_atomic(dest, KM_SOFTIRQ0);
 		if (rc == -EAGAIN)
 			/* continue with the next SKB/PDU */
diff --git a/drivers/scsi/megaraid.c b/drivers/scsi/megaraid.c
index de35ffe..7cb7590 100644
--- a/drivers/scsi/megaraid.c
+++ b/drivers/scsi/megaraid.c
@@ -671,6 +671,8 @@ #endif
 				struct scatterlist *sg;
 
 				sg = (struct scatterlist
*)cmd->request_buffer;
+				flush_kernel_dcache_page(
+					kmap_atomic_to_page(buf -
sg->offset));
 				kunmap_atomic(buf - sg->offset,
KM_IRQ0);
 			}
 			cmd->result = (DID_OK << 16);
diff --git a/drivers/scsi/qlogicpti.c b/drivers/scsi/qlogicpti.c
index c7e78dc..f8201f2 100644
--- a/drivers/scsi/qlogicpti.c
+++ b/drivers/scsi/qlogicpti.c
@@ -1146,6 +1146,7 @@ static void scsi_rbuf_put(struct scsi_cm
 		struct scatterlist *sg;
 
 		sg = (struct scatterlist *) cmd->request_buffer;
+		flush_kernel_dcache_page(kmap_atomic_to_page(buf -
sg->offset));
 		kunmap_atomic(buf - sg->offset, KM_IRQ0);
 	}
 }
diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
index 5a5d2af..88543db 100644
--- a/drivers/scsi/scsi_debug.c
+++ b/drivers/scsi/scsi_debug.c
@@ -511,6 +511,7 @@ static int fill_from_dev_buffer(struct s
 				len = arr_len - req_len;
 			}
 			memcpy(kaddr_off, arr + req_len, len);
+
flush_kernel_dcache_page(kmap_atomic_to_page(kaddr));
 			kunmap_atomic(kaddr, KM_USER0);
 			act_len += len;
 		}
diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index 764a8b3..8bb2f6c 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -945,6 +945,7 @@ void scsi_io_completion(struct scsi_cmnd
 			unsigned long flags;
 			char *to = bio_kmap_irq(req->bio, &flags);
 			memcpy(to, cmd->buffer, cmd->bufflen);
+
flush_kernel_dcache_page(kmap_atomic_to_page(to));
 			bio_kunmap_irq(to, &flags);
 		}
 		kfree(cmd->buffer);
-- 
1.3.2

[-- Attachment #2: aacraid_flush_dcache.patch --]
[-- Type: application/octet-stream, Size: 762 bytes --]

diff -ru a/drivers/scsi/aacraid/aachba.c b/drivers/scsi/aacraid/aachba.c
--- a/drivers/scsi/aacraid/aachba.c	2007-05-29 11:57:47.332437519 -0400
+++ b/drivers/scsi/aacraid/aachba.c	2007-05-29 12:37:04.581386639 -0400
@@ -31,9 +31,9 @@
 #include <linux/slab.h>
 #include <linux/completion.h>
 #include <linux/blkdev.h>
-#include <linux/dma-mapping.h>
 #include <asm/semaphore.h>
 #include <asm/uaccess.h>
+#include <linux/highmem.h> /* For flush_kernel_dcache_page */
 
 #include <scsi/scsi.h>
 #include <scsi/scsi_cmnd.h>
@@ -355,7 +355,9 @@
 		memcpy(buf + offset, data, transfer_len);
 
-	if (scsicmd->use_sg) 
+	if (scsicmd->use_sg) {
+		flush_kernel_dcache_page(kmap_atomic_to_page(buf - sg->offset));
 		kunmap_atomic(buf - sg->offset, KM_IRQ0);
+	}
 
 }
 

^ permalink raw reply related	[flat|nested] 28+ messages in thread

end of thread, other threads:[~2007-05-29 16:53 UTC | newest]

Thread overview: 28+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2006-06-04  3:41 [PATCHSET] block: fix PIO cache coherency bug, take 2 Tejun Heo
2006-06-04  3:41 ` [PATCH 1/5] arm: implement flush_kernel_dcache_page() Tejun Heo
2006-06-04  3:49   ` [PATCH 1/5] (REPOST) " Tejun Heo
2006-06-04  6:45   ` [PATCH 1/5] " David Miller
2006-06-04  6:53     ` Tejun Heo
2006-06-04  7:04       ` David Miller
2006-06-04  3:41 ` [PATCH 5/5] md: add cpu cache flushes after kmapping and modifying a page Tejun Heo
2006-06-04  3:41 ` [PATCH 4/5] SCSI: " Tejun Heo
2006-06-04  8:20   ` Christoph Hellwig
2006-06-04  9:13     ` Tejun Heo
2006-06-04 20:24       ` Guennadi Liakhovetski
2006-06-04  3:41 ` [PATCH 2/5] ide: " Tejun Heo
2006-06-04  8:17   ` Christoph Hellwig
2006-06-04  9:09     ` Tejun Heo
2006-06-04  3:41 ` [PATCH 3/5] libata: " Tejun Heo
2006-06-04 20:44 ` [PATCHSET] block: fix PIO cache coherency bug, take 2 Russell King
2006-06-04 22:23   ` Russell King
2006-06-05 14:27     ` James Bottomley
2006-06-05 14:44       ` Russell King
2006-06-05 15:24         ` James Bottomley
2006-06-05 15:34           ` Russell King
2006-06-05 15:47             ` James Bottomley
2006-06-05 15:48               ` Russell King
2006-06-05 16:16                 ` James Bottomley
2006-06-05 16:37                   ` Russell King
2006-06-05 13:43   ` James Bottomley
2006-06-06 11:00     ` Miklos Szeredi
  -- strict thread matches above, loose matches on Subject: below --
2007-05-29 16:53 [PATCH 4/5] SCSI: add cpu cache flushes after kmapping and modifying a page Salyzyn, Mark

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).