Tomohiro Misono (Fujitsu) <misono.tomohiro@xxxxxxxxxxx> 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@xxxxxxxxxxxxx>
> > Signed-off-by: Mihai Carabas <mihai.carabas@xxxxxxxxxx>
> > ---
> > 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(¤t_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
