If dom0_vcpus_pin is used, make sure the pinning is only done for
dom0 vcpus, instead of for the hardware domain (which might not be
dom0 at all!).

Suggested-by: Jan Beulich <jbeulich@suse.com>
Signed-off-by: Dario Faggioli <dfaggioli@suse.com>
---
Cc: George Dunlap <george.dunlap@citrix.com>
---
Difference from "RFC" [1]:
- new patch

[1] https://lore.kernel.org/xen-devel/e061a647cd77a36834e2085a96a07caa785c5066.camel@suse.com/
---
 xen/common/sched/core.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/xen/common/sched/core.c b/xen/common/sched/core.c
index f689b55783..379a791d62 100644
--- a/xen/common/sched/core.c
+++ b/xen/common/sched/core.c
@@ -575,7 +575,7 @@ int sched_init_vcpu(struct vcpu *v)
      * Initialize affinity settings. The idler, and potentially
      * domain-0 VCPUs, are pinned onto their respective physical CPUs.
      */
-    if ( is_idle_domain(d) || (is_hardware_domain(d) && opt_dom0_vcpus_pin) )
+    if ( is_idle_domain(d) || (is_control_domain(d) && opt_dom0_vcpus_pin) )
         sched_set_affinity(unit, cpumask_of(processor), &cpumask_all);
     else
         sched_set_affinity(unit, &cpumask_all, &cpumask_all);

On 02.08.2022 15:51, Dario Faggioli wrote:
> If dom0_vcpus_pin is used, make sure the pinning is only done for
> dom0 vcpus, instead of for the hardware domain (which might not be
> dom0 at all!).

Hmm, but the control domain may not be either, as it's derived from
d->is_privileged. I think ...

> --- a/xen/common/sched/core.c
> +++ b/xen/common/sched/core.c
> @@ -575,7 +575,7 @@ int sched_init_vcpu(struct vcpu *v)
>       * Initialize affinity settings. The idler, and potentially
>       * domain-0 VCPUs, are pinned onto their respective physical CPUs.
>       */
> -    if ( is_idle_domain(d) || (is_hardware_domain(d) && opt_dom0_vcpus_pin) )
> +    if ( is_idle_domain(d) || (is_control_domain(d) && opt_dom0_vcpus_pin) )

... for it to be strictly only Dom0, you want to check d->domain_id here.

Or else I guess the description wants adjusting.

Jan

On Tue, 2022-08-02 at 16:56 +0200, Jan Beulich wrote:
> On 02.08.2022 15:51, Dario Faggioli wrote:
> > If dom0_vcpus_pin is used, make sure the pinning is only done for
> > dom0 vcpus, instead of for the hardware domain (which might not be
> > dom0 at all!).
> 
> Hmm, but the control domain may not be either, as it's derived from
> d->is_privileged. I think ...
> 
Mmm... Right.

> > --- a/xen/common/sched/core.c
> > +++ b/xen/common/sched/core.c
> > @@ -575,7 +575,7 @@ int sched_init_vcpu(struct vcpu *v)
> >       * Initialize affinity settings. The idler, and potentially
> >       * domain-0 VCPUs, are pinned onto their respective physical
> > CPUs.
> >       */
> > -    if ( is_idle_domain(d) || (is_hardware_domain(d) &&
> > opt_dom0_vcpus_pin) )
> > +    if ( is_idle_domain(d) || (is_control_domain(d) &&
> > opt_dom0_vcpus_pin) )
> 
> ... for it to be strictly only Dom0, you want to check d->domain_id
> here.
> 
Ok, I'll send an update that does that.

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)

On 02.08.2022 18:08, Dario Faggioli wrote:
> On Tue, 2022-08-02 at 16:56 +0200, Jan Beulich wrote:
>> On 02.08.2022 15:51, Dario Faggioli wrote:
>>> If dom0_vcpus_pin is used, make sure the pinning is only done for
>>> dom0 vcpus, instead of for the hardware domain (which might not be
>>> dom0 at all!).
>>
>> Hmm, but the control domain may not be either, as it's derived from
>> d->is_privileged. I think ...
>>
> Mmm... Right.
> 
>>> --- a/xen/common/sched/core.c
>>> +++ b/xen/common/sched/core.c
>>> @@ -575,7 +575,7 @@ int sched_init_vcpu(struct vcpu *v)
>>>       * Initialize affinity settings. The idler, and potentially
>>>       * domain-0 VCPUs, are pinned onto their respective physical
>>> CPUs.
>>>       */
>>> -    if ( is_idle_domain(d) || (is_hardware_domain(d) &&
>>> opt_dom0_vcpus_pin) )
>>> +    if ( is_idle_domain(d) || (is_control_domain(d) &&
>>> opt_dom0_vcpus_pin) )
>>
>> ... for it to be strictly only Dom0, you want to check d->domain_id
>> here.
>>
> Ok, I'll send an update that does that.

Well - if you agree, I'd be happy to make the change while committing
and then adding
Reviewed-by: Jan Beulich <jbeulich@suse.com>

Jan

On Wed, 2022-08-03 at 08:19 +0200, Jan Beulich wrote:
> On 02.08.2022 18:08, Dario Faggioli wrote:
> > > ... for it to be strictly only Dom0, you want to check d-
> > > >domain_id
> > > here.
> > > 
> > Ok, I'll send an update that does that.
> 
> Well - if you agree, I'd be happy to make the change while committing
> and then adding
>
Thanks for offering that!

Since there were a couple of things to fix, I've just sent a v2, which
hopefully handles all of them. I hope this makes things easier. :-)

> Reviewed-by: Jan Beulich <jbeulich@suse.com>
> 
Great, added to v2.

Thanks again 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)