[PATCH 2/8] ARM: Implement read/write for ownership in theARMv6 DMA cache ops

[Catalin Marinas]

On Thu, 2010-05-13 at 21:34 +0100, George G. Davis wrote:
> On Thu, May 13, 2010 at 03:15:20PM +0100, Catalin Marinas wrote:
> > On Thu, 2010-05-13 at 14:54 +0100, George G. Davis wrote:
> > > On Thu, May 13, 2010 at 09:26:57AM +0100, Catalin Marinas wrote:
> > > > On Thu, 2010-05-13 at 06:27 +0100, Ronen Shitrit wrote:
> > > > > Our ARM v6 does have speculative prefetch...
> > > >
> > > > That's the I-cache and we should be ok with it. What may affect this
> > > > patch is speculative fetches into the D-cache.
> > >
> > > prefetch()?
> >
> > Drivers should not use prefetch (PLD) on memory handed over to a device
> > for DMA transfers. To me this is like explicitly accessing such memory
> > during a DMA transfer.
> 
> I agree and was merely speculating that prefetch() may be a case where
> D-Cache lines are explicitly prefetched and may be the cause of this
> problem, e.g. from e1000_clean_rx_irq() in drivers/net/e1000e/netdev.c:
> 
>         while (rx_desc->status & E1000_RXD_STAT_DD) {
>                 struct sk_buff *skb;
>                 u8 status;
> 
>                 if (*work_done >= work_to_do)
>                         break;
>                 (*work_done)++;
> 
>                 status = rx_desc->status;
>                 skb = buffer_info->skb;
>                 buffer_info->skb = NULL;
> 
>                 prefetch(skb->data - NET_IP_ALIGN);
> 
>                 i++;
>                 if (i == rx_ring->count)
>                         i = 0;
>                 next_rxd = E1000_RX_DESC(*rx_ring, i);
>                 prefetch(next_rxd);
> 
>                 next_buffer = &rx_ring->buffer_info[i];
> 
>                 cleaned = 1;
>                 cleaned_count++;
>                 pci_unmap_single(pdev,
>                                  buffer_info->dma,
>                                  adapter->rx_buffer_len,
>                                  PCI_DMA_FROMDEVICE);

If the prefetch is done after the DMA transfer has completed, then there
shouldn't be any problem. I suspect that's the case in this interrupt
routine.

What I think I missed in my patch (and didn't show up with a sata drive)
is the "sync" operations. The dma_sync_single_for_device(FROMDEVICE)
corrupts the existing buffer.

I'll post an updated patch shortly.

-- 
Catalin