lkml.org 
[lkml]   [2023]   [Nov]   [8]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH v5 6/7] sched/deadline: Deferrable dl server
    On Wed, Nov 08, 2023 at 09:01:17AM +0100, Daniel Bristot de Oliveira wrote:
    > On 11/8/23 04:20, Joel Fernandes wrote:
    > > Hi Daniel,
    > >
    > > On Tue, Nov 7, 2023 at 1:50 PM Daniel Bristot de Oliveira
    > > <bristot@kernel.org> wrote:
    > >>
    > >>> The code is not doing what I intended because I thought it was doing overload
    > >>> control on the replenishment, but it is not (my bad).
    > >>>
    > >>
    > >> I am still testing but... it is missing something like this (famous last words).
    > >>
    > >> diff --git a/kernel/sched/deadline.c b/kernel/sched/deadline.c
    > >> index 1092ca8892e0..6e2d21c47a04 100644
    > >> --- a/kernel/sched/deadline.c
    > >> +++ b/kernel/sched/deadline.c
    > >> @@ -842,6 +842,8 @@ static inline void setup_new_dl_entity(struct sched_dl_entity *dl_se)
    > >> * runtime, or it just underestimated it during sched_setattr().
    > >> */
    > >> static int start_dl_timer(struct sched_dl_entity *dl_se);
    > >> +static bool dl_entity_overflow(struct sched_dl_entity *dl_se, u64 t);
    > >> +
    > >> static void replenish_dl_entity(struct sched_dl_entity *dl_se)
    > >> {
    > >> struct dl_rq *dl_rq = dl_rq_of_se(dl_se);
    > >> @@ -852,9 +854,18 @@ static void replenish_dl_entity(struct sched_dl_entity *dl_se)
    > >> /*
    > >> * This could be the case for a !-dl task that is boosted.
    > >> * Just go with full inherited parameters.
    > >> + *
    > >> + * Or, it could be the case of a zerolax reservation that
    > >> + * was not able to consume its runtime in background and
    > >> + * reached this point with current u > U.
    > >> + *
    > >> + * In both cases, set a new period.
    > >> */
    > >> - if (dl_se->dl_deadline == 0)
    > >> - replenish_dl_new_period(dl_se, rq);
    > >> + if (dl_se->dl_deadline == 0 ||
    > >> + (dl_se->dl_zerolax_armed && dl_entity_overflow(dl_se, rq_clock(rq)))) {
    > >> + dl_se->deadline = rq_clock(rq) + pi_of(dl_se)->dl_deadline;
    > >> + dl_se->runtime = pi_of(dl_se)->dl_runtime;
    > >> + }
    > >>
    > >> if (dl_se->dl_yielded && dl_se->runtime > 0)
    > >> dl_se->runtime = 0;
    > >
    > > I was wondering does this mean GRUB needs to be enabled? Otherwise I
    > > can see that "runtime / (deadline - t) > dl_runtime / dl_deadline"
    > > will be true almost all the time due to the constraint of executing at
    > > the 0-lax time.
    >
    > No grub needed. It will only happen if the fair server did not have any chance to run.
    >
    > If it happens, it is not a problem, see that timeline I replied in the previous
    > email.

    Ah you're right, I mistakenly read your diff assuming you were calling
    replenish_dl_new_period() on dl_entity_overflow(). Indeed the diff is needed
    (I was actually wondering about why that was not done in my initial review as
    well -- so its good we found it in discussion).

    > We do not want a zerolax scheduler, because it breaks everything else. It is
    > a deferred EDF, that looking from wall clock, composes an "zerolaxish" timeline.

    Indeed. I was not intending that we do zerolax scheduler, I was merely
    misreading the diff assuming you were throttling the DL-server once again at
    the zerolax time.

    thanks,

    - Joel

    \
     
     \ /
      Last update: 2023-11-20 13:53    [W:2.697 / U:0.000 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site