Re: [Xen-devel] [PATCH v3 30/47] xen/sched: add support for multiple vcpus per sched unit where missing

[Dario Faggioli <dfaggioli@xxxxxxxx>]

On Wed, 2019-09-25 at 15:07 +0200, Jan Beulich wrote:
> On 25.09.2019 14:40, Jürgen Groß  wrote:
> > On 24.09.19 17:22, Jan Beulich wrote:
> > > On 24.09.2019 17:09, Jürgen Groß wrote:
> > > > On 24.09.19 17:00, Jan Beulich wrote:
> > > > > On 24.09.2019 16:41, Jürgen Groß wrote:
> > > > > > for_each_sched_unit_vcpu() for an idle
> > > > > > unit might end premature when one of the vcpus is running
> > > > > > in another
> > > > > > unit (idle_vcpu->sched_unit != idle_unit).
> > > > > 
> > > > > Oh, that (v)->sched_unit == (i) in the construct is clearly
> > > > > unexpected.
> > > > > Is this really still needed by the end of the series? I
> > > > > realize that
> > > > > _some_ check is needed, but could this perhaps be arranged in
> > > > > a way
> > > > > that you'd still hit all vCPU-s when using it on an idle
> > > > > unit, no
> > > > > matter whether they're in use as a substitute in a "real"
> > > > > unit?
> > > > 
> > > > I could do that by having another linked list in struct vcpu.
> > > > This way
> > > > I can avoid it.
> > > 
> > > Oh, no, not another list just for this purpose. I was rather
> > > thinking
> > > of e.g. a comparison of IDs.
> > 
> > That would result either in something like:
> > 
> > (v)->vcpu_id < (u)->unit_id + (u)->res->cpupool->granularity
> > 
> > requiring to make struct sched_resource public as keyhandler.c
> > needs
> > for_each_sched_unit_vcpu() plus being quite expensive,
> 
> I agree this is not a good option.
> 
> > or:
> > 
> > !(u)->next_in_list || (v)->vcpu_id < (u)->next_in_list->unit_id
> > 
> > which seems to be more expensive as the current variant, too.
> 
> It's not this much more expensive, and it eliminates unexpected
> (as I would call it) behavior, so I think I'd go this route. 
>
So, I honestly like the way it's currently done in Juergen's pathes.

However, I'm not sure I understand what it is the issue that Jan thinks
that has, and in what sense the code/behavior is regarded as
"unexpected".

Can you help me see the problem? Maybe, if I realize it, I'd change my
preference...

Thanks and Regards
-- 
Dario Faggioli, Ph.D
http://about.me/dario.faggioli
Virtualization Software Engineer
SUSE Labs, SUSE https://www.suse.com/
-------------------------------------------------------------------
<<This happens because _I_ choose it to happen!>> (Raistlin Majere)

Attachment: signature.asc
Description: This is a digitally signed message part

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