| Message ID | 1536126727-11629-1-git-send-email-neeraju@codeaurora.org (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Delegated to: | Andy Gross |
| Headers | show |
| Series | cpu/hotplug: Fix acquire of st->should_run in cpuhp_thread_fun() | expand |
On Wed, Sep 05, 2018 at 11:22:07AM +0530, Neeraj Upadhyay wrote: > The smp_mb() in cpuhp_thread_fun() appears to be misplaced, and > need to be after the load of st->should_run, to prevent > reordering of the later load/stores w.r.t. the load of > st->should_run. I'm tempted to say this changelog needs improvement, but given the comment that goes with the smp_mb(), yes this is obviously right. Acked-by: Peter Zijlstra (Intel) <peterz@infraded.org> > Signed-off-by: Neeraj Upadhyay <neeraju@codeaurora.org> > --- > kernel/cpu.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/kernel/cpu.c b/kernel/cpu.c > index aa7fe85..eb4041f 100644 > --- a/kernel/cpu.c > +++ b/kernel/cpu.c > @@ -607,15 +607,15 @@ static void cpuhp_thread_fun(unsigned int cpu) > bool bringup = st->bringup; > enum cpuhp_state state; > > + if (WARN_ON_ONCE(!st->should_run)) > + return; > + > /* > * ACQUIRE for the cpuhp_should_run() load of ->should_run. Ensures > * that if we see ->should_run we also see the rest of the state. > */ > smp_mb(); > > - if (WARN_ON_ONCE(!st->should_run)) > - return; > - > cpuhp_lock_acquire(bringup); > > if (st->single) {
diff --git a/kernel/cpu.c b/kernel/cpu.c index aa7fe85..eb4041f 100644 --- a/kernel/cpu.c +++ b/kernel/cpu.c @@ -607,15 +607,15 @@ static void cpuhp_thread_fun(unsigned int cpu) bool bringup = st->bringup; enum cpuhp_state state; + if (WARN_ON_ONCE(!st->should_run)) + return; + /* * ACQUIRE for the cpuhp_should_run() load of ->should_run. Ensures * that if we see ->should_run we also see the rest of the state. */ smp_mb(); - if (WARN_ON_ONCE(!st->should_run)) - return; - cpuhp_lock_acquire(bringup); if (st->single) {
The smp_mb() in cpuhp_thread_fun() appears to be misplaced, and need to be after the load of st->should_run, to prevent reordering of the later load/stores w.r.t. the load of st->should_run. Signed-off-by: Neeraj Upadhyay <neeraju@codeaurora.org> --- kernel/cpu.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)