Re: [Xen-devel] [PATCH v2 21/48] xen/sched: use sched_resource cpu instead smp_processor_id in schedulers

[Juergen Gross <jgross@xxxxxxxx>]

On 12.09.19 13:46, Jan Beulich wrote:
> On 12.09.2019 13:17, Juergen Gross wrote:
> > On 12.09.19 12:04, Jan Beulich wrote:
> > > On 12.09.2019 11:34, Juergen Gross wrote:
> > > > On 09.09.19 16:17, Jan Beulich wrote:
> > > > > On 09.08.2019 16:58, Juergen Gross wrote:
> > > > > > @@ -1825,8 +1825,9 @@ static struct task_slice
> > > > > >     csched_schedule(
> > > > > >         const struct scheduler *ops, s_time_t now, bool_t 
> > > > > > tasklet_work_scheduled)
> > > > > >     {
> > > > > > -    const int cpu = smp_processor_id();
> > > > > > -    struct list_head * const runq = RUNQ(cpu);
> > > > > > +    const unsigned int cpu = smp_processor_id();
> > > > > > +    const unsigned int sched_cpu = sched_get_resource_cpu(cpu);
> > > > > > +    struct list_head * const runq = RUNQ(sched_cpu);
> > > > > By retaining a local variable named "cpu" you make it close to
> > > > > impossible to notice, during a re-base, an addition to the
> > > > > function still referencing a variable of this name. Similarly
> > > > > review is being made harder because one needs to go hunt all
> > > > > the remaining uses of "cpu". For example there a trace entry
> > > > > being generated, and it's not obvious to me whether this wouldn't
> > > > > better also used sched_cpu.
> > > > Okayy, I'll rename "cpu" to "my_cpu".
> > > We've got a number of instances of "this_cpu" in such cases already,
> > > but no single "my_cpu". May I suggest to stick to this naming here
> > > as well?
> > Hmm, don't you think adding further overloading of "this_cpu" is a bad
> > idea?
> Not at all, no. A function-like macro and a variable of the same
> name will happily coexist.
I am aware that this is working correctly.

I just think such overloading isn't helping for readability and ease
of modification.

In the end I'm not feeling strong here, so in case there are no
objections I'll go with this_cpu.


Juergen

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