From mboxrd@z Thu Jan 1 00:00:00 1970 From: Juergen Gross Subject: Re: [PATCH 3/4] xen: sched: reorganize cpu_disable_scheduler() Date: Thu, 09 Jul 2015 12:45:05 +0200 Message-ID: <559E50B1.9070406@suse.com> References: <20150703152743.23194.15530.stgit@Solace.station> <20150703154930.23194.20319.stgit@Solace.station> <559BB4FC.1070106@suse.com> <1436368416.22672.220.camel@citrix.com> <1436437493.22672.308.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 1ZD9KI-00082D-3B for xen-devel@lists.xenproject.org; Thu, 09 Jul 2015 10:45:10 +0000 In-Reply-To: <1436437493.22672.308.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/09/2015 12:24 PM, Dario Faggioli wrote: > Hey, > > 1 thing... > > On Wed, 2015-07-08 at 17:13 +0200, Dario Faggioli wrote: >> On Tue, 2015-07-07 at 13:16 +0200, Juergen Gross wrote: > >>>> @@ -645,25 +675,72 @@ 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; >>>> } >>>> - else >>>> - vcpu_schedule_unlock_irqrestore(lock, flags, v); >>>> >>>> /* >>>> - * A vcpu active in the hypervisor will not be migratable. >>>> - * The caller should try again after releasing and reaquiring >>>> - * all locks. >>>> + * If we're here, it means that the vcpu is on cpu. Let's see how >>>> + * it's best to send it away, depending on whether we are shutting >>>> + * down/suspending, or doing cpupool manipulations. >>>> */ >>>> - if ( v->processor == cpu ) >>>> - ret = -EAGAIN; >>>> - } >>>> + set_bit(_VPF_migrating, &v->pause_flags); >>>> + vcpu_schedule_unlock_irqrestore(lock, flags, v); >>>> + vcpu_sleep_nosync(v); >>>> + >>>> + /* >>>> + * In case of 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, when dealing a cpupool with only non-boot pcpus, >>>> + * as the scheduler will always fail to send the vcpus away >>>> + * from the last online (non boot) pcpu! >>> >>> I'd add a comment that in shutdown/suspend case all domains are being >>> paused, so we can be active in dom0/Pool-0 only. >>> >> Sure, I'll add this. >> > ...while putting such a comment together, I'm realizing that I'm not > sure about what you meant, or what you wanted the comment itself to > express. > > I mean, it is certainly true that all domains are being paused (they've > been paused already, actually), but that include Dom0 too. Also, we are > in Xen, in stop_machine context, so I'm not sure what you meant either > with "we can be active in dom0/Pool-0 only". We are running on the vcpu which issued the hypercall resulting in pausing the domains. A vcpu can't pause itself. > So, I'm adding a line about things being paused. If you think I should > say anything more than that, let me know. I think dom0/Pool-0 should be mentioned. The coding is written with this assumption so it should be documented. Juergen