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 #define MSGOUT_OFFSET (L1_CACHE_ALIGN(sizeof(SCRIPT))) __u8 *msgout; #define MSGIN_OFFSET (MSGOUT_OFFSET + L1_CACHE_ALIGN(MSG_ARRAY_SIZE)) __u8 *msgin; #define STATUS_OFFSET (MSGIN_OFFSET + L1_CACHE_ALIGN(MSG_ARRAY_SIZE)) __u8 *status; #define SLOTS_OFFSET (STATUS_OFFSET + L1_CACHE_ALIGN(MSG_ARRAY_SIZE)) 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-arch" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html