Re: [Xen-devel] [PATCH v2 30/48] xen/sched: introduce unit_runnable_state()

From: Juergen Gross <jgross@suse.com>
To: Dario Faggioli <dfaggioli@suse.com>, xen-devel@lists.xenproject.org
Cc: Stefano Stabellini <sstabellini@kernel.org>, Wei Liu <wl@xen.org>,
	Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>,
	George Dunlap <George.Dunlap@eu.citrix.com>,
	Andrew Cooper <andrew.cooper3@citrix.com>,
	Ian Jackson <ian.jackson@eu.citrix.com>,
	Robert VanVossen <robert.vanvossen@dornerworks.com>,
	Tim Deegan <tim@xen.org>, Julien Grall <julien.grall@arm.com>,
	Josh Whitehead <josh.whitehead@dornerworks.com>,
	Meng Xu <mengxu@cis.upenn.edu>, Jan Beulich <jbeulich@suse.com>
Subject: Re: [Xen-devel] [PATCH v2 30/48] xen/sched: introduce unit_runnable_state()
Date: Fri, 13 Sep 2019 16:14:43 +0200	[thread overview]
Message-ID: <04990a04-9946-4796-66be-6b348d3f28e7@suse.com> (raw)
In-Reply-To: <08af44affecf2f635afc0ff1e1f377651866d78b.camel@suse.com>

On 12.09.19 12:24, Dario Faggioli wrote:
> On Fri, 2019-08-09 at 16:58 +0200, Juergen Gross wrote:
>> Today the vcpu runstate of a new scheduled vcpu is always set to
>> "running" even if at that time vcpu_runnable() is already returning
>> false due to a race (e.g. with pausing the vcpu).
>>
>> With core scheduling this can no longer work as not all vcpus of a
>> schedule unit have to be "running" when being scheduled. So the
>> vcpu's
>> new runstate has to be selected at the same time as the runnability
>> of
>> the related schedule unit is probed.
>>
> So, you're saying that this patch is meant at fixing a race. A race
> that, if I understood correctly, it's not there/it's not problematic
> right now, but will be when we have more than 1 vcpu in a unit and we
> enable core scheduling. Is this the case?

Correct.

> 
> If yes:
> - this very patch, at this point in the series, is basically
> introducing no functional (or at least behavioral) change, is this
> right too?

Yes.

> - can you provide some more detail about the race. When/how it occurs
> and how the changes done in credit and rt fix it?

The problem is that with more than one vcpu in a unit you can no longer
tell the new run_state from the mere fact that the unit has been
selected to run next. So testing e.g. vcpu_runnable() after selecting
a unit for running could result in all vcpus not being runnable any
longer (e.g. when a domain was paused via domain_pause_nosync()
after selecting the next unit, but before setting any vcpu's run_state).

In order to avoid that problem the new run_state of the vcpus of a unit
needs to be determined when doing the decision which unit to select.

Juergen

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel