Re: [Xen-devel] [PATCH v4 10/15] xen: sched: DOMCTL_*vcpuaffinity works with hard and soft affinity

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 22.11.13 at 19:57, Dario Faggioli <dario.faggioli@xxxxxxxxxx> wrote:
> +            /*
> +             * Report back to the caller what the "effective affinity", that
> +             * is the intersection of cpupool's pcpus, the (new?) hard
> +             * affinity and the (new?) soft-affinity.
> +             */
> +            if ( !guest_handle_is_null(op->u.vcpuaffinity.eff_cpumap.bitmap) 
> )
>              {
> -                ret = vcpu_set_affinity(v, new_affinity);
> -                free_cpumask_var(new_affinity);
> +                online = cpupool_online_cpumask(v->domain->cpupool);
> +                cpumask_and(new_affinity, online, v->cpu_hard_affinity);
> +                if ( op->u.vcpuaffinity.flags & XEN_VCPUAFFINITY_SOFT)
> +                    cpumask_and(new_affinity, new_affinity,
> +                                v->cpu_soft_affinity);
> +
> +                ret = cpumask_to_xenctl_bitmap(
> +                    &op->u.vcpuaffinity.eff_cpumap, new_affinity);

So with both flags set, how is the caller supposed to know what
hard affinity is now in effect? I said on the previous version already
that with you _having_ two CPU masks, you should return both.

>          else
>          {
> +            cpumask_var_t affinity;
> +
> +            /*
> +             * If the caller asks for both _HARD and _SOFT, what we return
> +             * is the intersection of hard and soft affinity for the vcpu.
> +             */
> +            if ( !alloc_cpumask_var(&affinity) )
> +            {
> +                ret = -ENOMEM;
> +                break;
> +            }
> +            cpumask_setall(affinity);
> +
> +            if ( op->u.vcpuaffinity.flags == 0 )
> +            {
> +                ret = -EINVAL;
> +                free_cpumask_var(affinity);
> +                break;
> +            }
> +            if ( op->u.vcpuaffinity.flags & XEN_VCPUAFFINITY_HARD )
> +                cpumask_copy(affinity, v->cpu_hard_affinity);
> +            if ( op->u.vcpuaffinity.flags & XEN_VCPUAFFINITY_SOFT )
> +                cpumask_and(affinity, affinity, v->cpu_soft_affinity);
> +
>              ret = cpumask_to_xenctl_bitmap(
> -                &op->u.vcpuaffinity.cpumap, v->cpu_hard_affinity);
> +                &op->u.vcpuaffinity.cpumap, affinity);

Similarly here.

Of course, the name "eff_cpumap" then is likely not really suitable.

> --- a/xen/include/public/domctl.h
> +++ b/xen/include/public/domctl.h
> @@ -300,8 +300,19 @@ DEFINE_XEN_GUEST_HANDLE(xen_domctl_nodeaffinity_t);
>  /* XEN_DOMCTL_setvcpuaffinity */
>  /* XEN_DOMCTL_getvcpuaffinity */
>  struct xen_domctl_vcpuaffinity {
> -    uint32_t  vcpu;              /* IN */
> -    struct xenctl_bitmap cpumap; /* IN/OUT */
> +    /* IN variables. */
> +    uint32_t  vcpu;
> + /* Set/get the hard affinity for vcpu */
> +#define _XEN_VCPUAFFINITY_HARD  0
> +#define XEN_VCPUAFFINITY_HARD   (1U<<_XEN_VCPUAFFINITY_HARD)
> + /* Set/get the soft affinity for vcpu */
> +#define _XEN_VCPUAFFINITY_SOFT  1
> +#define XEN_VCPUAFFINITY_SOFT   (1U<<_XEN_VCPUAFFINITY_SOFT)
> +    uint32_t flags;
> +    /* IN/OUT variables. */

In further revisions, please make these annotations correctly
reflect what you patch does: In its current shape, this is an IN for
set and an OUT for get, ...

> +    struct xenctl_bitmap cpumap;
> +    /* OUT variables. */

... while this is an OUT for set, and unused for get (and I also said
earlier that _if_ this remains unused for one of the two operations,
then sharing the interface structure wouldn't be appropriate
anymore).

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel