Re: [PATCH] xen/spinlock: move debug helpers inside the locked regions

[Julien Grall <julien@xxxxxxx>]

Hi Roger,

On 29/07/2020 14:50, Roger Pau Monné wrote:
> On Wed, Jul 29, 2020 at 02:37:44PM +0100, Julien Grall wrote:
> > Hi Roger,
> >
> > On 29/07/2020 12:13, Roger Pau Monne wrote:
> > > Debug helpers such as lock profiling or the invariant pCPU assertions
> > > must strictly be performed inside the exclusive locked region, or else
> > > races might happen.
> > >
> > > Note the issue was not strictly introduced by the pointed commit in
> > > the Fixes tag, since lock stats where already incremented before the
> > > barrier, but that commit made it more apparent as manipulating the cpu
> > > field could happen outside of the locked regions and thus trigger the
> > > BUG_ON.
> >  From the wording, it is not entirely clear which BUG_ON() you are referring
> > to. I am guessing, it is the one in rel_lock(). Am I correct?
> Yes, that's right. Expanding to:
>
> "...  and thus trigger the BUG_ON in rel_lock()." would be better.
Looks good to me. With that:

Reviewed-by: Julien Grall <jgrall@xxxxxxxxxx>

I am happy to do the update on commit if there is no more comments.

Cheers,

--
Julien Grall