Received-SPF: pass (google.com: domain of linux-kernel+bounces-84509-linux.lists.archive=gmail.com@vger.kernel.org designates 139.178.88.99 as permitted sender) client-ip=139.178.88.99;
From: Ankur Arora <ankur.a.arora@oracle.com>
To: misono.tomohiro@fujitsu.com
Cc: akpm@linux-foundation.org, ankur.a.arora@oracle.com, arnd@arndb.de,
        bp@alien8.de, catalin.marinas@arm.com, daniel.lezcano@linaro.org,
        dianders@chromium.org, hpa@zytor.com, joao.m.martins@oracle.com,
        juerg.haefliger@canonical.com, kvm@vger.kernel.org,
        linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org,
        linux-pm@vger.kernel.org, mic@digikod.net, mihai.carabas@oracle.com,
        mingo@redhat.com, npiggin@gmail.com, pbonzini@redhat.com,
        peterz@infradead.org, pmladek@suse.com, rafael@kernel.org,
        rick.p.edgecombe@intel.com, tglx@linutronix.de, vkuznets@redhat.com,
        wanpengli@tencent.com, will@kernel.org, x86@kernel.org
Subject: RE: [PATCH v4 7/8] cpuidle/poll_state: replace cpu_relax with smp_cond_load_relaxed
Date: Tue, 27 Feb 2024 20:36:45 -0800
Message-Id: <20240228043645.2716589-1-ankur.a.arora@oracle.com>
In-Reply-To: <TY2PR01MB4330F7FF122BBF12EC0ADE06E55A2@TY2PR01MB4330.jpnprd01.prod.outlook.com>
References: <TY2PR01MB4330F7FF122BBF12EC0ADE06E55A2@TY2PR01MB4330.jpnprd01.prod.outlook.com>
Content-Transfer-Encoding: 8bit
Content-Type: text/plain
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: 
	=?us-ascii?Q?yCNTkn4vcyFscRh6vqv2/9Ejcj2PRr/Zfu5I99V/ASPK6/AKqUQljfH3zfr6?=
 =?us-ascii?Q?dPMjkqmeWK4SYXxDLIbpvgfmKlOQfw1uDeXYPncWt9vVcxiI8JQL1g8kxjB1?=
 =?us-ascii?Q?cveXHHBeWz3s2YvJTPa3OxE2UnSiBdttr74XOClNNb3nwuRrkGSDjtrA1SsV?=
 =?us-ascii?Q?2rARpK692n+w0gU/DvQw6rfbUyhez1VQCTK4JDvLuYkyzQ/gI1ZGpjp8RJBg?=
 =?us-ascii?Q?yl1m3jZfn3KgENGf0RlJ9bVX1sitLiPibnK4XigIO8rVCw6WM6whS9gocIKy?=
 =?us-ascii?Q?Q+uhQyRnY3fH3WeWpDVOcuvkzCJsM7sOrLnhbRBaLugCaFk2LEUXcbdsD2L0?=
 =?us-ascii?Q?nlHOWufheqSIc5kYdfPCfdRzdFI4Gw7ta0hNtzJsTzRpT5/xyQRu2YZ7IZYW?=
 =?us-ascii?Q?FnvfCShjgqbd3P7r67bUxImlIvd+1vvO2+ApRO0mVzdTFgal/XIapChZEEIl?=
 =?us-ascii?Q?nlylXFFX89KfqZgZjNz36gWZ/i9OzvSeaO4bTTDaQKBmNm42z34Mh7kmBbvj?=
 =?us-ascii?Q?qLwjbM0c9K2PZ86gPTVkc+SY0J74DKXr0D8bar8rMYF8nfrRmFUMsTiX8D3g?=
 =?us-ascii?Q?bCIHWFn7YdI1BnHHYH2iUSr5/ypa3ILVeOGXsf3fWN0bQ8QT8dSYMzVTJn/y?=
 =?us-ascii?Q?F99Pdgs3RNMnQFCh9STS1WAIlqBhuW0i3oIWo8p8H/Px0bvyqrhDrpxE+CfZ?=
 =?us-ascii?Q?muqzykTqKj9u06a+oLp91wusHlutP2NiUFjV8M2YsZV0qf/KozITOhIBBzjq?=
 =?us-ascii?Q?i10cVzxZpvZTEsDAjfPUKxaVSGQKpj8v3S16zfCz9y1QB47PgW9rOp92Y+TO?=
 =?us-ascii?Q?l80tF4cPtpG2NdgdhAXHaLqTQ1llr2usPMRSFgRj3SZJbSX5L8YtvQBTTg6M?=
 =?us-ascii?Q?RozWbhAMe7NDeGNEXnLf+BdXKSZ8o5U3Wjh/I0gEa+jiMkLppjbzl7JlR+hA?=
 =?us-ascii?Q?eysgGzWFLOjOPwT5lyp4muCxR+XwBCG1sJYe9H/KFksW5C0ZfggRri6t3Wff?=
 =?us-ascii?Q?w23uoNIOTu414MMif1F2PtQyftTZ6qPhEFv5jefHCDi3sErnSXm7tB6E/OGU?=
 =?us-ascii?Q?DuWL8TE9ZALd1bFa1CjgOwP4ja7/+jeOVmgQbEzs3l6dMesO//+xIuni4PQl?=
 =?us-ascii?Q?hLv8k0435ZFu+kkyzptojh26BJS37hQfITvxVGgeTimHyxjPTPwE8EjgLJFd?=
 =?us-ascii?Q?GcEpSb8OHr2lL7KWTC0Zq4WbgBote0eQ+/5kUwgqieoIdp+/3k5qRyC4RN78?=
 =?us-ascii?Q?oN7Ey9l1BeTNtMpKXfFREuB4/EmKUkQ/6YU2KaV4ym2BeNSlsYju02YxpsWj?=
 =?us-ascii?Q?ojuTKcq0kMsnBH4dxl+e4Ge7NP0UC8adskY415MfN2fIc8rnqz4dYy7Hcr7B?=
 =?us-ascii?Q?tt6Xv5nSejhdMyF+LTZZpF2ElAVJ2AmnKxfPTOe8oopOuRR4PBE2YgwcNVRH?=
 =?us-ascii?Q?BRvTHqCttk/m0q08t746WeCYavc2cN1dm1W+gcMJ92OyJcr+TTJDMtSN4CNQ?=
 =?us-ascii?Q?2Vpp6seuJ3MWlol/uVoHSBLbWZLTHj5+aMnj5ioxXnKVbzlXWEzZ6RGxswA9?=
 =?us-ascii?Q?ii2VhaZvBhMrwGLv3Anyfhu6U6XA8zxxtt43b+RQqr63th/0hBF+cqX2DqTa?=
 =?us-ascii?Q?+A=3D=3D?=
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-0: 
	QkmbLVH6V9x/sc1OOlU1szMxf2xjyiwLbnQRxn52pi7UfdQTACZa8bzj2yO7tVjJ2B0N/TS5haC00u5xUoZM8bNrVvGJtd0ga/+6y9vZXWLm13kcn/a6zAgl1LG41Z+Qx+jQz5xH8S/L5v2hT75CMXp28DufX4yX3evANIS0UiZAIgQthnZIzJJmoxeWK0nhG6oEcNq85lenkLu2ABcE+lSNlqQT9pner1LnWCEG0ct/ZdIGGpqk1deN0dGZKaztECgZ5HfFq7OJzBgmmuJKAmH7rKREN5ubkpQ/lsgip2ACXJhFrAyvYj1Ft124I4v5AFLkrbO9kwK6V30AHU/LiEAl4ldB/Z1hAz4UL3LGNGR8j6sZPJUmUHYwH6S1THqu2qRt/TuJobJFGzezfH153QRqeBlFnDYa703c8iHd40LUbJ3EopTHpDnL9yEWRnI1IdUz+0QmNtS4V7fWwAT6tJtr3oRKHXwbvZ3JmOwBU6H9kiV/rPHelyR8UqCi53UnKw2ordSpTLHQNxxSb9oWRRuN3H9nQJ7xTZW7xTsRCvddFxGJ8Z1/Qo5Jj+nHczfWCzlhg7H2DTzAmknJ63thxURvgMYOagtWUSG0EXNlOsE=
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-Network-Message-Id: b092f383-d6a4-4cd9-111a-08dc3816cfaf
X-MS-Exchange-CrossTenant-AuthSource: CO6PR10MB5409.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 28 Feb 2024 04:36:20.9619
 (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: 5sx2py2I/nXqytGmTJp5WYDHvKliFC2kTEqr3nE2udMzJUtDAKCEZZre+qmVJ6gPen5Fnl2vZcpYUC2uCsMjGt6POPqcnWI0EBEJ0EodkIw=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: CO1PR10MB4641
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-28_03,2024-02-27_01,2023-05-22_02
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 bulkscore=0 suspectscore=0 phishscore=0
 spamscore=0 mlxlogscore=999 malwarescore=0 adultscore=0 mlxscore=0
 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2311290000
 definitions=main-2402280033
X-Proofpoint-GUID: 83m5_lT98BSbXZjKGAV72-ioJRvqV1Ph
X-Proofpoint-ORIG-GUID: 83m5_lT98BSbXZjKGAV72-ioJRvqV1Ph

Tomohiro Misono (Fujitsu) <misono.tomohiro@fujitsu.com> writes:

> Hi,
> > Subject: [PATCH v4 7/8] cpuidle/poll_state: replace cpu_relax with smp_cond_load_relaxed
> > 
> > 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 | 15 ++++++++++-----
> >  1 file changed, 10 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/cpuidle/poll_state.c b/drivers/cpuidle/poll_state.c
> > index 9b6d90a72601..1e45be906e72 100644
> > --- a/drivers/cpuidle/poll_state.c
> > +++ b/drivers/cpuidle/poll_state.c
> > @@ -13,6 +13,7 @@
> >  static int __cpuidle poll_idle(struct cpuidle_device *dev,
> >                              struct cpuidle_driver *drv, int index)
> >  {
> > +     unsigned long ret;
> >       u64 time_start;
> >
> >       time_start = local_clock_noinstr();
> > @@ -26,12 +27,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;
> > +
> > +                     ret = smp_cond_load_relaxed(&current_thread_info()->flags,
> > +                                                 VAL & _TIF_NEED_RESCHED ||
> > +                                                 loop_count++ >= POLL_IDLE_RELAX_COUNT);
> > +
> > +                     if (!(ret & _TIF_NEED_RESCHED))
> > +                             break;
> 
> Should this be "if (ret & _TIF_NEED_RESCHED) since we want to break here
> if the flag is set, or am I misunderstood?

Yeah, you are right. The check is inverted.

I'll be re-spinning this series. Will fix. Though, it probably makes sense
to just keep the original "while (!need_resched())" check.

Thanks for the review.

--
ankur