Michael Ellerman <m...@ellerman.id.au> writes: > Daniel Henrique Barboza <danielhb...@gmail.com> writes: >> This is enough to say that we can't easily see the history behind this >> comment. >> I also believe that we're better of without it since it doesn't make sense >> with the current codebase. > > It was added by the original CPU hotplug commit for ppc64:: > > https://github.com/mpe/linux-fullhistory/commit/0e9fd9441cd2113b67b14e739267c9e69761489b > > > The code was fairly similar: > > void __cpu_die(unsigned int cpu) > { > int tries; > int cpu_status; > unsigned int pcpu = get_hard_smp_processor_id(cpu); > > for (tries = 0; tries < 5; tries++) { > cpu_status = query_cpu_stopped(pcpu); > > if (cpu_status == 0) > break; > set_current_state(TASK_UNINTERRUPTIBLE); > schedule_timeout(HZ); > } > if (cpu_status != 0) { > printk("Querying DEAD? cpu %i (%i) shows %i\n", > cpu, pcpu, cpu_status); > } > > /* Isolation and deallocation are definatly done by > * drslot_chrp_cpu. If they were not they would be > * done here. Change isolate state to Isolate and > * change allocation-state to Unusable. > */ > paca[cpu].xProcStart = 0; > > /* So we can recognize if it fails to come up next time. */ > cpu_callin_map[cpu] = 0; > } > > > drslot_chrp_cpu() still exists in drmgr: > > > https://github.com/ibm-power-utilities/powerpc-utils/blob/e798c4a09fbf0fa0f421e624cfa366a6c405c9fe/src/drmgr/drslot_chrp_cpu.c#L406 > > > I agree the comment is no longer meaningful and can be removed.
Thanks for providing this background. > It might be good to then add a comment explaining why we need to set > cpu_start = 0. Sure, I can take that as a follow-up. Or perhaps it should be moved to the online path. > It's not immediately clear why we need to. When we bring a CPU back > online in smp_pSeries_kick_cpu() we ask RTAS to start it and then > immediately set cpu_start = 1, ie. there isn't a separate step that sets > cpu_start = 1 for hotplugged CPUs. Hmm I'm not following the distinction you seem to be drawing between bringing a CPU back online and a hotplugged CPU. kick_cpu is used in all cases AFAIK.