Re: [Xen-devel] [PATCH v2 24/48] xen: switch from for_each_vcpu() to for_each_sched_unit()

[Juergen Gross <jgross@xxxxxxxx>]

On 12.09.19 16:40, Jan Beulich wrote:
> On 12.09.2019 16:02, Juergen Gross wrote:
> > On 09.09.19 17:14, Jan Beulich wrote:
> > > On 09.08.2019 16:58, Juergen Gross wrote:
> > > > @@ -1002,17 +1032,17 @@ int cpu_disable_scheduler(unsigned int cpu)
> > > >                 *  * the scheduler will always find a suitable solution, or
> > > >                 *    things would have failed before getting in here.
> > > >                 */
> > > > -            vcpu_migrate_start(v);
> > > > +            vcpu_migrate_start(unit->vcpu_list);
> > > >                unit_schedule_unlock_irqrestore(lock, flags, unit);
> > > >    
> > > > -            vcpu_migrate_finish(v);
> > > > +            vcpu_migrate_finish(unit->vcpu_list);
> > > All the ->vcpu_list references look bogus considering where you're
> > > moving, but I can only guess that all of this will need touching
> > > again later in the series. I wonder though whether these wouldn't
> > > better change into for-each-vCPU-in-unit loops right away.
> > Especially the vcpu_migrate part is more complicated. I think it is
> > much easier to review with the more mechanical changes split from the
> > logical changes.
> Yes, and I appreciate you separating mechanical from logical changes.
> However, as already pointed out in the context where I had convinced
> you of using "vcpu_list" as a name, individual actions on vcpu_list
> now look bogus throughout the series. They should really (almost?)
> all be loops over the entire list; I have a hard time imagining
> possible exceptions, but I'm not going to exclude there may be one
> or a few. Introducing such loops should, as long as there's only
> ever on vCPU on such a list, too be a mostly mechanical step, which
> imo should have happened before (or with) changes like this one.
I think the easiest way to handle that is to add a comment like:

/* TODO: switch to for_each_sched_unit_vcpu() */

This will show the need for the loop without having to do logic
changes.


Juergen


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