Re: [PATCH -mm 1/2] scsi: remove dma_is_consistent usage in 53c700

From: FUJITA Tomonori
Date: Sun Jun 27 2010 - 23:38:46 EST

On Sun, 27 Jun 2010 10:08:48 -0500
James Bottomley <James.Bottomley@xxxxxxxxxxxxxxxxxxxxx> wrote:

> On Sun, 2010-06-27 at 19:10 +0900, FUJITA Tomonori wrote:
> > 53c700 is the only user of dma_is_consistent():
> >
> > BUG_ON(!dma_is_consistent(hostdata->dev, pScript) && L1_CACHE_BYTES < dma_get_cache_alignment());
> >
> > The above code tries to see if the system can allocate coherent memory
> > or not. It's for some old systems that can't allocate coherent memory
> > at all (e.g some parisc systems).
> Actually, that's not the right explanation. The BUG_ON is because of an
> efficiency in the driver ... it's nothing to do with the architecture.
> The driver uses a set of mailboxes, but for efficiency's sake, it packs
> them into a single coherent area and separates the different usages by a
> L1 cache stride). On architectures capable of manufacturing coherent
> memory, this is a nice speed up in the DMA infrastructure. However, for
> incoherent architectures, it's fatal if the dma coherence stride is
> greater than the L1 cache size, because now we'll get data corruption
> due to cacheline interference. That's what the BUG_ON is checking for.

Sorry, I should have looked the details of the driver.

You are talking about the following tricks, right?

#define MSG_ARRAY_SIZE 8
__u8 *msgout;
__u8 *msgin;
__u8 *status;
struct NCR_700_command_slot *slots;
#define TOTAL_MEM_SIZE (SLOTS_OFFSET + L1_CACHE_ALIGN(sizeof(struct NCR_700_command_slot) * NCR_700_COMMAND_SLOTS_PER_HOST))

> > I think that we can safely remove the above usage:
> >
> > - such old systems haven't triger the above checking for long.
> >
> > - the above condition is important for systems that can't allocate
> > coherent memory if these systems do DMA. So probably it would be
> > better to have such checking in arch's DMA initialization code
> > instead of a driver.
> Well, we can't check in the architecture because it's a driver specific
> thing ... I suppose making it a rule that dma_get_cache_alignment()
> *must* be <= L1_CACHE_BYTES fixes it ... we seem to have no architecture
> violating that, so just add it to the documentation, and the check can
> go.

Seems that on some architectures (arm and mips at least),
dma_get_cache_alignment() could greater than L1_CACHE_BYTES. But they
simply return the possible maximum size of cache size like:

static inline int dma_get_cache_alignment(void)
/* XXX Largest on any MIPS */
return 128;

So practically, we should be safe. I guess that we can simply convert
them to return L1_CACHE_BYTES.

Some PARISC and mips are only the fully non-coherent architectures
that we support now? We can remove the above checking if
dma_get_cache_alignment() is <= L1_CACHE_BYTES on PARISC and mips?
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at
Please read the FAQ at