All of lore.kernel.org
 help / color / mirror / Atom feed
From: Valentin Schneider <valentin.schneider@arm.com>
To: Dietmar Eggemann <dietmar.eggemann@arm.com>,
	linux-kernel@vger.kernel.org
Cc: Lingutla Chandrasekhar <clingutla@codeaurora.org>,
	Peter Zijlstra <peterz@infradead.org>,
	Ingo Molnar <mingo@kernel.org>,
	Vincent Guittot <vincent.guittot@linaro.org>,
	Morten Rasmussen <morten.rasmussen@arm.com>,
	Qais Yousef <qais.yousef@arm.com>,
	Quentin Perret <qperret@google.com>,
	Pavan Kondeti <pkondeti@codeaurora.org>,
	Rik van Riel <riel@surriel.com>
Subject: Re: [PATCH v3 1/7] sched/fair: Ignore percpu threads for imbalance pulls
Date: Tue, 16 Mar 2021 17:31:32 +0000	[thread overview]
Message-ID: <87pmzzronf.mognet@arm.com> (raw)
In-Reply-To: <bdea6c4f-262b-c3cb-637e-d4896e688dd3@arm.com>

On 16/03/21 16:49, Dietmar Eggemann wrote:
> On 11/03/2021 13:05, Valentin Schneider wrote:
>> From: Lingutla Chandrasekhar <clingutla@codeaurora.org>
>>
>> In load balancing, when balancing group is unable to pull task
>> due to ->cpus_ptr constraints from busy group, then it sets
>> LBF_SOME_PINNED to lb env flags, as a consequence, sgc->imbalance
>> is set for its parent domain level. which makes the group
>> classified as imbalance to get help from another balancing cpu.
>>
>> Consider a 4-CPU big.LITTLE system with CPUs 0-1 as LITTLEs and
>
> Does it have to be a big.LITTLE system? I assume this issue also happens
> on an SMP system.
>

Aye, though the consequences are "worse" on asym CPU capacity systems.

>> CPUs 2-3 as Bigs with below scenario:
>> - CPU0 doing newly_idle balancing
>> - CPU1 running percpu kworker and RT task (small tasks)
>
> What's the role of the small RT task here in the story?
>

I don't think it matters much here.

>> - CPU2 running 2 big tasks
>> - CPU3 running 1 medium task
>>
>> While CPU0 is doing newly_idle load balance at MC level, it fails to
>> pull percpu kworker from CPU1 and sets LBF_SOME_PINNED to lb env flag
>> and set sgc->imbalance at DIE level domain. As LBF_ALL_PINNED not cleared,
>> it tries to redo the balancing by clearing CPU1 in env cpus, but it don't
>> find other busiest_group, so CPU0 stops balacing at MC level without
>> clearing 'sgc->imbalance' and restart the load balacing at DIE level.
>>
>> And CPU0 (balancing cpu) finds LITTLE's group as busiest_group with group
>> type as imbalance, and Bigs that classified the level below imbalance type
>> would be ignored to pick as busiest, and the balancing would be aborted
>> without pulling any tasks (by the time, CPU1 might not have running tasks).
>>
>> It is suboptimal decision to classify the group as imbalance due to
>> percpu threads. So don't use LBF_SOME_PINNED for per cpu threads.
>
> This sentence mentioned per-cpu threads (and so does the patch name) but
> the implementation (only) deals with per-cpu kernel threads. IMHO, it
> would be good to align this.
>

Tell you what, I'll go for:
1) how can pcpu kthreads cause LBF_SOME_PINNED
2) why we may not want this, but still ignore !kthread pcpu tasks
3) why this is even more important for big.LITTLE

  parent reply	other threads:[~2021-03-16 17:32 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-03-11 12:05 [PATCH v3 0/7] sched/fair: misfit task load-balance tweaks Valentin Schneider
2021-03-11 12:05 ` [PATCH v3 1/7] sched/fair: Ignore percpu threads for imbalance pulls Valentin Schneider
2021-03-16 15:49   ` Dietmar Eggemann
2021-03-16 16:03     ` Chandra Sekhar Lingutla
2021-03-16 18:59       ` Dietmar Eggemann
2021-03-16 17:31     ` Valentin Schneider [this message]
2021-03-16 19:06       ` Dietmar Eggemann
2021-03-11 12:05 ` [PATCH v3 2/7] sched/fair: Clean up active balance nr_balance_failed trickery Valentin Schneider
2021-03-17 10:52   ` Dietmar Eggemann
2021-03-11 12:05 ` [PATCH v3 3/7] sched/fair: Add more sched_asym_cpucapacity static branch checks Valentin Schneider
2021-03-15 14:18   ` Vincent Guittot
2021-03-15 19:24     ` Valentin Schneider
2021-03-11 12:05 ` [PATCH v3 4/7] sched/fair: Introduce a CPU capacity comparison helper Valentin Schneider
2021-03-15 14:24   ` Vincent Guittot
2021-03-15 19:24     ` Valentin Schneider
2021-03-31 11:34   ` Chandra Sekhar Lingutla
2021-03-11 12:05 ` [PATCH v3 5/7] sched/fair: Employ capacity_greater() throughout load_balance() Valentin Schneider
2021-03-31 11:35   ` Chandra Sekhar Lingutla
2021-03-11 12:05 ` [PATCH v3 6/7] sched/fair: Filter out locally-unsolvable misfit imbalances Valentin Schneider
2021-03-15 15:13   ` Vincent Guittot
2021-03-15 19:18     ` Valentin Schneider
2021-03-19 15:19       ` Vincent Guittot
2021-03-23 18:51         ` Valentin Schneider
2021-03-11 12:05 ` [PATCH v3 7/7] sched/fair: Relax task_hot() for misfit tasks Valentin Schneider

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=87pmzzronf.mognet@arm.com \
    --to=valentin.schneider@arm.com \
    --cc=clingutla@codeaurora.org \
    --cc=dietmar.eggemann@arm.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@kernel.org \
    --cc=morten.rasmussen@arm.com \
    --cc=peterz@infradead.org \
    --cc=pkondeti@codeaurora.org \
    --cc=qais.yousef@arm.com \
    --cc=qperret@google.com \
    --cc=riel@surriel.com \
    --cc=vincent.guittot@linaro.org \
    /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.