Received-SPF: pass (google.com: domain of linux-kernel+bounces-34713-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.199.223 as permitted sender) client-ip=147.75.199.223;
Message-ID: <21c8694c-26e4-3bc1-edd8-2267b0164a09@amd.com>
Date: Tue, 23 Jan 2024 10:28:31 +0530
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:91.0) Gecko/20100101
 Thunderbird/91.13.1
Subject: Re: [PATCH] sched/fair: Skip newidle_balance() when an idle CPU is
 woken up to process an IPI
Content-Language: en-US
To: Tim Chen <tim.c.chen@linux.intel.com>, linux-kernel@vger.kernel.org
Cc: mingo@redhat.com, peterz@infradead.org, juri.lelli@redhat.com,
 vincent.guittot@linaro.org, dietmar.eggemann@arm.com, rostedt@goodmis.org,
 bsegall@google.com, mgorman@suse.de, bristot@redhat.com,
 vschneid@redhat.com, gautham.shenoy@amd.com,
 David Vernet <void@manifault.com>
References: <20240119084548.2788-1-kprateek.nayak@amd.com>
 <b4f5ac150685456cf45a342e3bb1f28cdd557a53.camel@linux.intel.com>
From: K Prateek Nayak <kprateek.nayak@amd.com>
In-Reply-To: <b4f5ac150685456cf45a342e3bb1f28cdd557a53.camel@linux.intel.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0:
	=?utf-8?B?K2l2YSt4ZmpFck96WmwvVUtZWmVrbWNsZG56ZGhkM0QveWVJYnY2UENzRXRM?=
 =?utf-8?B?ZHJVOTJ1YjBjeldmMVo3QWY3NnhGNVc5MHJNZmtBSjA2R0Nxc0wzdFF6KzJ6?=
 =?utf-8?B?UnE4MmcxMzMyNTBJcUNPK2NHMjU3M3lxcjgzYis5WUVsdlkxMjd4OU5ld29u?=
 =?utf-8?B?TU0rR3lVTXBqaURCMTZJY0ZzZnUvSUNka0tKbTZ4N25oSmhBME1jVmtSbFFQ?=
 =?utf-8?B?Ri9hOFIwcEVJWlVIb3FFeEwrVjBiT1lDT0Z0WUVTQ0pyeWhQaHRGUUdXanB4?=
 =?utf-8?B?MDJ3MjZieHhzSnJNYXl6aHFaa0hsYXN6VzdSVGQxVWhhbEloakoxVDFKeDk0?=
 =?utf-8?B?MTRyWTRkWTJzRUhNbmJ5aHpIekNiell5VEtndWk4aVJVenlJaTRYdGZTdmdh?=
 =?utf-8?B?eEFLUVV2M0dSczk2Ymora2RyRHpQaWhnWElyWjFxRk1SVnJlVXF5Ym1odmxE?=
 =?utf-8?B?STFiOWpRSU1ZamxJZzF1TDU0YmNzQUJjRzJTNnBwaVVhdU5TUWswZDd1dVcw?=
 =?utf-8?B?Y0pBckgwbXFmUDJVSWtuSGFEalIyMktHQWJJMSsybFplak5CMnUzRUdUc2d5?=
 =?utf-8?B?TUdIT21mYnZJWlpaKzc4SG13MFdhbnZJSitnM01iWXozY3k5YzRrcFBMOHNK?=
 =?utf-8?B?aVZKNGtrc0VCa3NWTmd4TzJwSEJrVGdCVUlVK1JnTmp6bGV1WXg4bnpLdkZm?=
 =?utf-8?B?bmd2RHpFN2xNL2Y5bnpVcWZ6T2dwYmNWaWUydHVWYVdIMWpzenlBcUVBRUFQ?=
 =?utf-8?B?d2JtejE0bUZWM3NVZHREVFdtUXR4dTN1TXFXU2RCSnVoMkN3YUNkODVoZG1P?=
 =?utf-8?B?QVdFWTYwVzNpOTd5dC9iRVZKM2xCSHlrQ2YvQ3lWUHZKaENpUUgyYk9iRnc0?=
 =?utf-8?B?cmduVUJGRTd5aXh5MUNPK1dnejFHSDMwbGpwSmZVKzZzUzkyMEVnNS9xVENB?=
 =?utf-8?B?SFdabk1HWHFIL01XaVpqNW9ZTTNTbnF3ckdWSmFJUDZZaWpPSUQ5cHlQNDd3?=
 =?utf-8?B?eXBCaFdPZllKMXRwT0RGYXJ3QnpZVmRGT3MzczZWdWsrTkF0aXd5T1VreEtx?=
 =?utf-8?B?WGlFdzluQllNSllsbXhxWk42TXZhbkNMOFJhaUMwN3dlWk93TFFKOE90a005?=
 =?utf-8?B?akZ2dTR5cmh6MlhjaCt4WXBrSmNTQU9mREtkK0VNcjJhZmxhTk8ySk9nZFNT?=
 =?utf-8?B?RDBlV1Y1bXhOekxyMVE4YjNST1FZNUZuazUyOHk1VFF0UnNuSlErc0JWd1hO?=
 =?utf-8?B?NjRJTkxxdVdoOXpwTE9HRnRtNEZHNUZKbnI1bnpKemJ0K1RUdjNmaHc1Qmly?=
 =?utf-8?B?d001WEtZdHd6czFVKzdUNHdpNTJsR0hxbXVFd3pNTXpLd3hvZDg3K2c5bUpt?=
 =?utf-8?B?TmlJYjZUc2R3YzNid0hNU0Z2WlJXaGp6NkxzRWFxaG5xSjZYbFBRd0VUdXZJ?=
 =?utf-8?B?RnF4Y0Y4b08yTXVIaTRkc09FMG04T2p0R3FoVWs2cldzQzVTbndQZUdUaVM0?=
 =?utf-8?B?WVFvSUZCNkgwUFZqMUVVRnJYbksrL2R1ZFlHMGtZNGdzR3ovR3AwZ3k4MFM5?=
 =?utf-8?B?eGIrUXFJT29Kck1mWlFMTzYwRkFGN21BVW1abWVCQndLcWNVcnp2em9hdHFN?=
 =?utf-8?B?SG9SUk1rbEcvckhZSzZqK0FPelplajloUzR0aXlSN0xyMHRiaWdwMTlIaUk5?=
 =?utf-8?B?bkVCUUU1ZXNUYVpETVJHNTRQYm5EcWR3SFdDNmM4c212YXRCSEVEcDkrY2t1?=
 =?utf-8?B?NWE4cENWY3JlU2YvTGF5TGlmL2RTdldKSS92eHk5OEtCaXBMcHd4TmtBcFJu?=
 =?utf-8?B?eEVmQTQyNnVIRkJLb0FUbmxoQURqbjdmTEZRaU9teVVCTGtJeFUxR1d2akhp?=
 =?utf-8?B?Sll0SFk4Mm14QnhLWmw5UFFmTFpNaS9uaGgyZjFFN1VJZlNCYjRHTUlTS3pm?=
 =?utf-8?B?Y3Q0QmU1NFV4MjVzOFAycHB3N0VqVTM1bHJpbFkyME12RHFLb05ISDNkWUJz?=
 =?utf-8?B?TmlhRk5HQXhjbmZoQWZFYjZGZ0c5aUdLRXl4aU8vUWZ5NHhBS0hCaE9rRGRq?=
 =?utf-8?B?aTdVRGZzbGdOLzFSQkpsN1FzOXpTVTRaenZiK3BpUmxOb2VPUStxRzZvejJr?=
 =?utf-8?Q?bWO58+KuocyTGDfjpw+8dqEzm?=
X-OriginatorOrg: amd.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 26486129-99cb-40e5-efa1-08dc1bcff7fe
X-MS-Exchange-CrossTenant-AuthSource: MW2PR12MB2379.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 23 Jan 2024 04:58:41.6301
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 3dd8961f-e488-4e60-8e11-a82d994e183d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: dIx8kBusq2eJv+GBxw8+lhVmleY6jh8/AV0CMBHDRUC4kBYFO1W19i+IWrycKVbokqIrNszu6hfB0EhGxlmWDw==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: PH8PR12MB7304

Hello Tim,

On 1/23/2024 3:29 AM, Tim Chen wrote:
> On Fri, 2024-01-19 at 14:15 +0530, K Prateek Nayak wrote:
>>
>> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
>> index b803030c3a03..1fedc7e29c98 100644
>> --- a/kernel/sched/fair.c
>> +++ b/kernel/sched/fair.c
>> @@ -8499,6 +8499,16 @@ done: __maybe_unused;
>>  	if (!rf)
>>  		return NULL;
>>  
>> +	/*
>> +	 * An idle CPU in TIF_POLLING mode might end up here after processing
>> +	 * an IPI when the sender sets the TIF_NEED_RESCHED bit and avoids
>> +	 * sending an actual IPI. In such cases, where an idle CPU was woken
>> +	 * up only to process an interrupt, without necessarily queuing a task
>> +	 * on it, skip newidle_balance() to facilitate faster idle re-entry.
>> +	 */
>> +	if (prev == rq->idle)
>> +		return NULL;
>> +
> 
> Should we check the call function queue directly to detect that there is
> an IPI waiting to be processed? something like
> 
> 	if (!llist_empty(&per_cpu(call_single_queue, rq->cpu)))
> 		return NULL;

That could be a valid check too. However, if an IPI is queued right
after this check, the processing is still delayed since
newidle_balance() only bails out for scenarios when a wakeup is trying
to queue a new task on the CPU running the newidle_balance().

> 
> Could there be cases where we want to do idle balance in this code path?
> Say a cpu is idle and a scheduling tick came in, we may try
> to look for something to run on the idle cpu.  Seems like after
> your change above, that would be skipped.

Wouldn't scheduler_tick() do load balancing when the time comes? In my
testing, I did not see a case where the workloads I tested were
sensitive to the aspect of newidle_balance() being invoked at scheduler
tick. Have you come across a workload which might be sensitive to this
aspect that I can quickly test and verify? Meanwhile, I'll run the
workloads mentioned in the commit log on an Intel system to see if I
can spot any sensitivity to this change.

Adding David to the thread too since HHVM seems to be one of those
workloads that is very sensitive to a successful newidle_balance().

> 
> Tim
> 
> 
>>  	new_tasks = newidle_balance(rq, rf);
>>  
>>  	/*
> 

--
Thanks and Regards,
Prateek