Do not delay the MTRR synchronization for

PATCH In review Yu Chen 3 participants 8 messages Oct 31, 2017 – Feb 6, 2018

Subsystems (2)

x86 CPU hotplug

lore

Share & tools

lore

Yu Chen

author

2017-10-31 09:55 UTC #0 lore raw

cover Do not delay the MTRR synchronization for

From: Chen Yu <[email protected]>

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(-)

Signature · 1 line

2.13.5

Yu Chen

author

2017-10-31 09:55 UTC #1 lore raw

PATCH 1/3 PM / sleep: Introduce a flag to identify the enable_nonboot_cpus stage

From: Chen Yu <[email protected]>

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 <[email protected]>

Cc Rafael J. Wysocki <[email protected]>

Cc Rui Zhang <[email protected]>

Signed-off-by Chen Yu <[email protected]>

---
 include/linux/cpu.h | 2 ++
 kernel/cpu.c        | 8 ++++++++
 2 files changed, 10 insertions(+)

include/linux/cpu.h +2 -0

index ca73bc1563f4..f0759d837919 100644
@@ -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);

kernel/cpu.c +8 -0

index 04892a82f6ac..071dcae8b0fb 100644
@@ -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);

Signature · 1 line

2.13.5

Yu Chen

author

2017-10-31 09:58 UTC #2 lore raw

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

In reply to Yu Chen

From: Chen Yu <[email protected]>

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 <[email protected]>

Cc Rafael J. Wysocki <[email protected]>

Cc Rui Zhang <[email protected]>

Signed-off-by Chen Yu <[email protected]>

---
 arch/x86/kernel/cpu/mtrr/main.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

arch/x86/kernel/cpu/mtrr/main.c +7 -3

index 40d5a8a75212..a4e7e23f3c2e 100644
@@ -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,

Signature · 1 line

2.13.5

Yu Chen

author

2017-10-31 09:58 UTC #3 lore raw

PATCH 3/3 PM / sleep: Do not delay the synchronization of MTRR during resume

In reply to Yu Chen

From: Chen Yu <[email protected]>

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 <[email protected]>

Cc Rafael J. Wysocki <[email protected]>

Cc Rui Zhang <[email protected]>

Signed-off-by Chen Yu <[email protected]>

---
 arch/x86/kernel/cpu/mtrr/main.c | 3 +++
 arch/x86/kernel/smpboot.c       | 2 --
 2 files changed, 3 insertions(+), 2 deletions(-)

arch/x86/kernel/cpu/mtrr/main.c +3 -0

index a4e7e23f3c2e..f4c2c60c6ac8 100644
@@ -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;

arch/x86/kernel/smpboot.c +0 -2

index ad59edd84de7..2d1ec878df63 100644
@@ -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();
 }
 
 /*

Signature · 1 line

2.13.5

Rafael J. Wysocki

2017-12-13 00:32 UTC #4 lore raw

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

Quoted 113 lines · CET Yu Chen

From: Chen Yu <[email protected]>
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

Quoted 8 lines

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 <[email protected]>
Cc Rafael J. Wysocki <[email protected]>
Cc Rui Zhang <[email protected]>
Signed-off-by Chen Yu <[email protected]>

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.

Quoted 37 lines

---
 arch/x86/kernel/cpu/mtrr/main.c | 3 +++
 arch/x86/kernel/smpboot.c       | 2 --
 2 files changed, 3 insertions(+), 2 deletions(-)

arch/x86/kernel/cpu/mtrr/main.c +3 -0

index a4e7e23f3c2e..f4c2c60c6ac8 100644
@@ -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;

arch/x86/kernel/smpboot.c +0 -2

index ad59edd84de7..2d1ec878df63 100644
@@ -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();
 }
 
 /*

Yu Chen

author

2017-12-13 16:00 UTC #5 lore raw

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

Quoted 126 lines · Rafael J. Wysocki

Quoted 113 lines · CET Yu Chen
From: Chen Yu <[email protected]>
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
Quoted 8 lines
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 <[email protected]>
Cc Rafael J. Wysocki <[email protected]>
Cc Rui Zhang <[email protected]>
Signed-off-by Chen Yu <[email protected]>
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.

Quoted 7 lines

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!

Lukas Wunner

Tested-by

2018-02-06 14:04 UTC #6 lore raw

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

Quoted 2 lines · Yu Chen

Quoted 1 line · Rafael J. Wysocki
On Tuesday, October 31, 2017 10:58:50 AM CET Yu Chen wrote:

[snip]

Quoted 17 lines

Quoted 17 lines
Quoted 8 lines
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 <[email protected]>
Cc Rafael J. Wysocki <[email protected]>
Cc Rui Zhang <[email protected]>
Signed-off-by Chen Yu <[email protected]>
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.

Quoted 4 lines

Quoted 2 lines
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 <[email protected]>

Thanks,

Lukas

Yu Chen

author

2018-02-06 14:12 UTC #7 lore raw

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

Quoted 24 lines · Lukas Wunner

Quoted 2 lines · Yu Chen
Quoted 1 line · Rafael J. Wysocki
On Tuesday, October 31, 2017 10:58:50 AM CET Yu Chen wrote:
[snip]
Quoted 20 lines
Quoted 19 lines
Quoted 8 lines
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 <[email protected]>
Cc Rafael J. Wysocki <[email protected]>
Cc Rui Zhang <[email protected]>
Signed-off-by Chen Yu <[email protected]>
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.

Quoted 8 lines

Quoted 1 line
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 <[email protected]>

Thanks Lukas,
I've sent the latest patch at:
https://patchwork.kernel.org/patch/10150077/

Quoted 3 lines

Thanks,
Lukas