From: Andrea Righi <arighi@nvidia.com>
To: Tejun Heo <tj@kernel.org>
Cc: David Vernet <void@manifault.com>,
Changwoo Min <changwoo@igalia.com>,
Ingo Molnar <mingo@redhat.com>,
Peter Zijlstra <peterz@infradead.org>,
Juri Lelli <juri.lelli@redhat.com>,
Vincent Guittot <vincent.guittot@linaro.org>,
Dietmar Eggemann <dietmar.eggemann@arm.com>,
Steven Rostedt <rostedt@goodmis.org>,
Ben Segall <bsegall@google.com>, Mel Gorman <mgorman@suse.de>,
Valentin Schneider <vschneid@redhat.com>,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH v7] sched_ext: idle: Refresh idle masks during idle-to-idle transitions
Date: Fri, 10 Jan 2025 22:40:45 +0100 [thread overview]
Message-ID: <Z4GT3Q4U_E8bQHrl@gpd3> (raw)
In-Reply-To: <Z4GTA4vw6-5_JAox@slm.duckdns.org>
On Fri, Jan 10, 2025 at 11:37:07AM -1000, Tejun Heo wrote:
> Hello,
>
> On Fri, Jan 10, 2025 at 10:23:21PM +0100, Andrea Righi wrote:
> ...
> > + /*
> > + * Update the idle masks:
> > + * - for real idle transitions (do_notify == true)
> > + * - for idle-to-idle transitions (indicated by the previous task
> > + * being the idle thread, managed by pick_task_idle())
> > + *
> > + * Skip updating idle masks if the previous task is not the idle
> > + * thread, since set_next_task_idle() has already handled it when
> > + * transitioning from a task to the idle thread (calling this
> > + * function with do_notify == true).
> > + *
> > + * In this way we can avoid updating the idle masks twice,
> > + * unnecessarily.
> > + */
> > + if (do_notify || is_idle_task(rq->curr))
> > + update_idle_masks(cpu, idle);
>
> Sorry about the multiple nitpicks but wouldn't it be better to have the
> static_key test before dynamic tests? I'd move scx_builtin_idle_enabled test
> out of update_idle_masks() and put that in front of other tests here. Also,
> maybe name the function update_builtin_idle() to stay consistent with the
> static_key name? Otherwise looks good to me.
No problem, all makes sense to me. I'll apply these changes and send a v8.
Thanks for the review!
-Andrea
prev parent reply other threads:[~2025-01-10 21:40 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-01-10 21:23 [PATCH v7] sched_ext: idle: Refresh idle masks during idle-to-idle transitions Andrea Righi
2025-01-10 21:37 ` Tejun Heo
2025-01-10 21:40 ` Andrea Righi [this message]
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=Z4GT3Q4U_E8bQHrl@gpd3 \
--to=arighi@nvidia.com \
--cc=bsegall@google.com \
--cc=changwoo@igalia.com \
--cc=dietmar.eggemann@arm.com \
--cc=juri.lelli@redhat.com \
--cc=linux-kernel@vger.kernel.org \
--cc=mgorman@suse.de \
--cc=mingo@redhat.com \
--cc=peterz@infradead.org \
--cc=rostedt@goodmis.org \
--cc=tj@kernel.org \
--cc=vincent.guittot@linaro.org \
--cc=void@manifault.com \
--cc=vschneid@redhat.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.