All of lore.kernel.org
 help / color / mirror / Atom feed
From: sashiko-bot@kernel.org
To: "Wanwu Li" <liwanwu@kylinos.cn>
Cc: sched-ext@lists.linux.dev
Subject: Re: [PATCH] sched_ext: Check bpf_timer_start return values in scx_qmap
Date: Thu, 27 Aug 2026 08:16:50 +0000	[thread overview]
Message-ID: <20260827081650.C58071F000E9@smtp.kernel.org> (raw)
In-Reply-To: <20260827080738.829103-3-liwanwu@kylinos.cn>

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [Medium] central_timerfn() unconditionally re-arms the timer with BPF_F_TIMER_CPU_PIN and ignores the return value, causing silent failure on older kernels.
--

commit b5a91ea2da5adc3305be9b35666d557645b76fd2
Author: Wanwu Li <liwanwu@kylinos.cn>

sched_ext: Check bpf_timer_start return values in scx_qmap

This commit adds return value checks to bpf_timer_start() calls in several
timer functions within scx_qmap. If the timer fails to re-arm, it now raises
an error via scx_bpf_error() rather than silently failing.

> diff --git a/tools/sched_ext/scx_qmap.bpf.c b/tools/sched_ext/scx_qmap.bpf.c
> index 5bb8b90a275a3..9f6e61d7ca071 100644
> --- a/tools/sched_ext/scx_qmap.bpf.c
> +++ b/tools/sched_ext/scx_qmap.bpf.c
> @@ -1246,7 +1246,8 @@ static int monitor_timerfn(void *map, int *key, struct bpf_timer *timer)

[ ... ]

> -	bpf_timer_start(timer, ONE_SEC_IN_NS, 0);
> +	if (bpf_timer_start(timer, ONE_SEC_IN_NS, 0))
> +		scx_bpf_error("failed to re-arm stats timer");
>  	return 0;
>  }

[Severity: Medium]
This is a pre-existing issue, but while adding these return value checks here,
should we apply the same fix to central_timerfn() in scx_central.bpf.c?

It unconditionally re-arms the timer with BPF_F_TIMER_CPU_PIN and ignores
the return value:

tools/sched_ext/scx_central.bpf.c:central_timerfn() {
    ...
    bpf_timer_start(timer, TIMER_INTERVAL_NS, BPF_F_TIMER_CPU_PIN);
    __sync_fetch_and_add(&nr_timers, 1);
    return 0;
}

On older kernels that don't support BPF_F_TIMER_CPU_PIN, this fails with
-EINVAL. While start_central_timer() gracefully falls back to flags = 0
if unsupported, the timer callback unconditionally passes the flag. Because
the return value is ignored, the timer permanently stops, potentially
starving tasks.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260827080738.829103-3-liwanwu@kylinos.cn?part=1

  reply	other threads:[~2026-08-27  8:16 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-27  8:07 [PATCH] sched_ext: Fix timer pinning and return value in scx_central Wanwu Li
2026-08-27  8:07 ` [PATCH] sched_ext: Fix vtime delta loss in scx_flatcg cgroup migration Wanwu Li
2026-08-27  8:07 ` [PATCH] sched_ext: Check bpf_timer_start return values in scx_qmap Wanwu Li
2026-08-27  8:16   ` sashiko-bot [this message]
2026-08-31 16:53 ` [PATCH] sched_ext: Fix timer pinning and return value in scx_central Tejun Heo

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=20260827081650.C58071F000E9@smtp.kernel.org \
    --to=sashiko-bot@kernel.org \
    --cc=liwanwu@kylinos.cn \
    --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.