Re: [PATCH v10 2/9] xen: do not free reserved memory into heap

[Julien Grall <julien@xxxxxxx>]

Hi,

On 16/08/2022 07:40, Jan Beulich wrote:
> On 16.08.2022 04:36, Penny Zheng wrote:
> > +void free_domstatic_page(struct page_info *page)
> > +{
> > +    struct domain *d = page_get_owner(page);
> > +    bool drop_dom_ref;
> > +
> > +    if ( unlikely(!d) )
> > +    {
> > +        ASSERT_UNREACHABLE();
> > +        printk("The about-to-free static page %"PRI_mfn" must be owned by a 
> > domain\n",
> > +               mfn_x(page_to_mfn(page)));
> > +        return;
> > +    }
> For the message to be useful as a hint if the assertion triggers, it
> wants printing ahead of the assertion. I also think it wants to be a
> XENLOG_G_* kind of log level, so it would be rate limited by default
> in release builds. Just to be on the safe side.
+1

> (I'm not in favor of
> the log message in the first place, but I do know that Julien had
> asked for one.)
TBH, I think all ASSERT_UNREACHABLE() paths should be accompanied with a 
printk(). This would also allow us to catch issue in production rather 
than in only in debug.
In particular, for the page allocator, I have seen issue that only 
happen very rarely. Although, this seems unlikely in this case.
Cheers,

--
Julien Grall