On 24.08.2022 11:03, Julien Grall wrote:
> 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().
If you want more than just the line number, use ASSERT() with a meaningful
expression. That'll be easily a fair replacement for a separate printk().
And no, as said in reply to Jürgen's patch, ASSERT() and friends should
not leave any traces in non-debug builds. If you want such, use WARN_ON()
or BUG_ON().
Jan
