From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mx0a-001b2d01.pphosted.com (mx0a-001b2d01.pphosted.com [148.163.156.1]) (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 BD1151AA1D2 for ; Fri, 20 Mar 2026 02:48:19 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=148.163.156.1 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1773974901; cv=none; b=lrhGUMbMXhd8c17U25RyjyvtJ0ItVpm8uMO15O5cAP6EWYckQYQsIp0ZMwhXE6WdalvS/l9dKYYjLsu/GDK8ByGgBOd6p7X9rJaTOFigPwRAOQVc3yrtVdeKTKOXZHe9O4rUhABfPOlQbNMGI0rNufzmB4HaF2TIyotdO0HYYGs= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1773974901; c=relaxed/simple; bh=AC7r2Y+jLgo2hfnW0/uk9eagyW9JkG/FSOnMYCsj4VU=; h=Message-ID:Date:MIME-Version:Subject:To:Cc:References:From: In-Reply-To:Content-Type; b=MAKvcXFVljGAaCAMFoNVHDzCcYsK+mGcXe5fzh71Cd7KYn6YDuBinkTVtSIy2ltcjsk1eXlkVNeFynTRbULsPmIZFHMNlAAfVxmEL1BgJRpP/FW2C6Z+aG8QDjRYdp0T655WJgaSo0Z23WngC8HYkiYSPe9vi6Jjz9zNzgEG/8o= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linux.ibm.com; spf=pass smtp.mailfrom=linux.ibm.com; dkim=pass (2048-bit key) header.d=ibm.com header.i=@ibm.com header.b=nH5HBor5; arc=none smtp.client-ip=148.163.156.1 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linux.ibm.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=linux.ibm.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=ibm.com header.i=@ibm.com header.b="nH5HBor5" Received: from pps.filterd (m0356517.ppops.net [127.0.0.1]) by mx0a-001b2d01.pphosted.com (8.18.1.11/8.18.1.11) with ESMTP id 62JFjAhx1484495; Fri, 20 Mar 2026 02:47:55 GMT DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ibm.com; h=cc :content-transfer-encoding:content-type:date:from:in-reply-to :message-id:mime-version:references:subject:to; s=pp1; bh=fqj7nD WqJTQq3Y4vw14o8lp3O4Vpg4k43hXeyluJbOA=; b=nH5HBor5ooTWRylHgluKLU Bz3VApI64fcwc24zYbbIJZvhuuPejJfMF2NpvkPB8i5f/NFVWnnvtUBxR9q40dff 4et6rjh9zdM5pwiUqkaD6/tVbQRvmymGcqNsieOOSzpIx9Ly8NeQZbOrFXizrIgR Dym23m39dpPPW7bTZqMzH1l+8A2S4ObWJdQ/ioIj+IHKX81hFWzJLrTJ9J/ByW1p 6RId5EjYCIDzF5Cp2ZqaGVnVzpLjlUDApEHS5x2RN0PrC9q5rrdQKf9FtqI0HMVU e96dyLAZGaXgpfWwjbQNEKxSOMDGGjejHzaZoiiSz0tS+QYHI5YnDJUvOla/8nzA == Received: from ppma13.dal12v.mail.ibm.com (dd.9e.1632.ip4.static.sl-reverse.com [50.22.158.221]) by mx0a-001b2d01.pphosted.com (PPS) with ESMTPS id 4cx7vfv0w2-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Fri, 20 Mar 2026 02:47:54 +0000 (GMT) Received: from pps.filterd (ppma13.dal12v.mail.ibm.com [127.0.0.1]) by ppma13.dal12v.mail.ibm.com (8.18.1.2/8.18.1.2) with ESMTP id 62JNnOF1032448; Fri, 20 Mar 2026 02:47:53 GMT Received: from smtprelay07.fra02v.mail.ibm.com ([9.218.2.229]) by ppma13.dal12v.mail.ibm.com (PPS) with ESMTPS id 4cwm7k4ydy-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Fri, 20 Mar 2026 02:47:53 +0000 Received: from smtpav02.fra02v.mail.ibm.com (smtpav02.fra02v.mail.ibm.com [10.20.54.101]) by smtprelay07.fra02v.mail.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP id 62K2lp5L35651842 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Fri, 20 Mar 2026 02:47:52 GMT Received: from smtpav02.fra02v.mail.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id DBAB920043; Fri, 20 Mar 2026 02:47:51 +0000 (GMT) Received: from smtpav02.fra02v.mail.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id 74EBB20075; Fri, 20 Mar 2026 02:47:49 +0000 (GMT) Received: from [9.39.23.72] (unknown [9.39.23.72]) by smtpav02.fra02v.mail.ibm.com (Postfix) with ESMTP; Fri, 20 Mar 2026 02:47:49 +0000 (GMT) Message-ID: Date: Fri, 20 Mar 2026 08:17:46 +0530 Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH 1/2] sched/fair: consider hk_mask early in triggering ilb To: Shubhang Kaushik , mingo@kernel.org, peterz@infradead.org, vincent.guittot@linaro.org, linux-kernel@vger.kernel.org Cc: kprateek.nayak@amd.com, juri.lelli@redhat.com, vschneid@redhat.com, tglx@linutronix.de, dietmar.eggemann@arm.com, frederic@kernel.org, longman@redhat.com References: <20260319065314.343932-1-sshegde@linux.ibm.com> <20260319065314.343932-2-sshegde@linux.ibm.com> <788875ec-787b-3024-6f01-ef3ed9bd6ec7@os.amperecomputing.com> Content-Language: en-US From: Shrikanth Hegde In-Reply-To: <788875ec-787b-3024-6f01-ef3ed9bd6ec7@os.amperecomputing.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit X-TM-AS-GCONF: 00 X-Proofpoint-ORIG-GUID: r-JeiggVomTxw5s74IzFVuDsoBPrG12U X-Authority-Analysis: v=2.4 cv=KajfcAYD c=1 sm=1 tr=0 ts=69bcb55a cx=c_pps a=AfN7/Ok6k8XGzOShvHwTGQ==:117 a=AfN7/Ok6k8XGzOShvHwTGQ==:17 a=IkcTkHD0fZMA:10 a=Yq5XynenixoA:10 a=VkNPw1HP01LnGYTKEx00:22 a=RnoormkPH1_aCDwRdu11:22 a=U7nrCbtTmkRpXpFmAIza:22 a=VwQbUJbxAAAA:8 a=VnNF1IyMAAAA:8 a=vzhER2c_AAAA:8 a=qe7mkg6I1TqCEWcM4lIA:9 a=3ZKOabzyN94A:10 a=QEXdDO2ut3YA:10 a=0YTRHmU2iG2pZC6F1fw2:22 X-Proofpoint-Spam-Details-Enc: AW1haW4tMjYwMzIwMDAyMCBTYWx0ZWRfXwLX4OufKYEGw 1Wd8b5uQbh7FKQbVZnO7rVObHo5gIsYQ34OK8/3dNGWXslTAEa5LvKAcY1EEE2RwPHCyZwcYhJb s2tWbF7mtQ5vu27k5kxFnfXX8xif1BD7cTB0FaPtrGAHYuMtl1ZTKyvCwlmH9s7EKArjd5yNXSu Ci3cu0yNUWYiCU/yV9ZJH3DY8Hh4nu5kSEJz+izYVWvwVxu2Y3UrUO4WZpZ8BZaKOGv0SZkTmyW wzjl2utya/J3+/LgnznGYTfRdFZDIA9l0TXv9xW2B5u5oY2Ub085cS034zYLPCODbm6/qgzLEhK 9vgi95e5XNlip9ahnGTzOy+5ZTh5SPkNXv2wvqT8tbdwj8znwXKEqARG37izuFbZVX8au/txYxR m1P5p6zsITU7+4Ma3MNRz8vH9lHuMIGpQin2etlMuhjrkKV+vkYPH/i5T9rcA9ZfzEeji+HAoml 2RwIPlNhNNEpg0AbZ7g== X-Proofpoint-GUID: r-JeiggVomTxw5s74IzFVuDsoBPrG12U X-Proofpoint-Virus-Version: vendor=baseguard engine=ICAP:2.0.293,Aquarius:18.0.1143,Hydra:6.1.51,FMLib:17.12.100.49 definitions=2026-03-19_04,2026-03-19_05,2025-10-01_01 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 suspectscore=0 adultscore=0 spamscore=0 malwarescore=0 clxscore=1011 impostorscore=0 bulkscore=0 lowpriorityscore=0 priorityscore=1501 phishscore=0 classifier=typeunknown authscore=0 authtc= authcc= route=outbound adjust=0 reason=mlx scancount=1 engine=8.22.0-2603050001 definitions=main-2603200020 Hi Shubhang. Thanks for taking a look. On 3/20/26 4:28 AM, Shubhang Kaushik wrote: > Hi Shrikanth, > > On Thu, 19 Mar 2026, Shrikanth Hegde wrote: > >> Current code around nohz_balancer_kick and kick_ilb: >> 1. Checks for nohz.idle_cpus_mask to see if idle load balance(ilb) is >>   needed. >> 2. Does a few checks to see if any conditions meet the criteria. >> 3. Tries to find the idle CPU. But the idle CPU found should be part of >>   housekeeping CPUs. >> >> If there is no housekeeping idle CPU, then step 2 is done >> un-necessarily, since 3 bails out without doing the ilb. >> >> Fix that by making the decision early and pass it on to find_new_ilb. >> Use a percpu cpumask instead of allocating it everytime since this is in >> fastpath. >> >> If flags is set to NOHZ_STATS_KICK since the time is after >> nohz.next_blocked >> but before nohz.next_balance and there are idle CPUs which are part of >> housekeeping, need to copy the same logic there too. >> >> While there, fix the stale comments around nohz.nr_cpus >> >> Signed-off-by: Shrikanth Hegde >> --- >> >> Didn't add the fixes tag since it addresses more than stale comments. >> >> kernel/sched/fair.c | 45 +++++++++++++++++++++++++++++++-------------- >> 1 file changed, 31 insertions(+), 14 deletions(-) >> >> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c >> index b19aeaa51ebc..02cca2c7a98d 100644 >> --- a/kernel/sched/fair.c >> +++ b/kernel/sched/fair.c >> @@ -7392,6 +7392,7 @@ static inline unsigned int >> cfs_h_nr_delayed(struct rq *rq) >> static DEFINE_PER_CPU(cpumask_var_t, load_balance_mask); >> static DEFINE_PER_CPU(cpumask_var_t, select_rq_mask); >> static DEFINE_PER_CPU(cpumask_var_t, should_we_balance_tmpmask); >> +static DEFINE_PER_CPU(cpumask_var_t, kick_ilb_tmpmask); >> >> #ifdef CONFIG_NO_HZ_COMMON >> >> @@ -12629,15 +12630,14 @@ static inline int on_null_domain(struct rq *rq) >>  * - When one of the busy CPUs notices that there may be an idle >> rebalancing >>  *   needed, they will kick the idle load balancer, which then does idle >>  *   load balancing for all the idle CPUs. >> + * >> + *   @cpus idle CPUs in HK_TYPE_KERNEL_NOISE housekeeping >>  */ >> -static inline int find_new_ilb(void) >> +static inline int find_new_ilb(struct cpumask *cpus) >> { >> -    const struct cpumask *hk_mask; >>     int ilb_cpu; >> >> -    hk_mask = housekeeping_cpumask(HK_TYPE_KERNEL_NOISE); >> - >> -    for_each_cpu_and(ilb_cpu, nohz.idle_cpus_mask, hk_mask) { >> +    for_each_cpu(ilb_cpu, cpus) { >> >>         if (ilb_cpu == smp_processor_id()) >>             continue; >> @@ -12656,7 +12656,7 @@ static inline int find_new_ilb(void) >>  * We pick the first idle CPU in the HK_TYPE_KERNEL_NOISE housekeeping >> set >>  * (if there is one). >>  */ >> -static void kick_ilb(unsigned int flags) >> +static void kick_ilb(unsigned int flags, struct cpumask *cpus) >> { >>     int ilb_cpu; >> >> @@ -12667,7 +12667,7 @@ static void kick_ilb(unsigned int flags) >>     if (flags & NOHZ_BALANCE_KICK) >>         nohz.next_balance = jiffies+1; >> >> -    ilb_cpu = find_new_ilb(); >> +    ilb_cpu = find_new_ilb(cpus); >>     if (ilb_cpu < 0) >>         return; >> >> @@ -12700,6 +12700,7 @@ static void kick_ilb(unsigned int flags) >>  */ >> static void nohz_balancer_kick(struct rq *rq) >> { >> +    struct cpumask *ilb_cpus = >> this_cpu_cpumask_var_ptr(kick_ilb_tmpmask); >>     unsigned long now = jiffies; >>     struct sched_domain_shared *sds; >>     struct sched_domain *sd; >> @@ -12715,27 +12716,41 @@ static void nohz_balancer_kick(struct rq *rq) >>      */ >>     nohz_balance_exit_idle(rq); >> >> +    /* ILB considers only HK_TYPE_KERNEL_NOISE housekeeping CPUs */ >> + >>     if (READ_ONCE(nohz.has_blocked_load) && >> -        time_after(now, READ_ONCE(nohz.next_blocked))) >> +        time_after(now, READ_ONCE(nohz.next_blocked))) { >>         flags = NOHZ_STATS_KICK; >> +        cpumask_and(ilb_cpus, nohz.idle_cpus_mask, >> +                housekeeping_cpumask(HK_TYPE_KERNEL_NOISE)); >> +    } >> > Moving cpumask_and() here would be taxing the busy/requesting CPU on > every kick check, thereby breaking the lazy evaluation. > There is time checks. So not every tick. > In nohz_full scenario, the housekeeping mask is static (default CPU 0). One could specify the mask. What do you mean CPU 0? > If this HK CPU is busy, it is already running the tick and will handle > load balancing itslef. If it is idle, it is already in the > idle_cpus_mask. Moving this intersection to the busy CPU disturbs lazy > evaluation. Why force every busy worker to perform bitmask math just to > handle a no-idle-HK case tha the system handles naturally by being busy? > lazy evaluation is about NOHZ_BALANCE_KICK which does the actual idle balance. But NOHZ_STATS_KICK would set more often on normal system at 32 msec. You do have a point. What if it was not due to nohz.next_blocked but due to later checks such as nr_running > 1. I would say thats rare. I didn;t want to put too many checks such as do this only for nohz_full systems and do lazy evaluation for systems without it. I do agree benefit of this patch is limited to case where one specifies nohz_full=. >>     /* >> -     * Most of the time system is not 100% busy. i.e nohz.nr_cpus > 0 >> -     * Skip the read if time is not due. >> +     * Most of the time system is not 100% busy. i.e there are idle >> +     * housekeeping CPUs. >> +     * >> +     * So, Skip the reading idle_cpus_mask if time is not due. >>      * >>      * If none are in tickless mode, there maybe a narrow window >>      * (28 jiffies, HZ=1000) where flags maybe set and kick_ilb called. >>      * But idle load balancing is not done as find_new_ilb fails. >> -     * That's very rare. So read nohz.nr_cpus only if time is due. >> +     * That's very rare. So check (idle_cpus_mask & >> HK_TYPE_KERNEL_NOISE) >> +     * only if time is due. >> +     * >>      */ >>     if (time_before(now, nohz.next_balance)) >>         goto out; >> >> +    /* Avoid the double computation */ >> +    if (flags != NOHZ_STATS_KICK) >> +        cpumask_and(ilb_cpus, nohz.idle_cpus_mask, >> +                housekeeping_cpumask(HK_TYPE_KERNEL_NOISE)); >> + >>     /* >>      * None are in tickless mode and hence no need for NOHZ idle load >>      * balancing >>      */ >> -    if (unlikely(cpumask_empty(nohz.idle_cpus_mask))) >> +    if (unlikely(cpumask_empty(ilb_cpus))) >>         return; >> > Checking ilb_cpus here is a broken fix because the underlying > nohz.idle_cpus_mask is currently unreliable. Under CONFIG_NO_HZ_FULL, > there is a documented visibility bug: tick_nohz_full_stop_tick() often > stops the tick before idle entry, causing nohz_balance_enter_idle() to > be skipped. This means many valid, tickless idle CPUs are never added to > the mask in the first place. This is not in the scope of this patch. > https://lore.kernel.org/lkml/20260203-fix-nohz-idle-v1-1- > ad05a5872080@os.amperecomputing.com/ > >>     if (rq->nr_running >= 2) { >> @@ -12767,7 +12782,7 @@ static void nohz_balancer_kick(struct rq *rq) >>          * When balancing between cores, all the SMT siblings of the >>          * preferred CPU must be idle. >>          */ >> -        for_each_cpu_and(i, sched_domain_span(sd), >> nohz.idle_cpus_mask) { >> +        for_each_cpu_and(i, sched_domain_span(sd), ilb_cpus) { >>             if (sched_asym(sd, i, cpu)) { >>                 flags = NOHZ_STATS_KICK | NOHZ_BALANCE_KICK; >>                 goto unlock; >> @@ -12820,7 +12835,7 @@ static void nohz_balancer_kick(struct rq *rq) >>         flags |= NOHZ_NEXT_KICK; >> >>     if (flags) >> -        kick_ilb(flags); >> +        kick_ilb(flags, ilb_cpus); >> } >> >> static void set_cpu_sd_state_busy(int cpu) >> @@ -14253,6 +14268,8 @@ __init void init_sched_fair_class(void) >>         zalloc_cpumask_var_node(&per_cpu(select_rq_mask,    i), >> GFP_KERNEL, cpu_to_node(i)); >>         zalloc_cpumask_var_node(&per_cpu(should_we_balance_tmpmask, i), >>                     GFP_KERNEL, cpu_to_node(i)); >> +        zalloc_cpumask_var_node(&per_cpu(kick_ilb_tmpmask, i), >> +                    GFP_KERNEL, cpu_to_node(i)); >> >> #ifdef CONFIG_CFS_BANDWIDTH >>         INIT_CSD(&cpu_rq(i)->cfsb_csd, __cfsb_csd_unthrottle, cpu_rq(i)); >> -- >> 2.43.0 >> >> > > Regards, > Shubhang Kaushik