On 09/06/2018 01:48 PM, Thomas Gleixner wrote:
On Thu, 6 Sep 2018, Neeraj Upadhyay wrote:
On 09/05/2018 06:47 PM, Thomas Gleixner wrote:
On Wed, 5 Sep 2018, Neeraj Upadhyay wrote:
On 09/05/2018 05:53 PM, Thomas Gleixner wrote:
And looking closer this is a general issue. Just that the TEARDOWN state
makes it simple to observe. It's universaly broken, when the first
teardown
callback fails because, st->state is only decremented _AFTER_ the
callback
returns success, but undo_cpu_down() increments unconditionally.
As per my understanding, there are 2 problems here; one is fixed with your
patch, and other is cpuhp_reset_state() is used during rollback from
non-AP to
AP state, which seem to result in 2 increments of st->state (one increment
done by cpuhp_reset_state() and another by cpu_thread_fun()) .
And how did your hack fix that up magically? I'll have a look later today.
Thanks,
tglx
The hack fixes it by not calling cpuhp_reset_state() and doing rollback state
reset inline in _cpu_down().
And how is that any different from the proper patch I sent? It ends up in
the same state. So I have a hard time to understand your blurb above where
you claim that my patch just solves one of two problems?
Thanks,
tglx
Yes, your patch solves the problem related to smpboot unparking being
skipped during rollback and with the hack we end up in same state. The
second thing, which I am referring to, is that there is one additional
state increment. I missed the part that, it could be required, so that
we reach CPUHP_AP_ONLINE_IDLE before calling __cpuhp_kick_ap(). So, it's
not a problem.
* cpuhp_reset_state() does one state increment and we reach
CPUHP_AP_ONLINE_IDLE.
if (ret && st->state > CPUHP_TEARDOWN_CPU && st->state < prev_state) {
cpuhp_reset_state(st, prev_state);
__cpuhp_kick_ap(st);
}
CPUHP_TEARDOWN_CPU,
CPUHP_AP_ONLINE_IDLE,
CPUHP_AP_SMPBOOT_THREADS,
* cpuhp_thread_fun() does one more increment before invoking state
callback (so, we skip CPUHP_AP_ONLINE_IDLE) and we reach
CPUHP_AP_SMPBOOT_THREADS:
static void cpuhp_thread_fun(unsigned int cpu)
<snip>
if (bringup) {
st->state++;
state = st->state;
<snip>
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a
member of the Code Aurora Forum, hosted by The Linux Foundation
