From: Ingo Molnar <mingo@kernel.org>
To: Shrikanth Hegde <sshegde@linux.ibm.com>
Cc: peterz@infradead.org, vincent.guittot@linaro.org,
dietmar.eggemann@arm.com, qyousef@layalina.io,
linux-kernel@vger.kernel.org, vschneid@redhat.com
Subject: Re: [PATCH v3 0/2] sched: Minor changes for rd->overload access
Date: Fri, 29 Mar 2024 07:55:36 +0100 [thread overview]
Message-ID: <ZgZl6JuEccBa2XT5@gmail.com> (raw)
In-Reply-To: <0b5a4432-384c-4470-a7b6-6fcaf8c28236@linux.ibm.com>
* Shrikanth Hegde <sshegde@linux.ibm.com> wrote:
>
>
> On 3/28/24 4:37 PM, Ingo Molnar wrote:
> >
> > * Ingo Molnar <mingo@kernel.org> wrote:
> >
> >> Plus I've applied a patch to rename ::overload to ::overloaded. It is
> >> silly to use an ambiguous noun instead of a clear adjective when naming
> >> such a flag ...
> >
> > Plus SG_OVERLOAD should be SG_OVERLOADED as well - it now looks in line
> > with SG_OVERUTILIZED:
> >
> > /* Scheduling group status flags */
> > #define SG_OVERLOADED 0x1 /* More than one runnable task on a CPU. */
> > #define SG_OVERUTILIZED 0x2 /* One or more CPUs are over-utilized. */
> >
> > My followup question is: why are these a bitmask, why not separate
> > flags?
> >
> > AFAICS we only ever set them separately:
> >
> > thule:~/tip> git grep SG_OVER kernel/sched/
> > kernel/sched/fair.c: set_rd_overutilized_status(rq->rd, SG_OVERUTILIZED);
> > kernel/sched/fair.c: *sg_status |= SG_OVERLOADED;
> > kernel/sched/fair.c: *sg_status |= SG_OVERUTILIZED;
> > kernel/sched/fair.c: *sg_status |= SG_OVERLOADED;
> > kernel/sched/fair.c: set_rd_overloaded(env->dst_rq->rd, sg_status & SG_OVERLOADED);
> > kernel/sched/fair.c: sg_status & SG_OVERUTILIZED);
> > kernel/sched/fair.c: } else if (sg_status & SG_OVERUTILIZED) {
> > kernel/sched/fair.c: set_rd_overutilized_status(env->dst_rq->rd, SG_OVERUTILIZED);
> > kernel/sched/sched.h:#define SG_OVERLOADED 0x1 /* More than one runnable task on a CPU. */
> > kernel/sched/sched.h:#define SG_OVERUTILIZED 0x2 /* One or more CPUs are over-utilized. */
> > kernel/sched/sched.h: set_rd_overloaded(rq->rd, SG_OVERLOADED);
> >
> > In fact this results in suboptimal code:
> >
> > /* update overload indicator if we are at root domain */
> > set_rd_overloaded(env->dst_rq->rd, sg_status & SG_OVERLOADED);
> >
> > /* Update over-utilization (tipping point, U >= 0) indicator */
> > set_rd_overutilized_status(env->dst_rq->rd,
> > sg_status & SG_OVERUTILIZED);
> >
> > Note how the bits that got mixed together in sg_status now have to be
> > masked out individually.
> >
> > The sg_status bitmask appears to make no sense at all to me.
> >
> > By turning these into individual bool flags we could also do away with
> > all the extra SG_OVERLOADED/SG_OVERUTILIZED abstraction.
> >
> > Ie. something like the patch below? Untested.
>
> Looks good. I see it is merged to sched/core.
> Did a boot with that patch and hackbench is showing same results 320 CPU system.
Thanks, I've added:
Acked-by: Shrikanth Hegde <sshegde@linux.ibm.com>
Tested-by: Shrikanth Hegde <sshegde@linux.ibm.com>
And applied the additional docbook fix below on top as well.
Thaks,
Ingo
=================>
kernel/sched/fair.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index ebc8d5f855de..1dd37168da50 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -9933,7 +9933,8 @@ sched_reduced_capacity(struct rq *rq, struct sched_domain *sd)
* @sds: Load-balancing data with statistics of the local group.
* @group: sched_group whose statistics are to be updated.
* @sgs: variable to hold the statistics for this group.
- * @sg_status: Holds flag indicating the status of the sched_group
+ * @sg_overloaded: sched_group is overloaded
+ * @sg_overutilized: sched_group is overutilized
*/
static inline void update_sg_lb_stats(struct lb_env *env,
struct sd_lb_stats *sds,
next prev parent reply other threads:[~2024-03-29 6:55 UTC|newest]
Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-03-25 5:45 [PATCH v3 0/2] sched: Minor changes for rd->overload access Shrikanth Hegde
2024-03-25 5:45 ` [PATCH v3 1/2] sched/fair: Check rd->overload value before update Shrikanth Hegde
2024-03-28 10:47 ` [tip: sched/core] sched/fair: Check root_domain::overload " tip-bot2 for Shrikanth Hegde
2024-03-25 5:45 ` [PATCH v3 2/2] sched/fair: Use helper functions to access rd->overload Shrikanth Hegde
2024-03-28 10:47 ` [tip: sched/core] sched/fair: Use helper functions to access root_domain::overload tip-bot2 for Shrikanth Hegde
2024-03-25 10:36 ` [PATCH v3 0/2] sched: Minor changes for rd->overload access Ingo Molnar
2024-03-25 11:33 ` Shrikanth Hegde
2024-03-26 8:00 ` Ingo Molnar
2024-03-27 6:04 ` Shrikanth Hegde
2024-03-28 10:34 ` Ingo Molnar
2024-03-28 10:56 ` [tip: sched/core] sched/fair: Rename SG_OVERLOAD to SG_OVERLOADED tip-bot2 for Ingo Molnar
2024-03-28 10:56 ` [tip: sched/core] sched/fair: Rename {set|get}_rd_overload() to {set|get}_rd_overloaded() tip-bot2 for Ingo Molnar
2024-03-28 10:56 ` [tip: sched/core] sched/fair: Rename root_domain::overload to ::overloaded tip-bot2 for Ingo Molnar
2024-03-28 11:07 ` [PATCH v3 0/2] sched: Minor changes for rd->overload access Ingo Molnar
2024-03-28 17:19 ` Shrikanth Hegde
2024-03-29 6:55 ` Ingo Molnar [this message]
2024-03-28 12:01 ` [tip: sched/core] sched/fair: Rename set_rd_overutilized_status() to set_rd_overutilized() tip-bot2 for Ingo Molnar
2024-03-28 12:58 ` [PATCH v3 0/2] sched: Minor changes for rd->overload access Shrikanth Hegde
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=ZgZl6JuEccBa2XT5@gmail.com \
--to=mingo@kernel.org \
--cc=dietmar.eggemann@arm.com \
--cc=linux-kernel@vger.kernel.org \
--cc=peterz@infradead.org \
--cc=qyousef@layalina.io \
--cc=sshegde@linux.ibm.com \
--cc=vincent.guittot@linaro.org \
--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.