Re: [PATCH 2/5] xen/sched: rtds: enforce admission control in xl sched-rtds

[Juergen Gross <jgross@xxxxxxxx>]

On 26.08.26 06:57, Furkan Caliskan wrote:
> Now that we have introduced admission control for new and
> removed units. Extend it to XEN_DOMCTL_SCHEDOP_putinfo and
> putvcpuinfo, so growing an existing reservation via
> xl sched-rtds is checked too.
>
> putinfo sets the same (period, budget) for every unit of a
> domain at once, so it tests the whole domain's utilization
> delta atomically, rather than unit-by-unit, which could
> spuriously reject an overall-acceptable change depending on
> iteration order.
>
> putvcpuinfo changes one unit at a time, so it just calls
> rt_admission_test() directly.
>
> Signed-off-by: Furkan Caliskan <frn1furkan10@xxxxxxxxx>
> ---
>   xen/common/sched/rt.c | 42 ++++++++++++++++++++++++++++++++++++++++++
>   1 file changed, 42 insertions(+)
>
> diff --git a/xen/common/sched/rt.c b/xen/common/sched/rt.c
> index 9126320801..9643a277fe 100644
> --- a/xen/common/sched/rt.c
> +++ b/xen/common/sched/rt.c
> @@ -1527,11 +1527,43 @@ rt_dom_cntl(
>           op->u.rtds.budget = RTDS_DEFAULT_BUDGET / MICROSECS(1);
>           break;
>       case XEN_DOMCTL_SCHEDOP_putinfo:
> +    {
> +        uint64_t dom_old_util = 0, new_util, new_total;
> +        unsigned int nr_units = 0;
> +
>           rc = rt_validate_params(&op->u.rtds, &period, &budget);
>           if ( rc )
>               break;
>   
> +        new_util = rt_unit_utilization(period, budget);
> +
>           spin_lock_irqsave(&prv->lock, flags);
> +
> +        /*
> +         * Same (period, budget) for every unit of d: test and commit
> +         * the domain's whole utilization delta atomically, rather
> +         * than unit-by-unit, which could spuriously reject an
> +         * overall-acceptable change depending on iteration order.
> +         */
> +        for_each_sched_unit ( d, unit )
> +        {
> +            svc = rt_unit(unit);
> +            dom_old_util += rt_unit_utilization(svc->period, svc->budget);
> +            nr_units++;
> +        }
> +
> +        new_total = prv->utilization - dom_old_util +
> +                    (uint64_t)nr_units * new_util;
> +
> +        if ( new_total > prv->utilization && new_total > rt_utilization_cap(d) 
> )
> +        {
> +            rc = -EINVAL;
Is EINVAL a good choice here? I think a different error code might be wanted.

> +            spin_unlock_irqrestore(&prv->lock, flags);
> +            break;
> +        }
> +
> +        prv->utilization = new_total;
> +
>           for_each_sched_unit ( d, unit )
>           {
>               svc = rt_unit(unit);
> @@ -1540,6 +1572,7 @@ rt_dom_cntl(
>           }
>           spin_unlock_irqrestore(&prv->lock, flags);
>           break;
> +    }
>       case XEN_DOMCTL_SCHEDOP_getvcpuinfo:
>       case XEN_DOMCTL_SCHEDOP_putvcpuinfo:
>           while ( index < op->u.v.nr_vcpus )
> @@ -1584,6 +1617,15 @@ rt_dom_cntl(
>   
>                   spin_lock_irqsave(&prv->lock, flags);
>                   svc = rt_unit(d->vcpu[local_sched.vcpuid]->sched_unit);
> +
> +                rc = rt_admission_test(prv, d, svc->period, svc->budget,
> +                                        period, budget);
> +                if ( rc )
> +                {
> +                    spin_unlock_irqrestore(&prv->lock, flags);
> +                    break;
> +                }
> +
>                   svc->period = period;
>                   svc->budget = budget;
>                   if ( local_sched.u.rtds.flags & XEN_DOMCTL_SCHEDRT_extra )

Juergen

Attachment: OpenPGP_0xB0DE9DD628BF132F.asc
Description: OpenPGP public key

Attachment: OpenPGP_signature.asc
Description: OpenPGP digital signature