2002-06-13 00:24:56

by Roland Dreier

[permalink] [raw]
Subject: [PATCH] 2.4 use __dma_buffer in scsi.h

Use __dma_buffer macro to align sense_buffer member of Scsi_Cmnd.

Patch is against 2.4.19-pre10.

Thanks,
Roland

scsi.h | 3 ++-
1 files changed, 2 insertions(+), 1 deletion(-)

diff -Naur linux-2.4.19-pre10.orig/drivers/scsi/scsi.h linux-2.4.19-pre10/drivers/scsi/scsi.h
--- linux-2.4.19-pre10.orig/drivers/scsi/scsi.h Wed Jun 12 15:08:52 2002
+++ linux-2.4.19-pre10/drivers/scsi/scsi.h Wed Jun 12 16:28:21 2002
@@ -30,6 +30,7 @@
#include <asm/hardirq.h>
#include <asm/scatterlist.h>
#include <asm/io.h>
+#include <asm/dma_buffer.h>

/*
* These are the values that the SCpnt->sc_data_direction and
@@ -779,7 +780,7 @@
struct request request; /* A copy of the command we are
working on */

- unsigned char sense_buffer[SCSI_SENSE_BUFFERSIZE]; /* obtained by REQUEST SENSE
+ unsigned char sense_buffer[SCSI_SENSE_BUFFERSIZE] __dma_buffer; /* obtained by REQUEST SENSE
* when CHECK CONDITION is
* received on original command
* (auto-sense) */


2002-06-13 02:18:10

by James Bottomley

[permalink] [raw]
Subject: Re: [PATCH] 2.4 use __dma_buffer in scsi.h

> Use __dma_buffer macro to align sense_buffer member of Scsi_Cmnd.
[...]

> - unsigned char sense_buffer[SCSI_SENSE_BUFFERSIZE]; /* obtained by REQUEST SENSE
> + unsigned char sense_buffer[SCSI_SENSE_BUFFERSIZE] __dma_buffer; /* obtained by REQUEST SENSE

This is actually unnecessary. We've already had this discussion on the parisc
mailing lists (over a year ago, I think).

The SCSI subsystem is designed with this type of cache incoherency problem in
mind. The Scsi_Cmnd structure has an ownership model to forestall cache line
bouncing. The idea is that when you want to dma to/from components of a
Scsi_Cmnd, you can only do it when the ownership is SCSI_OWNER_LOW_LEVEL,
which guarantees that nothing in the mid layer will touch the command and
trigger an incoherency (at least until it times out). The low level driver is
supposed to know about the cache incoherency problem, and so avoids touching
the structure between cache line invalidation/writeback and DMA completion,
thus, with a correctly implemented driver, there should be no possibility of
DMA corruption.

Each of the Scsi_Cmnd blocks is individually allocated with kmalloc, so
they're guaranteed to be non-interfering when it comes to DMA.

Incidentally, if you're really going to insist on padding the structure, some
drivers also use the cmnd element to DMA the command from, so that should be
aligned as well. But I think the best course of action is to fix any low
level drivers that are not following the cache rules rather than expanding
Scsi_Cmnd.

James Bottomley


2002-06-13 17:42:23

by Roland Dreier

[permalink] [raw]
Subject: Re: [PATCH] 2.4 use __dma_buffer in scsi.h

>>>>> "James" == James Bottomley <[email protected]> writes:

James> This is actually unnecessary. We've already had this
James> discussion on the parisc mailing lists (over a year ago, I
James> think).

Fair enough, the only reason I came up with this patch is that I
looked at all unaligned DMA done by USB, and
drivers/usb/storage/transport.c does DMA into sense_buffer.

James> The SCSI subsystem is designed with this type of cache
James> incoherency problem in mind. The Scsi_Cmnd structure has
James> an ownership model to forestall cache line bouncing. The
James> idea is that when you want to dma to/from components of a
James> Scsi_Cmnd, you can only do it when the ownership is
James> SCSI_OWNER_LOW_LEVEL, which guarantees that nothing in the
James> mid layer will touch the command and trigger an incoherency
James> (at least until it times out). The low level driver is
James> supposed to know about the cache incoherency problem, and
James> so avoids touching the structure between cache line
James> invalidation/writeback and DMA completion, thus, with a
James> correctly implemented driver, there should be no
James> possibility of DMA corruption.

Out of curiousity, how do you deal with someone writing to Scsi_Cmnd
_before_ the DMA and having that data lost when pci_map_single() with
PCI_DMA_FROMDEVICE invalidates the cache before writeback?

Best,
Roland

2002-06-13 17:52:46

by James Bottomley

[permalink] [raw]
Subject: Re: [PATCH] 2.4 use __dma_buffer in scsi.h

[email protected] said:
> Out of curiousity, how do you deal with someone writing to Scsi_Cmnd
> _before_ the DMA and having that data lost when pci_map_single() with
> PCI_DMA_FROMDEVICE invalidates the cache before writeback?

The the pci_map_single with PCI_DMA_FROMDEVICE is supposed to do a writeback
invalidate cycle on the cache when pci_sync_single is called (at least this is
how it is implemented on parisc) so any writes up to the sync point are
flushed to memory before the cache line is trashed.

James