[lkml]   [2010]   [Jun]   [27]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
Messages in this thread
    SubjectRe: [PATCH -mm 1/2] scsi: remove dma_is_consistent usage in 53c700
    On Sun, 27 Jun 2010 10:08:48 -0500
    James Bottomley <> 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;
    __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?

     \ /
      Last update: 2010-06-28 05:41    [W:0.029 / U:3.128 seconds]
    ©2003-2017 Jasper Spaans. hosted at Digital OceanAdvertise on this site