Re: [Xen-devel] [PATCH v3 37/47] xen/sched: move per-cpu variable scheduler to struct sched_resource

[Jan Beulich <jbeulich@xxxxxxxx>]

On 14.09.2019 10:52, Juergen Gross wrote:
> --- a/xen/common/sched_credit.c
> +++ b/xen/common/sched_credit.c
> @@ -352,9 +352,10 @@ DEFINE_PER_CPU(unsigned int, last_tickle_cpu);
>  static inline void __runq_tickle(struct csched_unit *new)
>  {
>      unsigned int cpu = sched_unit_cpu(new->unit);
> +    struct sched_resource *sd = get_sched_res(cpu);
>      struct sched_unit *unit = new->unit;
>      struct csched_unit * const cur = CSCHED_UNIT(curr_on_cpu(cpu));
> -    struct csched_private *prv = CSCHED_PRIV(per_cpu(scheduler, cpu));
> +    struct csched_private *prv = CSCHED_PRIV(sd->scheduler);

Is the single use "sd" (and, as mentioned elsewhere, mis-named)
really worth it? (Applicable further down as well.)

> @@ -931,7 +932,8 @@ csched_unit_acct(struct csched_private *prv, unsigned int 
> cpu)
>  {
>      struct sched_unit *currunit = current->sched_unit;
>      struct csched_unit * const svc = CSCHED_UNIT(currunit);
> -    const struct scheduler *ops = per_cpu(scheduler, cpu);
> +    struct sched_resource *sd = get_sched_res(cpu);
> +    const struct scheduler *ops = sd->scheduler;
>  
>      ASSERT( sched_unit_cpu(currunit) == cpu );
>      ASSERT( svc->sdom != NULL );
> @@ -987,8 +989,7 @@ csched_unit_acct(struct csched_private *prv, unsigned int 
> cpu)
>               * idlers. But, if we are here, it means there is someone running
>               * on it, and hence the bit must be zero already.
>               */
> -            ASSERT(!cpumask_test_cpu(cpu,
> -                                     CSCHED_PRIV(per_cpu(scheduler, 
> cpu))->idlers));
> +            ASSERT(!cpumask_test_cpu(cpu, 
> CSCHED_PRIV(sd->scheduler)->idlers));

While at the first glance "sd" is used twice here, this 2nd use is
actually unnecessary - "cpu" hasn't changed from the beginning of
the function afaics, and hence "ops" could be used here.

Preferably with the suggested adjustments (but final judgement is
with the scheduler maintainers anyway)
Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>

Jan

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