Re: [Xen-devel] [PATCH v2 2/9] xen/sched: make sched-if.h really scheduler private

[Jürgen Groß <jgross@xxxxxxxx>]

On 14.01.20 15:27, Jan Beulich wrote:
> On 08.01.2020 16:23, Juergen Gross wrote:
> > @@ -234,16 +233,6 @@ void domctl_lock_release(void)
> >       spin_unlock(&current->domain->hypercall_deadlock_mutex);
> >   }
> >   
> > -static inline
> > -int vcpuaffinity_params_invalid(const struct xen_domctl_vcpuaffinity *vcpuaff)
> > -{
> > -    return vcpuaff->flags == 0 ||
> > -           ((vcpuaff->flags & XEN_VCPUAFFINITY_HARD) &&
> > -            guest_handle_is_null(vcpuaff->cpumap_hard.bitmap)) ||
> > -           ((vcpuaff->flags & XEN_VCPUAFFINITY_SOFT) &&
> > -            guest_handle_is_null(vcpuaff->cpumap_soft.bitmap));
> > -}
> I'd like to suggest keeping this and ...
>
> > @@ -608,122 +597,8 @@ long do_domctl(XEN_GUEST_HANDLE_PARAM(xen_domctl_t) 
> > u_domctl)
> >   
> >       case XEN_DOMCTL_setvcpuaffinity:
> >       case XEN_DOMCTL_getvcpuaffinity:
> > -    {
> > -        struct vcpu *v;
> > -        const struct sched_unit *unit;
> > -        struct xen_domctl_vcpuaffinity *vcpuaff = &op->u.vcpuaffinity;
> > -
> > -        ret = -EINVAL;
> > -        if ( vcpuaff->vcpu >= d->max_vcpus )
> > -            break;
> > -
> > -        ret = -ESRCH;
> > -        if ( (v = d->vcpu[vcpuaff->vcpu]) == NULL )
> > -            break;
> > -
> > -        unit = v->sched_unit;
> > -        ret = -EINVAL;
> > -        if ( vcpuaffinity_params_invalid(vcpuaff) )
> > -            break;
> ... everything up to here (except the [too early] unit assignment),
> as not being scheduler specific at all. The remainder then would
> better become two distinct functions, eliminating the need to pass
> op->cmd (and presumably passing "v" instead of "d"). If, otoh, the
> decision (supported by others) is to move everything, then I think
> it would be appropriate to make at least some adjustments: The code
> above should be converted to use domain_vcpu(), and e.g. ...
Either would be fine with me.

> > -        if ( op->cmd == XEN_DOMCTL_setvcpuaffinity )
> > -        {
> > -            cpumask_var_t new_affinity, old_affinity;
> > -            cpumask_t *online = cpupool_domain_master_cpumask(v->domain);
> ... this should use "d".
Yes.

> > @@ -875,6 +876,16 @@ int cpupool_do_sysctl(struct xen_sysctl_cpupool_op *op)
> >       return ret;
> >   }
> >   
> > +int cpupool_get_id(const struct domain *d)
> I find plain int odd for something like an ID, but I can see why
> this is.
>
> > +cpumask_t *cpupool_valid_cpus(struct cpupool *pool)
> const twice?
See patch 9.


Juergen

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