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

[Mykola Kvach <xakep.amatop@xxxxxxxxx>]

On Wed, Mar 19, 2025 at 2:47 PM Jan Beulich <jbeulich@xxxxxxxx> wrote:
>
> 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.

got it, thank you

>
> Jan