All of lore.kernel.org
 help / color / mirror / Atom feed
From: sashiko-bot@kernel.org
To: "Tao Cui" <cui.tao@linux.dev>
Cc: bpf@vger.kernel.org, sched-ext@lists.linux.dev
Subject: Re: [PATCH 1/2] sched_ext: fix vtime priority queue inversion on wide vtime spread
Date: Tue, 01 Sep 2026 02:54:41 +0000	[thread overview]
Message-ID: <20260901025441.B3DCE1F000E9@smtp.kernel.org> (raw)
In-Reply-To: <20260901024038.730424-2-cui.tao@linux.dev>

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The patch silently breaks the established cyclic `vtime` ordering contract by switching to an absolute `<` comparison, causing permanent task starvation upon wraparound and contradicting existing kfunc documentation and selftests.
--

commit b0708e9275ee1ffad12d1e3811001b6d6ee4c741
Author: Tao Cui <cuitao@kylinos.cn>

sched_ext: fix vtime priority queue inversion on wide vtime spread

This commit replaces the time_before64() cyclic comparison in
scx_dsq_priq_less() with a plain less-than operation to prevent priority queue
inversion when vtimes have a wide spread.

> diff --git a/kernel/sched/ext/ext.c b/kernel/sched/ext/ext.c
> index 18183062f751c..db1ea9054c0ca 100644
> --- a/kernel/sched/ext/ext.c
> +++ b/kernel/sched/ext/ext.c
> @@ -1356,7 +1356,8 @@ static bool scx_dsq_priq_less(struct rb_node *node_a,
>  	const struct task_struct *b =
>  		container_of(node_b, struct task_struct, scx.dsq_priq);
>  
> -	return time_before64(a->scx.dsq_vtime, b->scx.dsq_vtime);
> +	/* dsq_vtime is arbitrary BPF input: keep a total order */
> +	return a->scx.dsq_vtime < b->scx.dsq_vtime;

[Severity: High]
Does this break the established cyclic vtime ordering contract and cause
permanent task starvation upon wraparound?

If a BPF scheduler assigns vtime to tasks, and the vtime advances and
eventually wraps around U64_MAX, tasks that are in the queue at the time of
wraparound (with vtime near U64_MAX) will be indefinitely starved. This happens
because new post-wrap tasks (with small absolute vtimes) will be strictly less
than the old ones and placed ahead of them.

The user-facing kfunc documentation for __scx_bpf_dsq_insert_vtime() in
kernel/sched/ext/ext.c still explicitly promises the cyclic wrapping behavior:

 * @args->vtime ordering is according to time_before64() which considers
 * wrapping. A numerically larger vtime may indicate an earlier position in the
 * ordering and vice-versa.

Additionally, BPF schedulers still actively rely on the cyclic vtime semantics,
as seen in tools/testing/selftests/sched_ext/select_cpu_vtime.bpf.c:

static inline bool vtime_before(u64 a, u64 b)
{
	return (s64)(a - b) < 0;
}

>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260901024038.730424-1-cui.tao@linux.dev?part=1

  reply	other threads:[~2026-09-01  2:54 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-01  2:40 [PATCH 0/2] sched_ext: fix wraparound-unsafe vtime orderings Tao Cui
2026-09-01  2:40 ` [PATCH 1/2] sched_ext: fix vtime priority queue inversion on wide vtime spread Tao Cui
2026-09-01  2:54   ` sashiko-bot [this message]
2026-09-01  6:47   ` Andrea Righi
2026-09-01  9:44     ` Tao Cui
2026-09-01  8:29   ` Tejun Heo
2026-09-01  2:40 ` [PATCH 2/2] sched_ext/scx_flatcg: make cgv_node_less() wraparound-safe Tao Cui
2026-09-01  2:51   ` sashiko-bot
2026-09-01  3:54   ` bot+bpf-ci

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260901025441.B3DCE1F000E9@smtp.kernel.org \
    --to=sashiko-bot@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=cui.tao@linux.dev \
    --cc=sashiko-reviews@lists.linux.dev \
    --cc=sched-ext@lists.linux.dev \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.