Received-SPF: pass (google.com: domain of linux-kernel+bounces-8951-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45e3:2400::1 as permitted sender) client-ip=2604:1380:45e3:2400::1;
Precedence: bulk
MIME-Version: 1.0
References: <20231220001856.3710363-1-jstultz@google.com> <20231220001856.3710363-11-jstultz@google.com>
 <d029fafd-658a-44ec-872d-62370c6422d7@arm.com>
In-Reply-To: <d029fafd-658a-44ec-872d-62370c6422d7@arm.com>
From: John Stultz <jstultz@google.com>
Date: Thu, 21 Dec 2023 10:49:21 -0800
Message-ID: <CANDhNCpsCNwnNLYJgtxxPnSLeJP+BO3xP3wryA2_LjVcm0znfA@mail.gmail.com>
Subject: Re: [PATCH v7 10/23] sched: Split out __sched() deactivate task logic
 into a helper
To: Metin Kaya <metin.kaya@arm.com>
Cc: LKML <linux-kernel@vger.kernel.org>, Joel Fernandes <joelaf@google.com>, 
	Qais Yousef <qyousef@google.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>, 
	Valentin Schneider <vschneid@redhat.com>, Steven Rostedt <rostedt@goodmis.org>, 
	Ben Segall <bsegall@google.com>, Zimuzo Ezeozue <zezeozue@google.com>, 
	Youssef Esmat <youssefesmat@google.com>, Mel Gorman <mgorman@suse.de>, 
	Daniel Bristot de Oliveira <bristot@redhat.com>, Will Deacon <will@kernel.org>, Waiman Long <longman@redhat.com>, 
	Boqun Feng <boqun.feng@gmail.com>, "Paul E. McKenney" <paulmck@kernel.org>, 
	Xuewen Yan <xuewen.yan94@gmail.com>, K Prateek Nayak <kprateek.nayak@amd.com>, 
	Thomas Gleixner <tglx@linutronix.de>, kernel-team@android.com
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

On Thu, Dec 21, 2023 at 4:30=E2=80=AFAM Metin Kaya <metin.kaya@arm.com> wro=
te:
>
> On 20/12/2023 12:18 am, John Stultz wrote:
> > As we're going to re-use the deactivation logic,
> > split it into a helper.
> >
>
> * Better to have a comment (e.g., in which conditions
> try_to_deactivate_task() returns true or false) here.

Ah, good point. I've added a comment to address this.

> * try_to_deactivate_task() is temporarily used by 2 commits in the patch
> set (i.e., it's only called by __schedule() just like in this patch at
> the end of the series). However, it's nice to make that big
> __scheduler() function a bit modular as we discussed off-list. So,
> should we move this function out of the Proxy Execution patch set to get
> it merged independently?

Yeah. I add and later remove the proxy_deactivate() function as it
goes unused, but I am thinking about keeping it to handle the case
where if the blocked on chains are too long, and we're spending too
much time re-running find_proxy_task() - where we should probably just
deactivate the selected task and move on.  But for now, you're right.

The suggestion of just the initial step of splitting the logic out
(probably make it a void function because the remaining usage in
__schedule() doesn't care about the result) is a good one, so I'll
rework it this way and send it separately (along with other early
patches Qais suggested).

thanks
-john