Re: [PATCH v2 02/31] cinergyT2-core: don't do DMA on stack

[Mauro Carvalho Chehab]

Em Sat, 15 Oct 2016 22:54:49 +0200
Johannes Stezenbach <js@xxxxxxxxxxx> escreveu:

> On Tue, Oct 11, 2016 at 07:09:17AM -0300, Mauro Carvalho Chehab wrote:
> > --- a/drivers/media/usb/dvb-usb/cinergyT2-core.c
> > +++ b/drivers/media/usb/dvb-usb/cinergyT2-core.c
> > @@ -41,6 +41,8 @@ DVB_DEFINE_MOD_OPT_ADAPTER_NR(adapter_nr);
> >  
> >  struct cinergyt2_state {
> >  	u8 rc_counter;
> > +	unsigned char data[64];
> > +	struct mutex data_mutex;
> >  };  
> 
> Sometimes my thinking is slow but it just occured to me
> that this creates a potential issue with cache line sharing.
> On an architecture which manages cache coherence in software
> (ARM, MIPS etc.) a write to e.g. rc_counter in this example
> would dirty the cache line, and a later writeback from the
> cache could overwrite parts of data[] which was received via DMA.
> In contrast, if the DMA buffer is allocated seperately via
> kmalloc it is guaranteed to be safe wrt cache line sharing.
> (see bottom of Documentation/DMA-API-HOWTO.txt).
> 

Interesting point. I'm not sure well this would work with non-fully
coherent cache lines. I guess that will depend on how the USB
driver will be handling it.

> But of course DMA on stack also had the same issue
> and no one ever noticed so it's apparently not critical...

Yes, this shouldn't do it any worse than what we currently have.
In the past, I tested some drivers that uses a shared buffed for control
URB transfers in the past, on arm32 and arm64. I don't remember seeing
anything weird there that could be related to cache coherency, although
I remember several problems with USB on OMAP and RPi version 1, leading
troubles after several minutes of ISOC transfers on analog TV,
but they seemed to be unrelated to URB control traffic.

I'd say that we should keep our eyes on those drivers, after applying
this patch series and see if people will notice bad behavior on non-x86
archs.

Thanks,
Mauro