On 19.03.2025 13:03, Mykola Kvach wrote:
> On Wed, Mar 5, 2025 at 6:48 PM Jan Beulich <jbeul...@suse.com> wrote:
>>
>> On 05.03.2025 10:11, Mykola Kvach wrote:
>>> From: Mirela Simonovic <mirela.simono...@aggios.com>
>>>
>>> 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
