Re: [PATCH] xen: Use wmb instead of rmb in xen_evtchn_do_upcall().

[Isaku Yamahata]

On Tue, Jun 10, 2008 at 05:41:39PM +1000, Nick Piggin wrote:
> On Tuesday 10 June 2008 17:35, Isaku Yamahata wrote:
> > This patch is ported one from 534:77db69c38249 of linux-2.6.18-xen.hg.
> > Use wmb instead of rmb to enforce ordering between
> > evtchn_upcall_pending and evtchn_pending_sel stores
> > in xen_evtchn_do_upcall().
> 
> There are a whole load of places in the kernel that should be using
> smp_ variants of memory barriers. This seemed to me like one of them,
> but I could be wrong.

Thank you for your comment.
Non smp_ version is intentionally used.
Because here the synchronization is done between other guest kernel
which might be running on another physical cpu. So non smp version is
really necessary even if this kernel is build for UP.


> Also, if you do that can you get rid of the ifdef? If it really *really*
> mattered, we could introduce smp_mb before/after xchg... but if you
> use smp_wmb anyway then it definitely does not matter because that is a
> noop on x86.

To be honest, I don't know whether the overhead of mb here matters
on x86 or not. I'm wondering Jeremy might have an idea.


>
> 
> Thanks,
> Nick
> 
> >
>  Cc: Samuel Thibault <samuel.thibault@xxxxxxxxxxxxx>
> > Signed-off-by: Isaku Yamahata <yamahata@xxxxxxxxxxxxx>
> > ---
> >  drivers/xen/events.c |    2 +-
> >  1 files changed, 1 insertions(+), 1 deletions(-)
> >
> > diff --git a/drivers/xen/events.c b/drivers/xen/events.c
> > index 73d78dc..332dd63 100644
> > --- a/drivers/xen/events.c
> > +++ b/drivers/xen/events.c
> > @@ -529,7 +529,7 @@ void xen_evtchn_do_upcall(struct pt_regs *regs)
> >
> >  #ifndef CONFIG_X86 /* No need for a barrier -- XCHG is a barrier on x86.
> > */ /* Clear master flag /before/ clearing selector flag. */
> > -		rmb();
> > +		wmb();
> >  #endif
> >  		pending_words = xchg(&vcpu_info->evtchn_pending_sel, 0);
> >  		while (pending_words != 0) {
> 

-- 
yamahata
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/