Re: [PATCH v4] page_alloc: assert IRQs are enabled in heap alloc/free

[Julien Grall <julien@xxxxxxx>]

Hi,

On 25/04/2022 15:13, David Vrabel wrote:
> On 25/04/2022 14:43, Julien Grall wrote:
> > Hi Jan,
> >
> > On 25/04/2022 14:37, Jan Beulich wrote:
> > > On 25.04.2022 15:34, Julien Grall wrote:
> > > > On 25/04/2022 14:28, David Vrabel wrote:
> > > > > --- a/xen/common/page_alloc.c
> > > > > +++ b/xen/common/page_alloc.c
> > > > > @@ -162,6 +162,13 @@
> > > > >    static char __initdata opt_badpage[100] = "";
> > > > >    string_param("badpage", opt_badpage);
> > > > > +/*
> > > > > + * Heap allocations may need TLB flushes which require IRQs to be
> > > > The comment needs to be updated to reflect the fact that at least Arm
> > > > doesn't use IPI to flush TLBs.
> > > I thought the use of "may" was satisfying your earlier request?
> > Maybe I read wrongly this comment... To me, anything after 'which' is 
> > optional (it is a non-defining clause) and describe how the TLB 
> > flushes works. So the 'may' here is referring to the possibility to 
> > use flush TLB.
> Oh dear, you're using formal grammar with a native English speaker who's 
> never seen a grammar rule in any of his schooling.
> I think this should be:
>
> "Heap allocations may need TLB flushes that require IRQs..."
>
> i.e., "that" instead of "which"
I am fine with that.

Cheers,

--
Julien Grall