From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from casper.infradead.org (casper.infradead.org [90.155.50.34]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 05783333743 for ; Thu, 18 Dec 2025 08:14:13 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=90.155.50.34 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1766045657; cv=none; b=RW2mdaZ26A8Andc0SueHnernfGGe9q4KbmFOLo5MjrH1bgSIThlj3H1CIiWgBN7CRMV7beGaME44/RrKm2yft/W1Zn34lRwOF/aQcy9a20xrE9lWyhZLNUgYnraWckyxzs8SxfQD6Db4lPxWEX2+6+kX1UL/2ytvwm/fAJ72imQ= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1766045657; c=relaxed/simple; bh=GQN3Kb5C4BufPfLkM5E96nMymJSRsHZ+RW6R0ZrGAX4=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=NiZlon+N53juZQP/Cm1+oqohrDu+xUtMOCf4ItdQsojq3cdNG3q/LHVe9EqZY1VQBWa4r98aKaz9qseEnRldOYGmj6ye4PEwhBiGvUYoQcgjSEBP9HCEwBWfF93UfUynKj+I++tnQb/MDMI5YlCQNm9hHno1YGUC0qoGQer1j2s= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=infradead.org; spf=none smtp.mailfrom=infradead.org; dkim=pass (2048-bit key) header.d=infradead.org header.i=@infradead.org header.b=KPlcdUKr; arc=none smtp.client-ip=90.155.50.34 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=infradead.org Authentication-Results: smtp.subspace.kernel.org; spf=none smtp.mailfrom=infradead.org Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=infradead.org header.i=@infradead.org header.b="KPlcdUKr" DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=infradead.org; s=casper.20170209; h=In-Reply-To:Content-Type:MIME-Version: References:Message-ID:Subject:Cc:To:From:Date:Sender:Reply-To: Content-Transfer-Encoding:Content-ID:Content-Description; bh=V9vDHpjlDH3rr8+5EVbP4s9cmsW3i56PEZfl1+iZpas=; b=KPlcdUKrKLPVtoTdFbBtAlKPoE ZU4V4OuiA8vXlhVfG1NY2QLARPYl1uc72Dj78qxL9A1yEsiLMKTb1swL7JiberHZs+PgAbYvf0XJo ixEakAQHsPzp2qSz62BJEXFxxDLD8QXfTXmFhjsmkbWwpr4hXh+lsw9CQ0xjpoyieABWSPRGBSe63 aWNhKJ/idvlLu8RYCiexShTe32D7cPjVY7CkKW44CX/KZYH+YhvyxtJKBBS5sEAIIsAf7ZDnCHq2G rDJP84U3k2APGMGk4w5POXdqjimahjhx9rOYGeW1Rn4UeG4bMIJiEEfmanvabFsEk1MJbiTGsLUMv pHf59yWA==; Received: from 2001-1c00-8d85-5700-266e-96ff-fe07-7dcc.cable.dynamic.v6.ziggo.nl ([2001:1c00:8d85:5700:266e:96ff:fe07:7dcc] helo=noisy.programming.kicks-ass.net) by casper.infradead.org with esmtpsa (Exim 4.98.2 #2 (Red Hat Linux)) id 1vW99G-00000005pmY-3CG8; Thu, 18 Dec 2025 08:14:10 +0000 Received: by noisy.programming.kicks-ass.net (Postfix, from userid 1000) id 8E1553005AF; Thu, 18 Dec 2025 09:14:09 +0100 (CET) Date: Thu, 18 Dec 2025 09:14:09 +0100 From: Peter Zijlstra To: Zhan Xusheng Cc: vincent.guittot@linaro.org, dietmar.eggemann@arm.com, linux-sched@vger.kernel.org, linux-kernel@vger.kernel.org, Zhan Xusheng Subject: Re: [PATCH] sched/fair: factor out common sched_entity stats/task lookup Message-ID: <20251218081409.GG3707891@noisy.programming.kicks-ass.net> References: <20251217070040.8723-1-zhanxusheng@xiaomi.com> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20251217070040.8723-1-zhanxusheng@xiaomi.com> On Wed, Dec 17, 2025 at 03:00:40PM +0800, Zhan Xusheng wrote: > The fair scheduler has several update_stats_*_fair() helpers which > open-code the same boilerplate to retrieve sched_statistics and the > associated task (if any) from a sched_entity. > > Factor this common logic into a small static inline helper to reduce > duplication and improve readability, without changing behaviour or > control flow. > > No functional change intended. > > Signed-off-by: Zhan Xusheng > --- > kernel/sched/fair.c | 24 ++++++++++++------------ > 1 file changed, 12 insertions(+), 12 deletions(-) > > diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c > index da46c3164537..b4a9319a5753 100644 > --- a/kernel/sched/fair.c > +++ b/kernel/sched/fair.c > @@ -1243,6 +1243,15 @@ static void update_curr_fair(struct rq *rq) > update_curr(cfs_rq_of(&rq->donor->se)); > } > > +static inline void > +get_se_stats_and_task(struct sched_entity *se, > + struct sched_statistics **stats, > + struct task_struct **p) > +{ > + *stats = __schedstats_from_se(se); > + *p = entity_is_task(se) ? task_of(se) : NULL; > +} > + > static inline void > update_stats_wait_start_fair(struct cfs_rq *cfs_rq, struct sched_entity *se) > { > @@ -1252,10 +1261,7 @@ update_stats_wait_start_fair(struct cfs_rq *cfs_rq, struct sched_entity *se) > if (!schedstat_enabled()) > return; > > - stats = __schedstats_from_se(se); > - > - if (entity_is_task(se)) > - p = task_of(se); > + get_se_stats_and_task(se, &stats, &p); > > __update_stats_wait_start(rq_of(cfs_rq), p, stats); > } > @@ -1269,7 +1275,7 @@ update_stats_wait_end_fair(struct cfs_rq *cfs_rq, struct sched_entity *se) > if (!schedstat_enabled()) > return; > > - stats = __schedstats_from_se(se); > + get_se_stats_and_task(se, &stats, &p); > > /* > * When the sched_schedstat changes from 0 to 1, some sched se > @@ -1280,9 +1286,6 @@ update_stats_wait_end_fair(struct cfs_rq *cfs_rq, struct sched_entity *se) > if (unlikely(!schedstat_val(stats->wait_start))) > return; > > - if (entity_is_task(se)) > - p = task_of(se); > - > __update_stats_wait_end(rq_of(cfs_rq), p, stats); > } > > @@ -1295,10 +1298,7 @@ update_stats_enqueue_sleeper_fair(struct cfs_rq *cfs_rq, struct sched_entity *se > if (!schedstat_enabled()) > return; > > - stats = __schedstats_from_se(se); > - > - if (entity_is_task(se)) > - tsk = task_of(se); > + get_se_stats_and_task(se, &stats, &p); > > __update_stats_enqueue_sleeper(rq_of(cfs_rq), tsk, stats); > } Urgh, no. The stats and task things are unrelated, so munging them together like this makes no sense. Worse, the helper is terrible for failing to use the return value for its intended purpose.