Re: [PATCH 7/7] xen/evtchn: read producer index only once

[Julien Grall]

Hi Juergen,

On 08/02/2021 11:48, Jürgen Groß wrote:
> On 08.02.21 12:40, Julien Grall wrote:
> > On 06/02/2021 10:49, Juergen Gross wrote:
> > > In evtchn_read() use READ_ONCE() for reading the producer index in
> > > order to avoid the compiler generating multiple accesses.
> > >
> > > Signed-off-by: Juergen Gross <jgross@xxxxxxxx>
> > > ---
> > >   drivers/xen/evtchn.c | 2 +-
> > >   1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/xen/evtchn.c b/drivers/xen/evtchn.c
> > > index 421382c73d88..f6b199b597bf 100644
> > > --- a/drivers/xen/evtchn.c
> > > +++ b/drivers/xen/evtchn.c
> > > @@ -211,7 +211,7 @@ static ssize_t evtchn_read(struct file *file, 
> > > char __user *buf,
> > >               goto unlock_out;
> > >           c = u->ring_cons;
> > > -        p = u->ring_prod;
> > > +        p = READ_ONCE(u->ring_prod);
> > For consistency, don't you also need the write side in 
> > evtchn_interrupt() to use WRITE_ONCE()?
>
> Only in case I'd consider the compiler needing multiple memory
> accesses for doing the update (see my reply to Jan's comment on this
> patch).

Right, I have just answered there :). AFAICT, without using 
WRITE_ONCE()/READ_ONCE() there is no guarantee that load/store tearing 
will not happen.

We can continue the conversation there.

Cheers,

> Juergen

--
Julien Grall