Re: [Xen-devel] [PATCH] x86/S3: Fix cpu pool scheduling after suspend/resume

[Juergen Gross <juergen.gross@xxxxxxxxxxxxxx>]

On 09.04.2013 14:46, Ben Guthro wrote:
> This review is another S3 scheduler problem with the system_state variable 
> introduced with the following changeset:
> http://xenbits.xen.org/gitweb/?p=xen.git;a=commit;h=269f543ea750ed567d18f2e819e5d5ce58eda5c5
>
> Specifically, the cpu_callback function that takes the CPU down during suspend, 
> and back up during resume.
> We were seeing situations where, after S3, only CPU0 was in cpupool0. Guest 
> performance suffered greatly, since all vcpus were only on a single pcpu. 
> Guests under high CPU load showed the problem much more quickly than an idle 
> guest.
>
> Removing this if condition forces the CPUs to go through the expected 
> online/offline state, and be properly scheduled after S3.
>
> This also includes a necessary partial change proposed earlier by Tomasz 
> Wroblewski here:
> http://lists.xen.org/archives/html/xen-devel/2013-01/msg02206.html
>
> It should also resolve the issues discussed in this thread:
> http://lists.xen.org/archives/html/xen-devel/2012-11/msg01801.html
>
> Signed-off-by: Ben Guthro<benjamin.guthro@xxxxxxxxxx>
> ---
>   xen/common/cpu.c     |    3 +++
>   xen/common/cpupool.c |    5 -----
>   2 files changed, 3 insertions(+), 5 deletions(-)
>
> diff --git a/xen/common/cpu.c b/xen/common/cpu.c
> index 630881e..e20868c 100644
> --- a/xen/common/cpu.c
> +++ b/xen/common/cpu.c
> @@ -5,6 +5,7 @@
>   #include<xen/init.h>
>   #include<xen/sched.h>
>   #include<xen/stop_machine.h>
> +#include<xen/sched-if.h>
>
>   unsigned int __read_mostly nr_cpu_ids = NR_CPUS;
>   #ifndef nr_cpumask_bits
> @@ -212,6 +213,8 @@ void enable_nonboot_cpus(void)
>               BUG_ON(error == -EBUSY);
>               printk("Error taking CPU%d up: %d\n", cpu, error);
>           }
> +        if (system_state == SYS_STATE_resume)
> +            cpumask_set_cpu(cpu, cpupool0->cpu_valid);
This might solve YOUR problem, but reintroduces the problem why the original
change was done: ALL cpus will be in cpupool0 after resume!

So: NAK


Juergen

--
Juergen Gross                 Principal Developer Operating Systems
PBG PDG ES&S SWE OS6                   Telephone: +49 (0) 89 3222 2967
Fujitsu Technology Solutions              e-mail: juergen.gross@xxxxxxxxxxxxxx
Domagkstr. 28                           Internet: ts.fujitsu.com
D-80807 Muenchen                 Company details: ts.fujitsu.com/imprint.html

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel