Re: [PATCH v3 1/8] xen/cpupool: support moving domain between cpupools with different granularity

[Dario Faggioli <dfaggioli@xxxxxxxx>]

On Wed, 2020-12-09 at 17:09 +0100, Juergen Gross wrote:
> When moving a domain between cpupools with different scheduling
> granularity the sched_units of the domain need to be adjusted.
> 
> Do that by allocating new sched_units and throwing away the old ones
> in sched_move_domain().
> 
> Signed-off-by: Juergen Gross <jgross@xxxxxxxx>
>
This looks fine, and can have:

Reviewed-by: Dario Faggioli <dfaggioli@xxxxxxxx>

I would only have one request. It's not a huge deal, and probably not
worth a resend only for that, but if either you or the committer are up
for complying with that in whatever way you find the most suitable,
that would be great.

I.e., can we...
> ---
>  xen/common/sched/core.c | 121 ++++++++++++++++++++++++++++++--------
> --
>  1 file changed, 90 insertions(+), 31 deletions(-)
> 
> diff --git a/xen/common/sched/core.c b/xen/common/sched/core.c
> index a429fc7640..2a61c879b3 100644
> --- a/xen/common/sched/core.c
> +++ b/xen/common/sched/core.c
> 
> [...]
> -    old_ops = dom_scheduler(d);
>      old_domdata = d->sched_priv;
> 
Move *here* (i.e., above this new call to cpumask_first()) the comment
that is currently inside the loop?
>  
> +    new_p = cpumask_first(d->cpupool->cpu_valid);
>      for_each_sched_unit ( d, unit )
>      {
> +        spinlock_t *lock;
> +
> +        /*
> +         * Temporarily move all units to same processor to make
> locking
> +         * easier when moving the new units to the new processors.
> +         */
>
This one here, basically ^^^

> +        lock = unit_schedule_lock_irq(unit);
> +        sched_set_res(unit, get_sched_res(new_p));
> +        spin_unlock_irq(lock);
> +
>          sched_remove_unit(old_ops, unit);
>      }
>  
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