RE: [XEN PATCH v2] xen/arinc653: fix delay in the start of major frame

[Nathan Studer <Nathan.Studer@xxxxxxxxxxxxxxx>]

On 7/17/25 9:21, Hildebrand, Stewart wrote:
> > -    if ( sched_priv->num_schedule_entries < 1 )
> > -        sched_priv->next_major_frame = now + DEFAULT_TIMESLICE;
> > -    else if ( now >= sched_priv->next_major_frame )
> > +    /* Switch to next major frame while handling potentially missed frames 
> > */
> > +    while ( now >= sched_priv->next_major_frame )
> >      {
> > -        /* time to enter a new major frame
> > -         * the first time this function is called, this will be true */
> > -        /* start with the first domain in the schedule */
> >          sched_priv->sched_index = 0;
> > -        sched_priv->next_major_frame = now + sched_priv->major_frame;
> > -        sched_priv->next_switch_time = now + 
> > sched_priv->schedule[0].runtime;
> > -    }
> > -    else
> > -    {
> > -        while ( (now >= sched_priv->next_switch_time) &&
> > -                (sched_priv->sched_index < 
> > sched_priv->num_schedule_entries) )
> > +
> > +        if ( sched_priv->num_schedule_entries < 1 )
> >          {
> > -            /* time to switch to the next domain in this major frame */
> > -            sched_priv->sched_index++;
> > -            sched_priv->next_switch_time +=
> > -                sched_priv->schedule[sched_priv->sched_index].runtime;
> > +            sched_priv->next_major_frame += DEFAULT_TIMESLICE;
> > +            sched_priv->next_switch_time =
> > + sched_priv->next_major_frame;
> >          }
> > +        else
> > +        {
> > +            sched_priv->next_switch_time = sched_priv->next_major_frame +
> > +                sched_priv->schedule[0].runtime;
> > +            sched_priv->next_major_frame += sched_priv->major_frame;
> > +        }
> > +    }
> 
> There's no need for the above loop, this can be fixed by subtracting the 
> remainder
> (modulus major_frame). E.g.:
> 
>     if ( now >= sched_priv->next_major_frame )
>     {
>         s_time_t major_frame = sched_priv->num_schedule_entries < 1
>                                ? DEFAULT_TIMESLICE
>                                : sched_priv->major_frame;
>         s_time_t remainder = (now - sched_priv->next_major_frame) %
> major_frame;
> 
>         sched_priv->sched_index = 0;
>         sched_priv->next_major_frame = now - remainder + major_frame;
>         sched_priv->next_switch_time = now - remainder +
>                                        (sched_priv->num_schedule_entries < 1
>                                         ? DEFAULT_TIMESLICE
>                                         : sched_priv->schedule[0].runtime);
>     }
> 

The direct method suggested by Stew is preferable in the unusual case where 
many major frames are missed.  (We have only seen that happen when using a 
debugger.)

To help uncover any issues like the one this patch addresses in the future we 
may also want to follow up this commit with a change to make scheduler misses 
more obvious.  Something like the following:

commit e95cbc9078127c412bd1605d93cb97837751b5b4 (HEAD -> master)
Author: Nathan Studer <nathan.studer@xxxxxxxxxxxxxxx>
Date:   Thu Jul 17 12:43:39 2025 -0400

    Do not silently skip frame overruns

diff --git a/xen/common/sched/arinc653.c b/xen/common/sched/arinc653.c
index 2d0d3bcbb3..a2c1c66c27 100644
--- a/xen/common/sched/arinc653.c
+++ b/xen/common/sched/arinc653.c
@@ -523,9 +523,17 @@ a653sched_do_schedule(
     a653sched_priv_t *sched_priv = SCHED_PRIV(ops);
     const unsigned int cpu = sched_get_resource_cpu(smp_processor_id());
     unsigned long flags;
+    unsigned int oindex;
+    unsigned int missed;
 
     spin_lock_irqsave(&sched_priv->lock, flags);
 
+    if ( now > (sched_priv->next_major_frame +  sched_priv->major_frame))
+    {
+        missed = (now - sched_priv->next_major_frame) / 
sched_priv->major_frame;
+        printk(XENLOG_ERR, "Missed %d major frame(s)!\n", missed);
+    }
+
     /* Switch to next major frame while handling potentially missed frames */
@@ -544,6 +552,7 @@ a653sched_do_schedule(
         }
     }
 
+    oindex = sched_priv->sched_index;
     /* Switch minor frame or find correct minor frame after a miss */
     while ( (now >= sched_priv->next_switch_time) &&
             (sched_priv->sched_index < sched_priv->num_schedule_entries) )
@@ -553,6 +562,12 @@ a653sched_do_schedule(
             sched_priv->schedule[sched_priv->sched_index].runtime;
     }
 
+    if ( (oindex - sched_priv->sched_index) > 1)
+    {
+        missed = (oindex - sched_priv->sched_index - 1);
+        printk(XENLOG_WARNING, "Missed %d minor frame(s)!\n", missed);
+    }
+
     /*

> The commit description may want some minor updating to reflect this.
> 
> > +
> > +    /* Switch minor frame or find correct minor frame after a miss */
> > +    while ( (now >= sched_priv->next_switch_time) &&
> > +        (sched_priv->sched_index < sched_priv->num_schedule_entries) )
> > +    {
> > +        sched_priv->sched_index++;
> > +        sched_priv->next_switch_time +=
> > +            sched_priv->schedule[sched_priv->sched_index].runtime;
> >      }
> >
> >      /*