On 17/03/25 05:31, Jan Beulich wrote:
> Even before its recent movement to the scheduler's private data structure it
> looks
> to have been wrong to update the field under lock, but then read it with the
> lock
> no longer held.
>
> Coverity-ID: 1644500
> Fixes: 9f0c658baedc ("arinc: add cpu-pool support to scheduler")
> Signed-off-by: Jan Beulich <jbeul...@suse.com>
> ---
> The Fixes: tag references where the locking was added; I can't exclude there
> was
> an issue here already before that.
>
> --- a/xen/common/sched/arinc653.c
> +++ b/xen/common/sched/arinc653.c
> @@ -579,6 +579,9 @@ a653sched_do_schedule(
> */
> BUG_ON(now >= sched_priv->next_major_frame);
>
> + prev->next_time = sched_priv->next_switch_time - now;
> +
> + /* Return the amount of time the next domain has to run. */
This could be pushed up to immediately after next_switch_time is set, but here
is
good enough. However, did you mean to put the comment after the assignment
separated by whitespace?
Nate
> spin_unlock_irqrestore(&sched_priv->lock, flags);
>
> /* Tasklet work (which runs in idle UNIT context) overrides all else. */
> @@ -
> 590,11 +593,7 @@ a653sched_do_schedule(
> && (sched_unit_master(new_task) != cpu) )
> new_task = IDLETASK(cpu);
>
> - /*
> - * Return the amount of time the next domain has to run and the address
> - * of the selected task's UNIT structure.
> - */
> - prev->next_time = sched_priv->next_switch_time - now;
> + /* Also return the address of the selected task's UNIT structure.
> + */
> prev->next_task = new_task;
> new_task->migrated = false;
