Daniel Henrique Barboza <danielhb...@gmail.com> writes: > On 9/27/21 17:19, Nathan Lynch wrote: >> This comment likely refers to the obsolete DLPAR workflow where some >> resource state transitions were driven more directly from user space >> utilities, but it also seems to contradict itself: "Change isolate state to >> Isolate [...]" is at odds with the preceding sentences, and it does not >> relate at all to the code that follows. > > This comment was added by commit 413f7c405a34, a 2006 commit where Mike > Ellerman moved code from platform/pseries/smp.c into hotplug-cpu.c. > > I checked the original code back in smp.c and this comment was added there > by commit 1da177e4c3f41, which is Linus's initial git commit, where he > mentions > that he didn't bothered with full history (although it is available somewhere, > allegedly). > > 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. It might be good to then add a comment explaining why we need to set cpu_start = 0. 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. cheers
