All of lore.kernel.org
 help / color / mirror / Atom feed
From: Andrea Righi <arighi@nvidia.com>
To: Changwoo Min <changwoo@igalia.com>
Cc: tj@kernel.org, void@manifault.com, kernel-dev@igalia.com,
	sched-ext@lists.linux.dev, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] sched_ext: Fix is_bpf_migration_disabled() false negative on non-PREEMPT_RCU
Date: Thu, 2 Apr 2026 11:45:38 +0200	[thread overview]
Message-ID: <ac46wjXIGgnMT_1p@gpd4> (raw)
In-Reply-To: <20260402023150.660967-1-changwoo@igalia.com>

On Thu, Apr 02, 2026 at 11:31:50AM +0900, Changwoo Min wrote:
> Since commit 8e4f0b1ebcf2 ("bpf: use rcu_read_lock_dont_migrate() for
> trampoline.c"), the BPF prolog (__bpf_prog_enter) calls migrate_disable()
> only when CONFIG_PREEMPT_RCU is enabled, via rcu_read_lock_dont_migrate().
> Without CONFIG_PREEMPT_RCU, the prolog never touches migration_disabled,
> so migration_disabled == 1 always means the task is truly
> migration-disabled regardless of whether it is the current task.
> 
> The old unconditional p == current check was a false negative in this
> case, potentially allowing a migration-disabled task to be dispatched to
> a remote CPU and triggering scx_error in task_can_run_on_remote_rq().
> 
> Only apply the p == current disambiguation when CONFIG_PREEMPT_RCU is
> enabled, where the ambiguity with the BPF prolog still exists.
> 
> Link: https://lore.kernel.org/lkml/20250821090609.42508-8-dongml2@chinatelecom.cn/
> Signed-off-by: Changwoo Min <changwoo@igalia.com>

Makes sense to me. Instead of the link we should probably add:

Fixes: 8e4f0b1ebcf2 ("bpf: use rcu_read_lock_dont_migrate() for trampoline.c")
Cc: stable@vger.kernel.org # v6.18+

So kernels with the BPF change will also pick this one.
Apart than that:

Reviewed-by: Andrea Righi <arighi@nvidia.com>

Thanks,
-Andrea

> ---
>  kernel/sched/ext_idle.c | 31 +++++++++++++++++++------------
>  1 file changed, 19 insertions(+), 12 deletions(-)
> 
> diff --git a/kernel/sched/ext_idle.c b/kernel/sched/ext_idle.c
> index a61339c36902..ecf7e09b54ae 100644
> --- a/kernel/sched/ext_idle.c
> +++ b/kernel/sched/ext_idle.c
> @@ -881,25 +881,32 @@ static bool check_builtin_idle_enabled(struct scx_sched *sch)
>   * code.
>   *
>   * We can't simply check whether @p->migration_disabled is set in a
> - * sched_ext callback, because migration is always disabled for the current
> - * task while running BPF code.
> + * sched_ext callback, because the BPF prolog (__bpf_prog_enter) may disable
> + * migration for the current task while running BPF code.
>   *
> - * The prolog (__bpf_prog_enter) and epilog (__bpf_prog_exit) respectively
> - * disable and re-enable migration. For this reason, the current task
> - * inside a sched_ext callback is always a migration-disabled task.
> + * Since the BPF prolog calls migrate_disable() only when CONFIG_PREEMPT_RCU
> + * is enabled (via rcu_read_lock_dont_migrate()), migration_disabled == 1 for
> + * the current task is ambiguous only in that case: it could be from the BPF
> + * prolog rather than a real migrate_disable() call.
>   *
> - * Therefore, when @p->migration_disabled == 1, check whether @p is the
> - * current task or not: if it is, then migration was not disabled before
> - * entering the callback, otherwise migration was disabled.
> + * Without CONFIG_PREEMPT_RCU, the BPF prolog never calls migrate_disable(),
> + * so migration_disabled == 1 always means the task is truly
> + * migration-disabled.
> + *
> + * Therefore, when migration_disabled == 1 and CONFIG_PREEMPT_RCU is enabled,
> + * check whether @p is the current task or not: if it is, then migration was
> + * not disabled before entering the callback, otherwise migration was disabled.
>   *
>   * Returns true if @p is migration-disabled, false otherwise.
>   */
>  static bool is_bpf_migration_disabled(const struct task_struct *p)
>  {
> -	if (p->migration_disabled == 1)
> -		return p != current;
> -	else
> -		return p->migration_disabled;
> +	if (p->migration_disabled == 1) {
> +		if (IS_ENABLED(CONFIG_PREEMPT_RCU))
> +			return p != current;
> +		return true;
> +	}
> +	return p->migration_disabled;
>  }
>  
>  static s32 select_cpu_from_kfunc(struct scx_sched *sch, struct task_struct *p,
> -- 
> 2.53.0
> 

  reply	other threads:[~2026-04-02  9:45 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-04-02  2:31 [PATCH] sched_ext: Fix is_bpf_migration_disabled() false negative on non-PREEMPT_RCU Changwoo Min
2026-04-02  9:45 ` Andrea Righi [this message]
2026-04-02 14:13 ` Kuba Piecuch
2026-04-02 19:28 ` 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=ac46wjXIGgnMT_1p@gpd4 \
    --to=arighi@nvidia.com \
    --cc=changwoo@igalia.com \
    --cc=kernel-dev@igalia.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=sched-ext@lists.linux.dev \
    --cc=tj@kernel.org \
    --cc=void@manifault.com \
    /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.