[PATCH 0/3][RFC/RFT] Do not delay the MTRR synchronization for

[PATCH 0/3][RFC/RFT] Do not delay the MTRR synchronization for

[Yu Chen]

From: Chen Yu <yu.c.chen@intel.com>

The original problem comes from a bug found on MacBookPro that,
the instructions run on each APs after resume are very slow, due
to the MTRR been scribbled by the BIOS thus it behaves like
running in 'uncached' mode.

Thus this patch tries to synchronize the MTRR as early as possible
by performing this action once the APs have been brought.

Test result shows a 6 seconds improvement on the bogus MacBookPro,
and 600 ms improvement on a Xeon Broadwell platform which has
88 cpus.

Chen Yu (3):
  PM / sleep: Introduce a flag to identify the enable_nonboot_cpus stage
  x86/mtrr: Add cpu id into the MTRR setting data structure
  PM / sleep: Do not delay the synchronization of MTRR during resume

 arch/x86/kernel/cpu/mtrr/main.c | 13 ++++++++++---
 arch/x86/kernel/smpboot.c       |  2 --
 include/linux/cpu.h             |  2 ++
 kernel/cpu.c                    |  8 ++++++++
 4 files changed, 20 insertions(+), 5 deletions(-)

-- 
2.13.5

[PATCH 1/3][RFC/RFT] PM / sleep: Introduce a flag to identify the enable_nonboot_cpus stage

[Yu Chen]

From: Chen Yu <yu.c.chen@intel.com>

A new flag is introduced to indicate that, we are in the process of
running enable_nonboot_cpus() to bring the APs up. This flag is to
prepare for the use of MTRR sync later.

Cc: Len Brown <len.brown@intel.com>
Cc: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Cc: Rui Zhang <rui.zhang@intel.com>
Signed-off-by: Chen Yu <yu.c.chen@intel.com>
---
 include/linux/cpu.h | 2 ++
 kernel/cpu.c        | 8 ++++++++
 2 files changed, 10 insertions(+)

diff --git a/include/linux/cpu.h b/include/linux/cpu.h
index ca73bc1563f4..f0759d837919 100644
--- a/include/linux/cpu.h
+++ b/include/linux/cpu.h
@@ -133,9 +133,11 @@ static inline int disable_nonboot_cpus(void)
 	return freeze_secondary_cpus(0);
 }
 extern void enable_nonboot_cpus(void);
+extern bool in_enable_nonboot_cpus(void);
 #else /* !CONFIG_PM_SLEEP_SMP */
 static inline int disable_nonboot_cpus(void) { return 0; }
 static inline void enable_nonboot_cpus(void) {}
+static inline bool in_enable_nonboot_cpus(void) { return false; }
 #endif /* !CONFIG_PM_SLEEP_SMP */
 
 void cpu_startup_entry(enum cpuhp_state state);
diff --git a/kernel/cpu.c b/kernel/cpu.c
index 04892a82f6ac..071dcae8b0fb 100644
--- a/kernel/cpu.c
+++ b/kernel/cpu.c
@@ -1077,6 +1077,12 @@ EXPORT_SYMBOL_GPL(cpu_up);
 
 #ifdef CONFIG_PM_SLEEP_SMP
 static cpumask_var_t frozen_cpus;
+static bool enable_nonboot_cpus_run;
+
+bool in_enable_nonboot_cpus(void)
+{
+	return enable_nonboot_cpus_run;
+}
 
 int freeze_secondary_cpus(int primary)
 {
@@ -1143,6 +1149,7 @@ void enable_nonboot_cpus(void)
 	pr_info("Enabling non-boot CPUs ...\n");
 
 	arch_enable_nonboot_cpus_begin();
+	enable_nonboot_cpus_run = true;
 
 	for_each_cpu(cpu, frozen_cpus) {
 		trace_suspend_resume(TPS("CPU_ON"), cpu, true);
@@ -1155,6 +1162,7 @@ void enable_nonboot_cpus(void)
 		pr_warn("Error taking CPU%d up: %d\n", cpu, error);
 	}
 
+	enable_nonboot_cpus_run = false;
 	arch_enable_nonboot_cpus_end();
 
 	cpumask_clear(frozen_cpus);
-- 
2.13.5

[PATCH 2/3][RFC/RFT] x86/mtrr: Add cpu id into the MTRR setting data structure

[Yu Chen]

From: Chen Yu <yu.c.chen@intel.com>

We record the cpu which has issued the MTRR setting command, then
in the mtrr_rendezvous_handler() we can distinguish it from the
other cpus. This is used for MTRR synchronization optimization.

Cc: Len Brown <len.brown@intel.com>
Cc: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Cc: Rui Zhang <rui.zhang@intel.com>
Signed-off-by: Chen Yu <yu.c.chen@intel.com>
---
 arch/x86/kernel/cpu/mtrr/main.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/arch/x86/kernel/cpu/mtrr/main.c b/arch/x86/kernel/cpu/mtrr/main.c
index 40d5a8a75212..a4e7e23f3c2e 100644
--- a/arch/x86/kernel/cpu/mtrr/main.c
+++ b/arch/x86/kernel/cpu/mtrr/main.c
@@ -148,6 +148,7 @@ struct set_mtrr_data {
 	unsigned long	smp_size;
 	unsigned int	smp_reg;
 	mtrr_type	smp_type;
+	int		smp_cpu;
 };
 
 /**
@@ -231,7 +232,8 @@ set_mtrr(unsigned int reg, unsigned long base, unsigned long size, mtrr_type typ
 	struct set_mtrr_data data = { .smp_reg = reg,
 				      .smp_base = base,
 				      .smp_size = size,
-				      .smp_type = type
+				      .smp_type = type,
+				      .smp_cpu = smp_processor_id()
 				    };
 
 	stop_machine(mtrr_rendezvous_handler, &data, cpu_online_mask);
@@ -243,7 +245,8 @@ static void set_mtrr_cpuslocked(unsigned int reg, unsigned long base,
 	struct set_mtrr_data data = { .smp_reg = reg,
 				      .smp_base = base,
 				      .smp_size = size,
-				      .smp_type = type
+				      .smp_type = type,
+				      .smp_cpu = smp_processor_id()
 				    };
 
 	stop_machine_cpuslocked(mtrr_rendezvous_handler, &data, cpu_online_mask);
@@ -255,7 +258,8 @@ static void set_mtrr_from_inactive_cpu(unsigned int reg, unsigned long base,
 	struct set_mtrr_data data = { .smp_reg = reg,
 				      .smp_base = base,
 				      .smp_size = size,
-				      .smp_type = type
+				      .smp_type = type,
+				      .smp_cpu = smp_processor_id()
 				    };
 
 	stop_machine_from_inactive_cpu(mtrr_rendezvous_handler, &data,
-- 
2.13.5

[PATCH 3/3][RFC/RFT] PM / sleep: Do not delay the synchronization of MTRR during resume

[Yu Chen]

From: Chen Yu <yu.c.chen@intel.com>

Sometimes it is too late for the APs to synchronize the MTRR
after all the APs have been brought up. In some cases the BIOS
might scribble the MTRR across suspend/resume, as a result we
might get insane MTRR after resumed back, thus every instruction
run on this AP would be extremely slow if it happended to be 'uncached'
in the MTRR, although after all the APs have been brought up, the
delayed invoking of set_mtrr_state() will adjust the MTRR on APs
thus brings everything back to normal. In practice it is necessary
to synchronize the MTRR as early as possible to get it fixed during
each AP's online. And this is what this patch does.

Moreover, since we have put the synchronization earlier, there is
a side effect that, during each AP's online, the other cpus already
online will be forced stopped to run mtrr_rendezvous_handler() and
reprogram the MTRR again and again. This symptom can be lessened
by checking if this cpu has already finished the synchronization
during the enable_nonboot_cpus() stage, if it is, we can safely
bypass the reprogramming action. (We can not bypass the
mtrr_rendezvous_handler(), because the other online cpus must be
stopped running the VMX transaction while one cpu is updating
its MTRR, which is described here:
Commit d0af9eed5aa9 ("x86, pat/mtrr: Rendezvous all the cpus for
MTRR/PAT init")

This patch does not impact the normal boot up and cpu hotplug.

On a platform with insane MTRR after resumed,
1 .before this patch:
   [  619.810899] Enabling non-boot CPUs ...
   [  619.825537] smpboot: Booting Node 0 Processor 1 APIC 0x2
   [  621.723809] CPU1 is up
   [  621.840228] smpboot: Booting Node 0 Processor 3 APIC 0x3
   [  626.690900] CPU3 is up

So it took cpu1 621.723809 - 619.825537 = 1898.272 ms, and
cpu3 626.690900 - 621.840228 = 4850.672 ms.

2. after this patch:
   [  106.931790] smpboot: Booting Node 0 Processor 1 APIC 0x2
   [  106.948360] CPU1 is up
   [  106.986534] smpboot: Booting Node 0 Processor 3 APIC 0x3
   [  106.990702] CPU3 is up

It took cpu1 106.948360 - 106.931790 = 16.57 ms, and
cpu3 106.990702 - 106.986534 = 4.16 ms.

For comparison, I also verify the suspend on a 88 cpus Xeon Broadwell
platform, and the result also shows that with this patch applied,
the overall APs online time has decreased a little bit, I think this
is because after we synchronize the MTRR earlier, the MTRRs also get
updated to the correct value earlier.

I've tested 5 times each, before/after the patch applied:

1. before this patch:
[   64.549430] Enabling non-boot CPUs ...
[   66.253304] End enabling non-boot CPUs

overall cpu online: 1.703874 second

[   62.159063] Enabling non-boot CPUs ...
[   64.483443] End enabling non-boot CPUs

overall cpu online: 2.32438 second

[   58.351449] Enabling non-boot CPUs ...
[   60.796951] End enabling non-boot CPUs

overall cpu online: 2.445502 second

[   64.491317] Enabling non-boot CPUs ...
[   66.996208] End enabling non-boot CPUs

overall cpu online: 2.504891 second

[   60.593235] Enabling non-boot CPUs ...
[   63.397886] End enabling non-boot CPUs

overall cpu online: 2.804651 second

average: 2.3566596 second

2. after this patch:

[   66.077368] Enabling non-boot CPUs ...
[   68.343374] End enabling non-boot CPUs

overall cpu online: 2.266006 second

[   64.594605] Enabling non-boot CPUs ...
[   66.092688] End enabling non-boot CPUs

overall cpu online: 1.498083 second

[   64.778546] Enabling non-boot CPUs ...
[   66.277577] End enabling non-boot CPUs

overall cpu online: 1.499031 second

[   63.773091] Enabling non-boot CPUs ...
[   65.601637] End enabling non-boot CPUs

overall cpu online: 1.828546 second

[   64.638855] Enabling non-boot CPUs ...
[   66.327098] End enabling non-boot CPUs

overall cpu online: 1.688243 second

average: 1.7559818 second

In one word, with the patch applied, the cpu online time during resume
has decreased by about 6 seconds on a bogus MTRR platform, and decreased
by about 600ms on a 88 cpus Xeon platform after resumed.

Cc: Len Brown <len.brown@intel.com>
Cc: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Cc: Rui Zhang <rui.zhang@intel.com>
Signed-off-by: Chen Yu <yu.c.chen@intel.com>
---
 arch/x86/kernel/cpu/mtrr/main.c | 3 +++
 arch/x86/kernel/smpboot.c       | 2 --
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kernel/cpu/mtrr/main.c b/arch/x86/kernel/cpu/mtrr/main.c
index a4e7e23f3c2e..f4c2c60c6ac8 100644
--- a/arch/x86/kernel/cpu/mtrr/main.c
+++ b/arch/x86/kernel/cpu/mtrr/main.c
@@ -179,6 +179,9 @@ static int mtrr_rendezvous_handler(void *info)
 		mtrr_if->set(data->smp_reg, data->smp_base,
 			     data->smp_size, data->smp_type);
 	} else if (mtrr_aps_delayed_init || !cpu_online(smp_processor_id())) {
+		if (in_enable_nonboot_cpus() &&
+		    data->smp_cpu != smp_processor_id())
+			return 0;
 		mtrr_if->set_all();
 	}
 	return 0;
diff --git a/arch/x86/kernel/smpboot.c b/arch/x86/kernel/smpboot.c
index ad59edd84de7..2d1ec878df63 100644
--- a/arch/x86/kernel/smpboot.c
+++ b/arch/x86/kernel/smpboot.c
@@ -1366,12 +1366,10 @@ void __init native_smp_prepare_cpus(unsigned int max_cpus)
 
 void arch_enable_nonboot_cpus_begin(void)
 {
-	set_mtrr_aps_delayed_init();
 }
 
 void arch_enable_nonboot_cpus_end(void)
 {
-	mtrr_aps_init();
 }
 
 /*
-- 
2.13.5

Re: [PATCH 3/3][RFC/RFT] PM / sleep: Do not delay the synchronization of MTRR during resume

[Rafael J. Wysocki]

On Tuesday, October 31, 2017 10:58:50 AM CET Yu Chen wrote:
> From: Chen Yu <yu.c.chen@intel.com>
> 
> Sometimes it is too late for the APs to synchronize the MTRR
> after all the APs have been brought up. In some cases the BIOS
> might scribble the MTRR across suspend/resume, as a result we
> might get insane MTRR after resumed back, thus every instruction
> run on this AP would be extremely slow if it happended to be 'uncached'
> in the MTRR, although after all the APs have been brought up, the
> delayed invoking of set_mtrr_state() will adjust the MTRR on APs
> thus brings everything back to normal. In practice it is necessary
> to synchronize the MTRR as early as possible to get it fixed during
> each AP's online. And this is what this patch does.
> 
> Moreover, since we have put the synchronization earlier, there is
> a side effect that, during each AP's online, the other cpus already
> online will be forced stopped to run mtrr_rendezvous_handler() and
> reprogram the MTRR again and again. This symptom can be lessened
> by checking if this cpu has already finished the synchronization
> during the enable_nonboot_cpus() stage, if it is, we can safely
> bypass the reprogramming action. (We can not bypass the
> mtrr_rendezvous_handler(), because the other online cpus must be
> stopped running the VMX transaction while one cpu is updating
> its MTRR, which is described here:
> Commit d0af9eed5aa9 ("x86, pat/mtrr: Rendezvous all the cpus for
> MTRR/PAT init")
> 
> This patch does not impact the normal boot up and cpu hotplug.
> 
> On a platform with insane MTRR after resumed,
> 1 .before this patch:
>    [  619.810899] Enabling non-boot CPUs ...
>    [  619.825537] smpboot: Booting Node 0 Processor 1 APIC 0x2
>    [  621.723809] CPU1 is up
>    [  621.840228] smpboot: Booting Node 0 Processor 3 APIC 0x3
>    [  626.690900] CPU3 is up
> 
> So it took cpu1 621.723809 - 619.825537 = 1898.272 ms, and
> cpu3 626.690900 - 621.840228 = 4850.672 ms.
> 
> 2. after this patch:
>    [  106.931790] smpboot: Booting Node 0 Processor 1 APIC 0x2
>    [  106.948360] CPU1 is up
>    [  106.986534] smpboot: Booting Node 0 Processor 3 APIC 0x3
>    [  106.990702] CPU3 is up
> 
> It took cpu1 106.948360 - 106.931790 = 16.57 ms, and
> cpu3 106.990702 - 106.986534 = 4.16 ms.
> 
> For comparison, I also verify the suspend on a 88 cpus Xeon Broadwell
> platform, and the result also shows that with this patch applied,
> the overall APs online time has decreased a little bit, I think this
> is because after we synchronize the MTRR earlier, the MTRRs also get
> updated to the correct value earlier.
> 
> I've tested 5 times each, before/after the patch applied:
> 
> 1. before this patch:
> [   64.549430] Enabling non-boot CPUs ...
> [   66.253304] End enabling non-boot CPUs
> 
> overall cpu online: 1.703874 second
> 
> [   62.159063] Enabling non-boot CPUs ...
> [   64.483443] End enabling non-boot CPUs
> 
> overall cpu online: 2.32438 second
> 
> [   58.351449] Enabling non-boot CPUs ...
> [   60.796951] End enabling non-boot CPUs
> 
> overall cpu online: 2.445502 second
> 
> [   64.491317] Enabling non-boot CPUs ...
> [   66.996208] End enabling non-boot CPUs
> 
> overall cpu online: 2.504891 second
> 
> [   60.593235] Enabling non-boot CPUs ...
> [   63.397886] End enabling non-boot CPUs
> 
> overall cpu online: 2.804651 second
> 
> average: 2.3566596 second
> 
> 2. after this patch:
> 
> [   66.077368] Enabling non-boot CPUs ...
> [   68.343374] End enabling non-boot CPUs
> 
> overall cpu online: 2.266006 second
> 
> [   64.594605] Enabling non-boot CPUs ...
> [   66.092688] End enabling non-boot CPUs
> 
> overall cpu online: 1.498083 second
> 
> [   64.778546] Enabling non-boot CPUs ...
> [   66.277577] End enabling non-boot CPUs
> 
> overall cpu online: 1.499031 second
> 
> [   63.773091] Enabling non-boot CPUs ...
> [   65.601637] End enabling non-boot CPUs
> 
> overall cpu online: 1.828546 second
> 
> [   64.638855] Enabling non-boot CPUs ...
> [   66.327098] End enabling non-boot CPUs
> 
> overall cpu online: 1.688243 second
> 
> average: 1.7559818 second
> 
> In one word, with the patch applied, the cpu online time during resume
> has decreased by about 6 seconds on a bogus MTRR platform, and decreased
> by about 600ms on a 88 cpus Xeon platform after resumed.
> 
> Cc: Len Brown <len.brown@intel.com>
> Cc: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> Cc: Rui Zhang <rui.zhang@intel.com>
> Signed-off-by: Chen Yu <yu.c.chen@intel.com>

It will be better to combine this with patch [2/3] IMO, because that makes it
clear why the changes in that patch are needed.

Also you can define the new flag in mtrr/main.c, set it in
arch_enable_nonboot_cpus_begin() and clear it in
arch_enable_nonboot_cpus_end().  It is better to put it into the arch-specific
code as the flag itself is arch-specific.

Then, of course, you don't need patch [1/3] and all can be done in one patch.

> ---
>  arch/x86/kernel/cpu/mtrr/main.c | 3 +++
>  arch/x86/kernel/smpboot.c       | 2 --
>  2 files changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/x86/kernel/cpu/mtrr/main.c b/arch/x86/kernel/cpu/mtrr/main.c
> index a4e7e23f3c2e..f4c2c60c6ac8 100644
> --- a/arch/x86/kernel/cpu/mtrr/main.c
> +++ b/arch/x86/kernel/cpu/mtrr/main.c
> @@ -179,6 +179,9 @@ static int mtrr_rendezvous_handler(void *info)
>  		mtrr_if->set(data->smp_reg, data->smp_base,
>  			     data->smp_size, data->smp_type);
>  	} else if (mtrr_aps_delayed_init || !cpu_online(smp_processor_id())) {
> +		if (in_enable_nonboot_cpus() &&
> +		    data->smp_cpu != smp_processor_id())
> +			return 0;
>  		mtrr_if->set_all();
>  	}
>  	return 0;
> diff --git a/arch/x86/kernel/smpboot.c b/arch/x86/kernel/smpboot.c
> index ad59edd84de7..2d1ec878df63 100644
> --- a/arch/x86/kernel/smpboot.c
> +++ b/arch/x86/kernel/smpboot.c
> @@ -1366,12 +1366,10 @@ void __init native_smp_prepare_cpus(unsigned int max_cpus)
>  
>  void arch_enable_nonboot_cpus_begin(void)
>  {
> -	set_mtrr_aps_delayed_init();
>  }
>  
>  void arch_enable_nonboot_cpus_end(void)
>  {
> -	mtrr_aps_init();
>  }
>  
>  /*
> 

Re: [PATCH 3/3][RFC/RFT] PM / sleep: Do not delay the synchronization of MTRR during resume

[Yu Chen]

On Wed, Dec 13, 2017 at 01:31:50AM +0100, Rafael J. Wysocki wrote:
> On Tuesday, October 31, 2017 10:58:50 AM CET Yu Chen wrote:
> > From: Chen Yu <yu.c.chen@intel.com>
> > 
> > Sometimes it is too late for the APs to synchronize the MTRR
> > after all the APs have been brought up. In some cases the BIOS
> > might scribble the MTRR across suspend/resume, as a result we
> > might get insane MTRR after resumed back, thus every instruction
> > run on this AP would be extremely slow if it happended to be 'uncached'
> > in the MTRR, although after all the APs have been brought up, the
> > delayed invoking of set_mtrr_state() will adjust the MTRR on APs
> > thus brings everything back to normal. In practice it is necessary
> > to synchronize the MTRR as early as possible to get it fixed during
> > each AP's online. And this is what this patch does.
> > 
> > Moreover, since we have put the synchronization earlier, there is
> > a side effect that, during each AP's online, the other cpus already
> > online will be forced stopped to run mtrr_rendezvous_handler() and
> > reprogram the MTRR again and again. This symptom can be lessened
> > by checking if this cpu has already finished the synchronization
> > during the enable_nonboot_cpus() stage, if it is, we can safely
> > bypass the reprogramming action. (We can not bypass the
> > mtrr_rendezvous_handler(), because the other online cpus must be
> > stopped running the VMX transaction while one cpu is updating
> > its MTRR, which is described here:
> > Commit d0af9eed5aa9 ("x86, pat/mtrr: Rendezvous all the cpus for
> > MTRR/PAT init")
> > 
> > This patch does not impact the normal boot up and cpu hotplug.
> > 
> > On a platform with insane MTRR after resumed,
> > 1 .before this patch:
> >    [  619.810899] Enabling non-boot CPUs ...
> >    [  619.825537] smpboot: Booting Node 0 Processor 1 APIC 0x2
> >    [  621.723809] CPU1 is up
> >    [  621.840228] smpboot: Booting Node 0 Processor 3 APIC 0x3
> >    [  626.690900] CPU3 is up
> > 
> > So it took cpu1 621.723809 - 619.825537 = 1898.272 ms, and
> > cpu3 626.690900 - 621.840228 = 4850.672 ms.
> > 
> > 2. after this patch:
> >    [  106.931790] smpboot: Booting Node 0 Processor 1 APIC 0x2
> >    [  106.948360] CPU1 is up
> >    [  106.986534] smpboot: Booting Node 0 Processor 3 APIC 0x3
> >    [  106.990702] CPU3 is up
> > 
> > It took cpu1 106.948360 - 106.931790 = 16.57 ms, and
> > cpu3 106.990702 - 106.986534 = 4.16 ms.
> > 
> > For comparison, I also verify the suspend on a 88 cpus Xeon Broadwell
> > platform, and the result also shows that with this patch applied,
> > the overall APs online time has decreased a little bit, I think this
> > is because after we synchronize the MTRR earlier, the MTRRs also get
> > updated to the correct value earlier.
> > 
> > I've tested 5 times each, before/after the patch applied:
> > 
> > 1. before this patch:
> > [   64.549430] Enabling non-boot CPUs ...
> > [   66.253304] End enabling non-boot CPUs
> > 
> > overall cpu online: 1.703874 second
> > 
> > [   62.159063] Enabling non-boot CPUs ...
> > [   64.483443] End enabling non-boot CPUs
> > 
> > overall cpu online: 2.32438 second
> > 
> > [   58.351449] Enabling non-boot CPUs ...
> > [   60.796951] End enabling non-boot CPUs
> > 
> > overall cpu online: 2.445502 second
> > 
> > [   64.491317] Enabling non-boot CPUs ...
> > [   66.996208] End enabling non-boot CPUs
> > 
> > overall cpu online: 2.504891 second
> > 
> > [   60.593235] Enabling non-boot CPUs ...
> > [   63.397886] End enabling non-boot CPUs
> > 
> > overall cpu online: 2.804651 second
> > 
> > average: 2.3566596 second
> > 
> > 2. after this patch:
> > 
> > [   66.077368] Enabling non-boot CPUs ...
> > [   68.343374] End enabling non-boot CPUs
> > 
> > overall cpu online: 2.266006 second
> > 
> > [   64.594605] Enabling non-boot CPUs ...
> > [   66.092688] End enabling non-boot CPUs
> > 
> > overall cpu online: 1.498083 second
> > 
> > [   64.778546] Enabling non-boot CPUs ...
> > [   66.277577] End enabling non-boot CPUs
> > 
> > overall cpu online: 1.499031 second
> > 
> > [   63.773091] Enabling non-boot CPUs ...
> > [   65.601637] End enabling non-boot CPUs
> > 
> > overall cpu online: 1.828546 second
> > 
> > [   64.638855] Enabling non-boot CPUs ...
> > [   66.327098] End enabling non-boot CPUs
> > 
> > overall cpu online: 1.688243 second
> > 
> > average: 1.7559818 second
> > 
> > In one word, with the patch applied, the cpu online time during resume
> > has decreased by about 6 seconds on a bogus MTRR platform, and decreased
> > by about 600ms on a 88 cpus Xeon platform after resumed.
> > 
> > Cc: Len Brown <len.brown@intel.com>
> > Cc: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> > Cc: Rui Zhang <rui.zhang@intel.com>
> > Signed-off-by: Chen Yu <yu.c.chen@intel.com>
> 
> It will be better to combine this with patch [2/3] IMO, because that makes it
> clear why the changes in that patch are needed.
> 
> Also you can define the new flag in mtrr/main.c, set it in
> arch_enable_nonboot_cpus_begin() and clear it in
> arch_enable_nonboot_cpus_end().  It is better to put it into the arch-specific
> code as the flag itself is arch-specific.
> 
> Then, of course, you don't need patch [1/3] and all can be done in one patch.
> 
Ok, will rewrite the patch, thanks! 

Re: [PATCH 3/3][RFC/RFT] PM / sleep: Do not delay the synchronization of MTRR during resume

[Lukas Wunner]

On Thu, Dec 14, 2017 at 12:02:42AM +0800, Yu Chen wrote:
> On Wed, Dec 13, 2017 at 01:31:50AM +0100, Rafael J. Wysocki wrote:
> > On Tuesday, October 31, 2017 10:58:50 AM CET Yu Chen wrote:
[snip]
> > > In one word, with the patch applied, the cpu online time during resume
> > > has decreased by about 6 seconds on a bogus MTRR platform, and decreased
> > > by about 600ms on a 88 cpus Xeon platform after resumed.
> > > 
> > > Cc: Len Brown <len.brown@intel.com>
> > > Cc: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> > > Cc: Rui Zhang <rui.zhang@intel.com>
> > > Signed-off-by: Chen Yu <yu.c.chen@intel.com>
> > 
> > It will be better to combine this with patch [2/3] IMO, because that makes
> > it clear why the changes in that patch are needed.
> > 
> > Also you can define the new flag in mtrr/main.c, set it in
> > arch_enable_nonboot_cpus_begin() and clear it in
> > arch_enable_nonboot_cpus_end().  It is better to put it into the
> > arch-specific code as the flag itself is arch-specific.
> > 
> > Then, of course, you don't need patch [1/3] and all can be done in one
> >  patch.
> 
> Ok, will rewrite the patch, thanks! 

Just for the record, this series cuts down resume time from system sleep
by 4-5 seconds on my MacBookPro9,1.  Great work, looking forward to this
being respun and merged.

Tested-by: Lukas Wunner <lukas@wunner.de>

Thanks,

Lukas

Re: [PATCH 3/3][RFC/RFT] PM / sleep: Do not delay the synchronization of MTRR during resume

[Yu Chen]

On Tue, Feb 06, 2018 at 03:04:17PM +0100, Lukas Wunner wrote:
> On Thu, Dec 14, 2017 at 12:02:42AM +0800, Yu Chen wrote:
> > On Wed, Dec 13, 2017 at 01:31:50AM +0100, Rafael J. Wysocki wrote:
> > > On Tuesday, October 31, 2017 10:58:50 AM CET Yu Chen wrote:
> [snip]
> > > > In one word, with the patch applied, the cpu online time during resume
> > > > has decreased by about 6 seconds on a bogus MTRR platform, and decreased
> > > > by about 600ms on a 88 cpus Xeon platform after resumed.
> > > > 
> > > > Cc: Len Brown <len.brown@intel.com>
> > > > Cc: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> > > > Cc: Rui Zhang <rui.zhang@intel.com>
> > > > Signed-off-by: Chen Yu <yu.c.chen@intel.com>
> > > 
> > > It will be better to combine this with patch [2/3] IMO, because that makes
> > > it clear why the changes in that patch are needed.
> > > 
> > > Also you can define the new flag in mtrr/main.c, set it in
> > > arch_enable_nonboot_cpus_begin() and clear it in
> > > arch_enable_nonboot_cpus_end().  It is better to put it into the
> > > arch-specific code as the flag itself is arch-specific.
> > > 
> > > Then, of course, you don't need patch [1/3] and all can be done in one
> > >  patch.
> > 
> > Ok, will rewrite the patch, thanks! 
> 
> Just for the record, this series cuts down resume time from system sleep
> by 4-5 seconds on my MacBookPro9,1.  Great work, looking forward to this
> being respun and merged.
> 
> Tested-by: Lukas Wunner <lukas@wunner.de>
>
Thanks Lukas,
I've sent the latest patch at:
https://patchwork.kernel.org/patch/10150077/
> Thanks,
> 
> Lukas