Re: [PATCH] media: dvb_ringbuffer: Add memory barriers

[Soeren Moch]

On 05/07/16 15:26, Mauro Carvalho Chehab wrote:
> Em Sat, 7 May 2016 10:22:35 -0300
> Mauro Carvalho Chehab <mchehab@xxxxxxxxxxxxxxx> escreveu:
>
> > Hi Soeren,
> >
> > Em Sun, 7 Feb 2016 20:22:36 +0100
> > Soeren Moch <smoch@xxxxxx> escreveu:
> >
> > > On 27.12.2015 21:41, Soeren Moch wrote:
> > > > Implement memory barriers according to Documentation/circular-buffers.txt:
> > > > - use smp_store_release() to update ringbuffer read/write pointers
> > > > - use smp_load_acquire() to load write pointer on reader side
> > > > - use ACCESS_ONCE() to load read pointer on writer side
> > > >
> > > > This fixes data stream corruptions observed e.g. on an ARM Cortex-A9
> > > > quad core system with different types (PCI, USB) of DVB tuners.
> > > >
> > > > Signed-off-by: Soeren Moch <smoch@xxxxxx>
> > > > Cc: stable@xxxxxxxxxxxxxxx # 3.14+
> > >
> > > Mauro,
> > >
> > > any news or comments on this?
> > > Since this is a real fix for broken behaviour, can you pick this up, please?
> >
> > The problem here is that I'm very reluctant to touch at the DVB core
> > without doing some tests myself, as things like locking can be
> > very sensible.
>
> In addition, it is good if other DVB developers could also test it.
> Even being sent for some time, until now, nobody else tested it.

I know that people from the german vdrportal.de also use this patch
for quite some time now. Unfortunately they are not active on the
linux-media mailing list.

> > I'll try to find some time to take a look on it for Kernel 4.8,
> > but I'd like to reproduce the bug locally.
> >
> > Could you please provide me enough info to reproduce it (and
> > eventually some test MPEG-TS where you know this would happen)?
> >
> > I have two DekTek RF generators here, so I should be able to
> > play such TS and see what happens with and without the patch
> > on x86, arm32 and arm64.
>
> Ah,  forgot to mention, but checkpatch.pl wants comments for the memory
> barriers:

OK, when I wrote this patch (linux-4.4-rc6) checkpatch.pl did not
complain about missing comments. I will send a version 2 of this
patch to address these warnings.

Regards,
Soeren

> WARNING: memory barrier without comment
> #52: FILE: drivers/media/dvb-core/dvb_ringbuffer.c:58:
> +	return (rbuf->pread == smp_load_acquire(&rbuf->pwrite));
>
> WARNING: memory barrier without comment
> #70: FILE: drivers/media/dvb-core/dvb_ringbuffer.c:79:
> +	avail = smp_load_acquire(&rbuf->pwrite) - rbuf->pread;
>
> WARNING: memory barrier without comment
> #79: FILE: drivers/media/dvb-core/dvb_ringbuffer.c:89:
> +	smp_store_release(&rbuf->pread, smp_load_acquire(&rbuf->pwrite));
>
> WARNING: memory barrier without comment
> #87: FILE: drivers/media/dvb-core/dvb_ringbuffer.c:96:
> +	smp_store_release(&rbuf->pread, 0);
>
> WARNING: memory barrier without comment
> #88: FILE: drivers/media/dvb-core/dvb_ringbuffer.c:97:
> +	smp_store_release(&rbuf->pwrite, 0);
>
> WARNING: memory barrier without comment
> #97: FILE: drivers/media/dvb-core/dvb_ringbuffer.c:123:
> +		smp_store_release(&rbuf->pread, 0);
>
> WARNING: memory barrier without comment
> #103: FILE: drivers/media/dvb-core/dvb_ringbuffer.c:128:
> +	smp_store_release(&rbuf->pread, (rbuf->pread + todo) % rbuf->size);
>
> WARNING: memory barrier without comment
> #112: FILE: drivers/media/dvb-core/dvb_ringbuffer.c:143:
> +		smp_store_release(&rbuf->pread, 0);
>
> WARNING: memory barrier without comment
> #117: FILE: drivers/media/dvb-core/dvb_ringbuffer.c:147:
> +	smp_store_release(&rbuf->pread, (rbuf->pread + todo) % rbuf->size);
>
> WARNING: memory barrier without comment
> #126: FILE: drivers/media/dvb-core/dvb_ringbuffer.c:162:
> +		smp_store_release(&rbuf->pwrite, 0);
>
> WARNING: memory barrier without comment
> #130: FILE: drivers/media/dvb-core/dvb_ringbuffer.c:165:
> +	smp_store_release(&rbuf->pwrite, (rbuf->pwrite + todo) % rbuf->size);
>
> WARNING: memory barrier without comment
> #139: FILE: drivers/media/dvb-core/dvb_ringbuffer.c:185:
> +		smp_store_release(&rbuf->pwrite, 0);
>
> WARNING: memory barrier without comment
> #145: FILE: drivers/media/dvb-core/dvb_ringbuffer.c:190:
> +	smp_store_release(&rbuf->pwrite, (rbuf->pwrite + todo) % rbuf->size);
>
> Thanks,
> Mauro