From: Mykola Kvach <mykola_kvach@epam.com>

The scheduler_disable and scheduler_enable calls have been removed
from freeze_domains and thaw_domains, respectively, and relocated
to their usage context in enter_state. This change addresses
the concern about misleading function semantics, as the scheduler
operations are not directly related to the domain pausing/resuming
implied by the freeze/thaw naming.

Suggested-by: Jan Beulich <jbeulich@suse.com>
Signed-off-by: Mykola Kvach <mykola_kvach@epam.com>
---
The discussion about these changes can be found here:
https://lists.xenproject.org/archives/html/xen-devel/2025-03/msg00229.html
---
 xen/arch/x86/acpi/power.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/xen/arch/x86/acpi/power.c b/xen/arch/x86/acpi/power.c
index 095ca391ad..448aa9f3a7 100644
--- a/xen/arch/x86/acpi/power.c
+++ b/xen/arch/x86/acpi/power.c
@@ -151,16 +151,12 @@ static void freeze_domains(void)
     for_each_domain ( d )
         domain_pause(d);
     rcu_read_unlock(&domlist_read_lock);
-
-    scheduler_disable();
 }
 
 static void thaw_domains(void)
 {
     struct domain *d;
 
-    scheduler_enable();
-
     rcu_read_lock(&domlist_read_lock);
     for_each_domain ( d )
         domain_unpause(d);
@@ -216,6 +212,7 @@ static int enter_state(u32 state)
     printk(XENLOG_INFO "Preparing system for ACPI S%d state.\n", state);
 
     freeze_domains();
+    scheduler_disable();
 
     acpi_dmar_reinstate();
 
@@ -334,6 +331,7 @@ static int enter_state(u32 state)
     mtrr_aps_sync_end();
     iommu_adjust_irq_affinities();
     acpi_dmar_zap();
+    scheduler_enable();
     thaw_domains();
     system_state = SYS_STATE_active;
     spin_unlock(&pm_lock);
-- 
2.48.1

On 27/05/2025 11:04 am, Mykola Kvach wrote:
> From: Mykola Kvach <mykola_kvach@epam.com>
>
> The scheduler_disable and scheduler_enable calls have been removed
> from freeze_domains and thaw_domains, respectively, and relocated
> to their usage context in enter_state. This change addresses
> the concern about misleading function semantics, as the scheduler
> operations are not directly related to the domain pausing/resuming
> implied by the freeze/thaw naming.
>
> Suggested-by: Jan Beulich <jbeulich@suse.com>
> Signed-off-by: Mykola Kvach <mykola_kvach@epam.com>

Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>

On 27/05/2025 11:04 am, Mykola Kvach wrote:
> From: Mykola Kvach <mykola_kvach@epam.com>
>
> The scheduler_disable and scheduler_enable calls have been removed
> from freeze_domains and thaw_domains, respectively, and relocated
> to their usage context in enter_state. This change addresses
> the concern about misleading function semantics, as the scheduler
> operations are not directly related to the domain pausing/resuming
> implied by the freeze/thaw naming.
>
> Suggested-by: Jan Beulich <jbeulich@suse.com>
> Signed-off-by: Mykola Kvach <mykola_kvach@epam.com>

FYI I've kicked off a run with this patch:

https://gitlab.com/xen-project/hardware/xen-staging/-/pipelines/1838715729

which includes the real suspend/resume testing on several pieces of
hardware.

~Andrew

Hi, @Andrew Cooper

On Tue, May 27, 2025 at 3:53 PM Andrew Cooper <andrew.cooper3@citrix.com> wrote:
>
> On 27/05/2025 11:04 am, Mykola Kvach wrote:
> > From: Mykola Kvach <mykola_kvach@epam.com>
> >
> > The scheduler_disable and scheduler_enable calls have been removed
> > from freeze_domains and thaw_domains, respectively, and relocated
> > to their usage context in enter_state. This change addresses
> > the concern about misleading function semantics, as the scheduler
> > operations are not directly related to the domain pausing/resuming
> > implied by the freeze/thaw naming.
> >
> > Suggested-by: Jan Beulich <jbeulich@suse.com>
> > Signed-off-by: Mykola Kvach <mykola_kvach@epam.com>
>
> FYI I've kicked off a run with this patch:
>
> https://gitlab.com/xen-project/hardware/xen-staging/-/pipelines/1838715729
>
> which includes the real suspend/resume testing on several pieces of
> hardware.

It appears I made a mistake by failing to mark this patch as
containing only non-functional changes.

>
> ~Andrew

Best regards,
Mykola

On 27/05/2025 5:17 pm, Mykola Kvach wrote:
> Hi, @Andrew Cooper
>
> On Tue, May 27, 2025 at 3:53 PM Andrew Cooper <andrew.cooper3@citrix.com> wrote:
>> On 27/05/2025 11:04 am, Mykola Kvach wrote:
>>> From: Mykola Kvach <mykola_kvach@epam.com>
>>>
>>> The scheduler_disable and scheduler_enable calls have been removed
>>> from freeze_domains and thaw_domains, respectively, and relocated
>>> to their usage context in enter_state. This change addresses
>>> the concern about misleading function semantics, as the scheduler
>>> operations are not directly related to the domain pausing/resuming
>>> implied by the freeze/thaw naming.
>>>
>>> Suggested-by: Jan Beulich <jbeulich@suse.com>
>>> Signed-off-by: Mykola Kvach <mykola_kvach@epam.com>
>> FYI I've kicked off a run with this patch:
>>
>> https://gitlab.com/xen-project/hardware/xen-staging/-/pipelines/1838715729
>>
>> which includes the real suspend/resume testing on several pieces of
>> hardware.
> It appears I made a mistake by failing to mark this patch as
> containing only non-functional changes.

x86 suspend/resume is sufficiently fragile that you only have to
threaten a change for something to break.

But yes, "No functional change." goes a long way towards helping the
reviewer.

~Andrew