Re: [PATCH 02/16] xen/x86: Move freeze/thaw_domains into common files

[Jan Beulich <jbeulich@xxxxxxxx>]

On 19.03.2025 13:03, Mykola Kvach wrote:
> On Wed, Mar 5, 2025 at 6:48 PM Jan Beulich <jbeulich@xxxxxxxx> wrote:
>>
>> On 05.03.2025 10:11, Mykola Kvach wrote:
>>> From: Mirela Simonovic <mirela.simonovic@xxxxxxxxxx>
>>>
>>> These functions will be reused by suspend/resume support for ARM.
>>
>> And until then they are going to violate the Misra rule requiring there
>> to not be unreachable code.
>>
>>> --- a/xen/common/domain.c
>>> +++ b/xen/common/domain.c
>>> @@ -2259,6 +2259,36 @@ int continue_hypercall_on_cpu(
>>>      return 0;
>>>  }
>>>
>>> +
>>> +void freeze_domains(void)
>>
>> Nit: No double blank lines please.
> 
> Thanks for pointing that out! I'll fix it in the next version of the
> patch series.
> 
>>
>>> +{
>>> +    struct domain *d;
>>> +
>>> +    rcu_read_lock(&domlist_read_lock);
>>> +    /*
>>> +     * Note that we iterate in order of domain-id. Hence we will pause dom0
>>> +     * first which is required for correctness (as only dom0 can add 
>>> domains to
>>> +     * the domain list). Otherwise we could miss concurrently-created 
>>> domains.
>>> +     */
>>> +    for_each_domain ( d )
>>> +        domain_pause(d);
>>> +    rcu_read_unlock(&domlist_read_lock);
>>> +
>>> +    scheduler_disable();
>>
>> When made generally available I'm unsure having this and ...
>>
>>> +}
>>> +
>>> +void thaw_domains(void)
>>> +{
>>> +    struct domain *d;
>>> +
>>> +    scheduler_enable();
>>
>> ... this here is a good idea. Both scheduler operations aren't related
>> to what the function names say is being done here.
> 
> I have just moved these functions from x86-specific headers to a common one,
> but they are still used only for suspend/resume purposes.
> It's not a problem for me to adjust the names slightly in the next
> version of the
> patch series.

I wasn't after a rename really; my suggestion was to leave the scheduler
calls at the original call sites, and remove them from here.

Jan