[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <aHedrl4G5DecVzpS@gpd4>
Date: Wed, 16 Jul 2025 14:40:14 +0200
From: Andrea Righi <arighi@...dia.com>
To: Breno Leitao <leitao@...ian.org>
Cc: Tejun Heo <tj@...nel.org>, David Vernet <void@...ifault.com>,
Changwoo Min <changwoo@...lia.com>, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/2] sched_ext: Track currently locked rq
Hi Breno,
On Wed, Jul 16, 2025 at 03:47:38AM -0700, Breno Leitao wrote:
> Hello Andrea,
>
> On Tue, Jul 15, 2025 at 07:20:28PM +0200, Andrea Righi wrote:
> > > On Tue, Apr 22, 2025 at 10:26:32AM +0200, Andrea Righi wrote:
>
> > >
> > > > + lockdep_assert_rq_held(rq);
> > > > + __this_cpu_write(locked_rq, rq);
> > >
> > > This is hitting the following BUG() on some of my debug kernels:
> > >
> > > BUG: using __this_cpu_write() in preemptible [00000000] code: scx_layered_6-9/68770
> > >
> > > I have lockdep enabled, and I don't see the assert above. I am wondering
> > > if rq is locked but preemption continues to be enabled (!?)
> >
> > Interesting. And it makes sense, because we may have callbacks called from
> > a preemptible context (especially when rq == NULL).
> >
> > I think we can just put a preempt_disable() / preempt_enable() around
> > __this_cpu_write(). If we jump to another CPU during the callback it's
> > fine, since we would track the rq state on the other CPU with its own local
> > variable. And if we were able to jump there, it means that preemption was
> > disabled as well.
>
> First of all thanks for the suggestion!
>
> What about a patch like the following:
Looks good to me, feel free to add my:
Acked-by: Andrea Righi <arighi@...dia.com>
Thanks,
-Andrea
>
> commit 9ed31e914181ec8f2d0b4484c42b00b6794661b9
> Author: Breno Leitao <leitao@...ian.org>
> Date: Wed Jul 16 03:10:59 2025 -0700
>
> sched/ext: Suppress warning in __this_cpu_write() by disabling preemption
>
> __this_cpu_write() emits a warning if used with preemption enabled.
>
> Function update_locked_rq() might be called with preemption enabled,
> which causes the following warning:
>
> BUG: using __this_cpu_write() in preemptible [00000000] code: scx_layered_6-9/68770
>
> Disable preemption around the __this_cpu_write() call in
> update_locked_rq() to suppress the warning, without affecting behavior.
>
> If preemption triggers a jump to another CPU during the callback it's
> fine, since we would track the rq state on the other CPU with its own
> local variable.
>
> Suggested-by: Andrea Righi <arighi@...dia.com>
> Signed-off-by: Breno Leitao <leitao@...ian.org>
> Fixes: 18853ba782bef ("sched_ext: Track currently locked rq")
>
> diff --git a/kernel/sched/ext.c b/kernel/sched/ext.c
> index b498d867ba210..24fcbd7331f73 100644
> --- a/kernel/sched/ext.c
> +++ b/kernel/sched/ext.c
> @@ -1258,7 +1258,14 @@ static inline void update_locked_rq(struct rq *rq)
> */
> if (rq)
> lockdep_assert_rq_held(rq);
> + /*
> + * __this_cpu_write() emits a warning when used with preemption enabled.
> + * While there's no functional issue if the callback runs on another
> + * CPU, we disable preemption here solely to suppress that warning.
> + */
> + preempt_disable();
> __this_cpu_write(locked_rq, rq);
> + preempt_enable();
> }
>
> /*
Powered by blists - more mailing lists