On 24.08.22 11:31, Julien Grall wrote:
> Hi Juergen,
> 
> On 24/08/2022 10:27, Juergen Gross wrote:
>> On 24.08.22 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(). This would also allow us to catch issue in production rather than 
>>> in only in debug.
>>
>> What about something like the following then?
> 
> That could be a first step. I still think a message like Penny has added in the 
> patch is useful.

As the WARN() would spit out file and line, a comment might be enough.

In the end that is something the maintainer of the related code should decide.


Juergen