Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Thu, 27 Jul 2023 15:25:29 +0800
From:   Chen Yu <yu.c.chen@intel.com>
To:     Shrikanth Hegde <sshegde@linux.vnet.ibm.com>
CC:     <peterz@infradead.org>, <vincent.guittot@linaro.org>,
        <srikar@linux.vnet.ibm.com>, <linux-kernel@vger.kernel.org>,
        <mingo@redhat.com>, <dietmar.eggemann@arm.com>, <mgorman@suse.de>,
        <rui.zhang@intel.com>, <tim.c.chen@intel.com>
Subject: Re: [RFC PATCH] sched/fair: Skip idle CPU search on busy system
Message-ID: <ZMIb6TYMrtzLHSBb@chenyu5-mobl2>
References: <20230726093612.1882644-1-sshegde@linux.vnet.ibm.com>
Content-Type: text/plain; charset="us-ascii"
Content-Disposition: inline
In-Reply-To: <20230726093612.1882644-1-sshegde@linux.vnet.ibm.com>
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?GOp9fZBj3jU259eEw5+WNzMgW1gn4MKx5YoIVHa5JVfxzHa15PIEvMyhDR73?=
 =?us-ascii?Q?kegFrfn9Hw3IlGkvgAxLtIuw0ONvMzgXf9l9BMQc1laUTEq8MEEeLQsjyPaf?=
 =?us-ascii?Q?UsfzNQh9MqxIBHTJzLZwKJzjah9fUWfgTWC6iWSpIZrMlWq79MfWbUf1b5/+?=
 =?us-ascii?Q?70/kA6lLgRsJ3xdz3YnzNU0qAWdJWUDRS/qQIjsEONrr+cIVNJf4CmQRj5xf?=
 =?us-ascii?Q?SeTYqocesAFkn7t4ZJpYB8FjT2TzX7Wgj/RTSVQbmkhyFvuMhVyxP88Zliii?=
 =?us-ascii?Q?dm0FUUT4ZRGYLUaKnf1uUm6en/+rGrFOrHE2yWYL6lCTuOkq1tVYme5XEYlB?=
 =?us-ascii?Q?ISvhw9qTONvv9XOhGM1GHjibC0U0WFCVOYO21sepJsPzRbYjpXCLQrXp5LCL?=
 =?us-ascii?Q?pyD3BmTOQaRgFbn0k26Lbo1toCX3c4yV64na6uLhacVuRMbr0iqAUIvlwB56?=
 =?us-ascii?Q?ohyRCvzKEk/DNmWAvCz34p5nnHHuH6wKRv3+DUMtF0f3zCB8rFzTaaxi6Gb5?=
 =?us-ascii?Q?Mv1sTJVsL+ETWOLyvdRma+Tq2lPFL1GXNYkTmoXtgMPyZnJ7tK2Ql0K/kDiL?=
 =?us-ascii?Q?fGCqb7yQqLBE6nuTEjLtXOzlBwmHLSoRO9U5xT1YhPLv0BfegNiRQ/IxFpjD?=
 =?us-ascii?Q?Vc+nRn/+K2pmF5RD+aol7yhN6cdLjf6EXyvNag+dFxLIAnL5t5KMxDvwJH3M?=
 =?us-ascii?Q?ky5ayXI5g/vJmfR3nJvT8NyIVo8rmuDVwrTpj+bjCYy6fLNgnCWeb0A3gbFc?=
 =?us-ascii?Q?hqpDBD1B0csmyeTUp6ZNeqyeSPdFDcNXakQekBPJfYPXiH6edbBdR56ma6/d?=
 =?us-ascii?Q?Hhyuj+Qegy8aOvs1hLHknvTZDUFNWDOh2x5RE7HoDQYxx54glN/h7Grtp/3/?=
 =?us-ascii?Q?t7cP67eNldczhiSjNMl/mpeALPRAeQveZHah1J4Z8mGcSswSqzoORu09FDfh?=
 =?us-ascii?Q?UVz8wKa0U4GPLCaQj6/0GoS2JR/BLhSzJEFxPlTcs8qF8LuYgB2BLj9qWYNi?=
 =?us-ascii?Q?VGcHcexejP2iNm1wR+k2YjbV9wtQqrN8qKab0OIR5LDOeXhqVzaETEo0L98Z?=
 =?us-ascii?Q?38ncumqcstNb8LEIIIDf6maU+RylDs5158HF/09f5o74eULEFpuZI3Fafu2v?=
 =?us-ascii?Q?ZFu335M6y2u4rH+57tDgHji3uhdmT3Ly20B0e+ISfkFgaQ4C3Fm7OQejpJrI?=
 =?us-ascii?Q?sQGS43+cMzaKiR75oQo79jmqKpaww0XYPl5D92HuxNbeYyvOF06MCnQmGWdR?=
 =?us-ascii?Q?AAG2ZTOzJ2EEJu+SmodeWgBRBQfuwvSp5ajbRcHR/LugE/KuGf6Vhly/GDZQ?=
 =?us-ascii?Q?Czmut7XPdK0xs8nugqnXZdPywA8TZMx3MuR/wFtCVDL3mqmETbMZ7yg5dr6w?=
 =?us-ascii?Q?7MJhZiWPHIWhEmHPzCUbhxuBaTCp3dDHxc2HZ6NlHXpwaXmMYjZHJX1lXN/b?=
 =?us-ascii?Q?Th0U9U0GdAWEq/DTa2X4TSG8788EziV/vdH9KLLDGZsDUrBDycda80Hx9hJu?=
 =?us-ascii?Q?yf+8EJxguWLUd0DwU9HzXyeTZGcMSRxExgZmOy5Pdbh+At0d1ZzD18lbq3lV?=
 =?us-ascii?Q?XNGROav7cVA2CSJyKrGrPnOf+k1ePpbYzi5H2wAT?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 124ee34f-a856-4990-83c1-08db8e72b1f7
X-MS-Exchange-CrossTenant-AuthSource: MN0PR11MB6206.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 27 Jul 2023 07:25:46.8287
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 46c98d88-e344-4ed4-8496-4ed7712e255d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: miOcI1PwST8/iVGwHLCLIF/Kr/BlajTsRjzVDnqXkO/LD1+No/jt7XPNl8ckr46UQpkJAFnGRui+ZXl94OAD2g==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: MW4PR11MB7101
X-OriginatorOrg: intel.com
X-Spam-Status: No, score=-4.4 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,
        DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_MED,
        SPF_HELO_NONE,SPF_NONE,T_SCC_BODY_TEXT_LINE autolearn=ham
        autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On 2023-07-26 at 15:06:12 +0530, Shrikanth Hegde wrote:
> When the system is fully busy, there will not be any idle CPU's.
> In that case, load_balance will be called mainly with CPU_NOT_IDLE
> type. In should_we_balance its currently checking for an idle CPU if
> one exist. When system is 100% busy, there will not be an idle CPU and
> these idle_cpu checks can be skipped. This would avoid fetching those rq
> structures.
>

Yes, I guess this could help reducing the cost if the sched group
has many CPUs. 

> This is a minor optimization for a specific case of 100% utilization.
> 
> .....
> Coming to the current implementation. It is a very basic approach to the
> issue. It may not be the best/perfect way to this.  It works only in
> case of CONFIG_NO_HZ_COMMON. nohz.nr_cpus is a global info available which
> tracks idle CPU's. AFAIU there isn't any other. If there is such info, we
> can use that instead. nohz.nr_cpus is atomic, which might be costly too.
>
> Alternative way would be to add a new attribute to sched_domain and update
> it in cpu idle entry/exit path per CPU. Advantage is, check can be per
> env->sd instead of global. Slightly complicated, but maybe better. there
> could other advantage at wake up to limit the scan etc.
> 

When checking the code, I found that there is per domain nr_busy_cpus.
However that variable is only for LLC domain. Maybe extend the sd_share
for domains under NUMA is applicable IMO.

thanks,
Chenyu

> Your feedback would really help. Does this optimization makes sense?
> 
> Signed-off-by: Shrikanth Hegde <sshegde@linux.vnet.ibm.com>
> ---
>  kernel/sched/fair.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> index 373ff5f55884..903d59b5290c 100644
> --- a/kernel/sched/fair.c
> +++ b/kernel/sched/fair.c
> @@ -10713,6 +10713,12 @@ static int should_we_balance(struct lb_env *env)
>  		return 1;
>  	}
> 
> +#ifdef CONFIG_NO_HZ_COMMON
> +	/* If the system is fully busy, its better to skip the idle checks */
> +	if (env->idle == CPU_NOT_IDLE && atomic_read(&nohz.nr_cpus) == 0)
> +		return group_balance_cpu(sg) == env->dst_cpu;
> +#endif
> +
>  	/* Try to find first idle CPU */
>  	for_each_cpu_and(cpu, group_balance_mask(sg), env->cpus) {
>  		if (!idle_cpu(cpu))
> --
> 2.31.1
>