From mboxrd@z Thu Jan 1 00:00:00 1970 From: Juergen Gross Subject: Re: [PATCH v2 1/2] xen: sched: reorganize cpu_disable_scheduler() Date: Mon, 20 Jul 2015 14:06:13 +0200 Message-ID: <55ACE435.6040308@suse.com> References: <20150717133013.29612.53960.stgit@Solace.station> <20150717133554.29612.14835.stgit@Solace.station> <55ACDE7D.50108@suse.com> <1437393585.5036.8.camel@citrix.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: Received: from mail6.bemta14.messagelabs.com ([193.109.254.103]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1ZH9pn-000351-Pd for xen-devel@lists.xenproject.org; Mon, 20 Jul 2015 12:06:15 +0000 In-Reply-To: <1437393585.5036.8.camel@citrix.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Dario Faggioli Cc: George Dunlap , xen-devel@lists.xenproject.org List-Id: xen-devel@lists.xenproject.org On 07/20/2015 01:59 PM, Dario Faggioli wrote: > On Mon, 2015-07-20 at 13:41 +0200, Juergen Gross wrote: >> On 07/17/2015 03:35 PM, Dario Faggioli wrote: > >>> @@ -644,25 +673,66 @@ int cpu_disable_scheduler(unsigned int cpu) >>> cpumask_setall(v->cpu_hard_affinity); >>> } >>> >>> - if ( v->processor == cpu ) >>> + if ( v->processor != cpu ) >>> { >>> - set_bit(_VPF_migrating, &v->pause_flags); >>> + /* This vcpu is not on cpu, so we can move on. */ >>> vcpu_schedule_unlock_irqrestore(lock, flags, v); >>> - vcpu_sleep_nosync(v); >>> - vcpu_migrate(v); >>> + continue; >>> + } >>> + >>> + /* If it is on cpu, we must send it away. */ >>> + if ( unlikely(system_state == SYS_STATE_suspend) ) >>> + { >>> + /* >>> + * If we are doing a shutdown/suspend, it is not necessary to >>> + * ask the scheduler to chime in. In fact: >>> + * * there is no reason for it: the end result we are after >>> + * is just 'all the vcpus on the boot pcpu, and no vcpu >>> + * anywhere else', so let's just go for it; >>> + * * it's wrong, for cpupools with only non-boot pcpus, as >>> + * the scheduler would always fail to send the vcpus away >>> + * from the last online (non boot) pcpu! >>> + * >>> + * Therefore, in the shutdown/suspend case, we just pick up >>> + * one (still) online pcpu. Note that, at this stage, all >>> + * domains (including dom0) have been paused already, so we >>> + * do not expect any vcpu activity at all. >>> + */ >>> + cpumask_andnot(&online_affinity, &cpu_online_map, >>> + cpumask_of(cpu)); >>> + BUG_ON(cpumask_empty(&online_affinity)); >>> + /* >>> + * As boot cpu is, usually, pcpu #0, using cpumask_first() >>> + * will make us converge quicker. >>> + */ >>> + new_cpu = cpumask_first(&online_affinity); >>> + vcpu_move_nosched(v, new_cpu); >> >> Shouldn't there be a vcpu_schedule_unlock_irqrestore() ? >> > I'm sure I put one there, as I was sure that it was there the last time > I inspected the patch before hitting send. > > But I see that it's not there now, so I must have messed up when > formatting the patch, or something like that. :-( > > It's really really weird, as I forgot it during development, and then > the system was hanging, and then I added it, and that's why I'm sure I > did have it in place... but perhaps I fat fingered some stgit command > which made it disappear. Or you forgot stg refresh? I just managed to do so. :-( Juergen