From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ben Guthro Subject: Re: [PATCH] x86/S3: Restore broken vcpu affinity on resume Date: Wed, 27 Mar 2013 08:04:13 -0400 Message-ID: <5152E03D.5040404@citrix.com> References: <1364318408-15970-1-git-send-email-benjamin.guthro@citrix.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: George Dunlap Cc: "xen-devel@lists.xen.org" List-Id: xen-devel@lists.xenproject.org On 03/27/2013 08:01 AM, George Dunlap wrote: > On Tue, Mar 26, 2013 at 5:20 PM, Ben Guthro wrote: >> When in SYS_STATE_suspend, and going through the cpu_disable_scheduler >> path, save a copy of the current cpu affinity, and mark a flag to >> restore it later. >> >> Later, in the resume process, when enabling nonboot cpus restore these >> affinities. >> >> This is the second submission of this patch. >> Primary differences from the first patch is to fix formatting problems. >> However, when doing so, I tested with another patch in the >> cpu_disable_scheduler() path that is also appropriate here. >> >> Signed-off-by: Ben Guthro > > Overall looks fine to me; just a few comments below. > >> diff --git a/xen/common/cpupool.c b/xen/common/cpupool.c >> index 10b10f8..7a04f5e 100644 >> --- a/xen/common/cpupool.c >> +++ b/xen/common/cpupool.c >> @@ -19,13 +19,10 @@ >> #include >> #include >> >> -#define for_each_cpupool(ptr) \ >> - for ((ptr) = &cpupool_list; *(ptr) != NULL; (ptr) = &((*(ptr))->next)) >> - > > You're taking this out because it's not used, I presume? > > Since you'll probably be sending another patch anyway (see below), I > think it would be better if you pull this out into a specific > "clean-up" patch. No. This was moved to an h file to allow use elsewhere. I'm in the process of looking into Jan's suggestion of eliminating the need for it by moving some code into thaw_domains() > > >> @@ -569,6 +609,13 @@ int cpu_disable_scheduler(unsigned int cpu) >> { >> printk("Breaking vcpu affinity for domain %d vcpu %d\n", >> v->domain->domain_id, v->vcpu_id); >> + >> + if (system_state == SYS_STATE_suspend) >> + { > > This appears to have two tabs instead of 16 spaces? Yes, I'll fix this in v3. Thanks for your review Ben