Re: [Xen-devel] [PATCH v3 5/6] xen/rcu: add assertions to debug build

[Julien Grall <julien@xxxxxxx>]

Hi,

On 06/03/2020 14:35, Jürgen Groß wrote:
> On 04.03.20 14:42, Julien Grall wrote:
> > Hi,
> >
> > On 04/03/2020 06:32, Juergen Gross wrote:
> > > diff --git a/xen/include/xen/rcupdate.h b/xen/include/xen/rcupdate.h
> > > index 31c8b86d13..9f6d420898 100644
> > > --- a/xen/include/xen/rcupdate.h
> > > +++ b/xen/include/xen/rcupdate.h
> > > @@ -34,10 +34,40 @@
> > >   #include <xen/cache.h>
> > >   #include <xen/spinlock.h>
> > >   #include <xen/cpumask.h>
> > > -#include <xen/preempt.h>
> > > +#include <xen/percpu.h>
> > > +#include <asm/atomic.h>
> > >   #define __rcu
> > > +#ifndef NDEBUG
> > > +DECLARE_PER_CPU(unsigned int, rcu_lock_cnt);
> > > +
> > > +static inline void rcu_quiesce_disable(void)
> > > +{
> > > +    this_cpu(rcu_lock_cnt)++;
> > > +    arch_lock_acquire_barrier();
> > I am not sure to understand the goal of this barrier. What are you 
> > trying to protect against?
> This is the result of a request by Roger, which seemed reasonable,
> although I should have checked the suggested barrier type more
> thoroughly.
>
> He suggested to add barriers like in the former preempt_[en|dis]able()
> cases, but to use the acquire and release barriers like in locks.
I have CCed Roger as I don't understand why you would want memory 
ordering with all the CPUs on Arm.
> Thinking more about it I think a simple barrier() should do the trick as
> only cpu local protection is needed.
Note that on Arm barrier() is only a compiler barrier. It does not 
prevent a CPU to re-order the memory access. But I think the barrier() 
ought to be fine in this case (although, I am not 100% sure).
Cheers,

--
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel