Received-SPF: pass (google.com: domain of linux-kernel+bounces-52596-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:4601:e00::3 as permitted sender) client-ip=2604:1380:4601:e00::3;
Message-ID: <1b25b492-b9e7-4411-90d1-463d44084043@oracle.com>
Date: Mon, 5 Feb 2024 14:28:13 +0200
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH 7/7] cpuidle/poll_state: replace cpu_relax with
 smp_cond_load_relaxed
Content-Language: ro
To: Will Deacon <will@kernel.org>
Cc: linux-arm-kernel@lists.infradead.org, kvm@vger.kernel.org,
        linux-pm@vger.kernel.org, linux-kernel@vger.kernel.org,
        catalin.marinas@arm.com, tglx@linutronix.de, mingo@redhat.com,
        bp@alien8.de, x86@kernel.org, hpa@zytor.com, pbonzini@redhat.com,
        wanpengli@tencent.com, vkuznets@redhat.com, rafael@kernel.org,
        daniel.lezcano@linaro.org, akpm@linux-foundation.org, pmladek@suse.com,
        peterz@infradead.org, dianders@chromium.org, npiggin@gmail.com,
        rick.p.edgecombe@intel.com, joao.m.martins@oracle.com,
        juerg.haefliger@canonical.com, mic@digikod.net, arnd@arndb.de,
        ankur.a.arora@oracle.com
References: <1700488898-12431-1-git-send-email-mihai.carabas@oracle.com>
 <1700488898-12431-8-git-send-email-mihai.carabas@oracle.com>
 <20231211114642.GB24899@willie-the-truck>
 <1b3650c5-822e-4789-81d2-0304573cabd9@oracle.com>
 <20240129181547.GA12305@willie-the-truck>
From: Mihai Carabas <mihai.carabas@oracle.com>
In-Reply-To: <20240129181547.GA12305@willie-the-truck>
Content-Type: text/plain; charset=UTF-8; format=flowed
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?bGdpdzNzd3VWV1FxL1phN0ZwYzhFaU9yVEtFNlg5Z2RFcmRHRi96bVkxV2tU?=
 =?utf-8?B?aG9Ca2RnZjEzQVIyclNPK3ZYbmpjL2duekgxUXNhWEhCczBrNkVyTE1xakU3?=
 =?utf-8?B?ZTl2bGtFdDBhOGxjQTZsKzlmNHBNQUN2b01ESXlXTlZqL21GVmgrY1NpRFM2?=
 =?utf-8?B?dUJXWmJCVnppclFPYXZacUZUeGVVbk8vZGxNUzVsWFYrZThVcGpTVHJNcDZa?=
 =?utf-8?B?MHh0OGx2MUgyM3NPN0owNGowTGNsaHQ1MEJFUXBTTW1wN2R4TEIxdXFmNGhl?=
 =?utf-8?B?NXJaRlpOUEVBTCttOTFhUDM0UE94UHMrbE9QbG5QS1NvMHZXL281YThaWEZw?=
 =?utf-8?B?MXJ2SENsbVZUb1g1S2YxYnE5WC9pOWNQZm1NY0d0NWZmc1NjUnpFaVUrVWpF?=
 =?utf-8?B?N0ZUTjdraFNiTGc0QlBza0JCU2xUR2I1MUxwQk41SXhObmxqZGkyYXRUaGUr?=
 =?utf-8?B?Vk1WMGNvWUJ6eUJFVDlKemc4MW5SbVdyQ0wyc1VUallGLzdVUDlkOU93dkM1?=
 =?utf-8?B?UnlBa2g0UnhpNm55Skx3dDg3ZjJXOEJHVndsRGNyMERKakRPV09aQ3ZLaThz?=
 =?utf-8?B?K1EwOFp2Y0E3ODBwODQvYml5Smc1OGZHdlJrU0RQN3huUktYRDJlZ2RiNFQy?=
 =?utf-8?B?SHJRMFN4WnRnaHVsOEtvbjRZd21jUThnK1ZYZE9pTmVtL1liM04rai9WdXdx?=
 =?utf-8?B?eHB3bmJMeUhQcCtGYTU4WVFUT2FCaWpLRWpUVm1xNXZVWSsrY1grUnVtNWJX?=
 =?utf-8?B?Y1lWM1JKOEFWUUsrQWNFc1RWN3E4WG1wM1ZvWlhmbXVvN1ZlTkd1UkFUV1Qw?=
 =?utf-8?B?SHdoNzNiSmtPWmNleHdpREpZaUZzV3ExNmZuZGU3N3I1MW9SUmNFeERIblZu?=
 =?utf-8?B?ZkVSOGZzWkg1cVFsSjExN05NMDN3U1E1Yzc2QkhVQlFBQ0pZZVNEV0lZNjVP?=
 =?utf-8?B?SThSY3c5QzVCYlYzQWhlYkxyUzFoZkh2Y2ZERllHcVFRdWRCd20rQVN0WGd0?=
 =?utf-8?B?SW54dEEwR0dSeHNUWU8wby9RbGhvYTc3NEJWd01VOCt2REZsaksxb1BtQXlU?=
 =?utf-8?B?dFBLaDVrKytNTzhTNEU5czlxNWEyODVRRnl4TW9lV3NtL0dGMitmZW5yMnR2?=
 =?utf-8?B?TStBTkdiZnExblRpM0xRNllOY1hGWHZiOFZkako1MmpPOFNUN2daclJDOEl0?=
 =?utf-8?B?YzNGcm80Tzc0OVliTi9KRGxsa3EyNXNWY1psYU9kUGtrdDVvbDB6TmlRVHZY?=
 =?utf-8?B?UDlidjNSazMxS0MwTEtmOEF1eFFUMnZLUXhwYkphTnRUK0xhSW5QdHNYNTY4?=
 =?utf-8?B?eVdMRDd5UHhoU2lMYVo3em40TzlLbWlqNExPUWZRcVlQWVBIQmE3ckpVVTB4?=
 =?utf-8?B?R0lLODdvcVo4ZTJqaXl0eEJyU1c2c0NsYVNtN3l2YWRqb0NNellvMjJDUVJ0?=
 =?utf-8?B?bkhEb2YyRzY2SXp6VlBVbEJlOHE3NzVYbnNramtKSHpLc1FIN05Oamp1ZGJI?=
 =?utf-8?B?dGpzSVlyQkkxeDFLSTh3a2xKOFBPVG9tMGx1V2dzRWZoeFBZUVBEUEQ1M0hT?=
 =?utf-8?B?VmVzV1dTbTVjS1VYNEFMRVlVdVlpci80TmpnUHoxU1BvdGZkcytSdk9HMjNh?=
 =?utf-8?B?RWFxY2lLR1NqdUFqM3J2Y0pRV2NpejFWSnJibmxzdEg3NWc4VFZSQ1ROaGw5?=
 =?utf-8?B?MGpKbk45b3lYc28vRml1YmNHalk5WDRYWEVmMkNydmZ5YXpXUUR3QzI3U2F4?=
 =?utf-8?B?VHNoR0ltOTJRQmkxMkVEYUFIeGhydXhVYTZQanlFbzQvTDhvbXhaMFhPaFd5?=
 =?utf-8?B?alV5SEE4NHZxQjJPK2paWk40V2RRb0FTQk1vU0JLNXcyZkkwM0ZMMHd0MHVD?=
 =?utf-8?B?YTVTN0Nqbld6ZDVTUlBlL2FOUUc0ZnEveHRCbXBFYkp6ekZFVHRLNEZSYnl1?=
 =?utf-8?B?SU51K3Axc090bzJlZGkvRjBOMkFITXl3TWJwM1dnVFFQVXpWVWoyejB1dVpB?=
 =?utf-8?B?a1U0U0UzOXo3VGZPUm9yMXh3aHBHK1UvWlBrL3RINUp2R3VhOU1wL3FaTGN5?=
 =?utf-8?B?WUc5NUlNRE04QkY3aGsyM0RiNUxBdGQ2Zm56azNIcXREdTVab24rcWU4TFFQ?=
 =?utf-8?B?aDBnQ2ZreEJWNm5YNmZ6TGhxb2phaUltRnRhS1R6cnB6b1E4OWhmbStIdlZL?=
 =?utf-8?B?Snc9PQ==?=
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-0: 
	knlu53Dqu8HD4xeSgny0dWcmE8+FH/SEyMYBoeWYoBnXQ04MGnb10eXmzcMLkdzdSqqXbKBzmY2iZmqk5zZHUsJNy/9xbmq+r/QgF28t27UC0NTYXh4t9iofTaJ2ACKktjVOrpxZqY3M5hbMQt1I9z+X+Bq8nP9EZ4dSEmUfLnCkTxvECcagxSZyTYN7kcSqVH07V77SlP8WP4qx+RgSZfoqWkzTU23edTZcmRFRG+yBVb+9aINMwOLQsRNynfjPUM03W0XpGuPvjiriZut/4S5Vy3DU6XwDPYARxTCvoDho7a+RTJ5EOkFQhUcWDSvN2o4m6SH+Ha+ePIwTyOP0y52bpTNEfh2RprWKMB59TyziSERpbiv7ISy7GFLAXEOeAvblEIpHRT6L/ys/KDbsvd5XrSvcgnkNWOHaD6d4pYYvT5jhNZHh3j5/gmy4v/i2zAx42BoPEuEO3rjDwXER6KginBE0NHAUuGKRnmsSypYXvN1ppHws4TQuez0H9+sqaDwSQvD8OGS5KM5tmiu4LirApZcf0FnoYBWCkxDvIAwvSm+o9tSOtEx2ZJ+CIF+gfCVBT+YjNmfrht9BnybbAWxVQoPnKhUqICDgSw+C/J0=
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 31f3b7e1-12ae-40a6-50cf-08dc2645f0a6
X-MS-Exchange-CrossTenant-AuthSource: PH0PR10MB5894.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 05 Feb 2024 12:28:21.4522
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 4e2c6054-71cb-48f1-bd6c-3a9705aca71b
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: 2J0GYQq5Pq0OkAwohd2ysxhV5XKHoHDySKtbm9j8K4Zol3z71SfdmoPqJhb/ECW3JQuV0XmVzTKI6h7y6Plo/yM9DycJJRr99W+tDXbN07c=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: BY5PR10MB4260
X-Proofpoint-Virus-Version: vendor=baseguard
 engine=ICAP:2.0.272,Aquarius:18.0.1011,Hydra:6.0.619,FMLib:17.11.176.26
 definitions=2024-02-05_06,2024-01-31_01,2023-05-22_02
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 phishscore=0 suspectscore=0 mlxscore=0
 adultscore=0 spamscore=0 malwarescore=0 mlxlogscore=999 bulkscore=0
 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2311290000
 definitions=main-2402050094
X-Proofpoint-ORIG-GUID: tVf-ydn39PEZJNy2uFYdfuwdFyFvmM2M
X-Proofpoint-GUID: tVf-ydn39PEZJNy2uFYdfuwdFyFvmM2M

>>>> cpu_relax on ARM64 does a simple "yield". Thus we replace it with
>>>> smp_cond_load_relaxed which basically does a "wfe".
>>>>
>>>> Suggested-by: Peter Zijlstra <peterz@infradead.org>
>>>> Signed-off-by: Mihai Carabas <mihai.carabas@oracle.com>
>>>> ---
>>>>    drivers/cpuidle/poll_state.c | 14 +++++++++-----
>>>>    1 file changed, 9 insertions(+), 5 deletions(-)
>>>>
>>>> diff --git a/drivers/cpuidle/poll_state.c b/drivers/cpuidle/poll_state.c
>>>> index 9b6d90a72601..440cd713e39a 100644
>>>> --- a/drivers/cpuidle/poll_state.c
>>>> +++ b/drivers/cpuidle/poll_state.c
>>>> @@ -26,12 +26,16 @@ static int __cpuidle poll_idle(struct cpuidle_device *dev,
>>>>    		limit = cpuidle_poll_time(drv, dev);
>>>> -		while (!need_resched()) {
>>>> -			cpu_relax();
>>>> -			if (loop_count++ < POLL_IDLE_RELAX_COUNT)
>>>> -				continue;
>>>> -
>>>> +		for (;;) {
>>>>    			loop_count = 0;
>>>> +
>>>> +			smp_cond_load_relaxed(&current_thread_info()->flags,
>>>> +					      (VAL & _TIF_NEED_RESCHED) ||
>>>> +					      (loop_count++ >= POLL_IDLE_RELAX_COUNT));
>>>> +
>>>> +			if (loop_count < POLL_IDLE_RELAX_COUNT)
>>>> +				break;
>>>> +
>>>>    			if (local_clock_noinstr() - time_start > limit) {
>>>>    				dev->poll_time_limit = true;
>>>>    				break;
>>> Doesn't this make ARCH_HAS_CPU_RELAX a complete misnomer?
>> This controls the build of poll_state.c and the generic definition of
>> smp_cond_load_relaxed (used by x86) is using cpu_relax(). Do you propose
>> other approach here?
> Give it a better name? Having ARCH_HAS_CPU_RELAX control a piece of code
> that doesn't use cpu_relax() doesn't make sense to me.

The generic code for smp_cond_load_relaxed is using cpu_relax and this 
one is used on x86 - so ARCH_HAS_CPU_RELAX is a prerequisite on x86 when 
using haltpoll. Only on ARM64 this is overwritten. Moreover 
ARCH_HAS_CPU_RELAX is controlling the function definition for 
cpuidle_poll_state_init (this is how it was originally designed).

Thanks,
Mihai