Messages in this thread | | | Subject | RE: [PATCH v5 03/12] x86/topology.c: Support functions for CPU0 online/offline | From | Ben Hutchings <> | Date | Tue, 24 Jan 2012 22:52:57 +0000 |
| |
On Tue, 2012-01-24 at 22:31 +0000, Yu, Fenghua wrote: > > On Wed, 2012-01-11 at 09:04 -0800, Fenghua Yu wrote: > > > From: Fenghua Yu <fenghua.yu@intel.com> > > > > > > If CONFIG_BOOTPARAM_HOTPLUG_CPU is turned on, CPU0 hotplug feature is > > enabled > > > by default. > > > > > > If CONFIG_BOOTPARAM_HOTPLUG_CPU is not turned on, CPU0 hotplug > > feature is not > > > enabled by default. The kernel parameter cpu0_hotplug can enable CPU0 > > hotplug > > > feature at boot. > > [...] > > > int __ref arch_register_cpu(int num) > > > { > > > /* > > > - * CPU0 cannot be offlined due to several > > > - * restrictions and assumptions in kernel. This basically > > > - * doesn't add a control file, one cannot attempt to offline > > > - * BSP. > > > + * Two known BSP/CPU0 dependencies: Resume from suspend/hibernate > > > + * depends on BSP. PIC interrupts depend on BSP. > > > * > > > - * Also certain PCI quirks require not to enable hotplug control > > > - * for all CPU's. > > > + * If the BSP depencies are under control, one can tell kernel to > > > + * enable BSP hotplug. This basically adds a control file and > > > + * one can attempt to offline BSP. > > > */ > > > - if (num) > > > + if (num || cpu0_hotpluggable) > > > per_cpu(cpu_devices, num).cpu.hotpluggable = 1; > > > > > > return register_cpu(&per_cpu(cpu_devices, num).cpu, num); > > > > This change belongs at the end of the series. It should not be > > possible > > to enable CPU0 hotplug until after the hotplug logic can do it > > correctly, and this might break bisection. > > Quote from https://www.linux.com/how-to-participate-in-the-linux-community > "It can be tempting to add a whole new infrastructure with a series of > patches, but to leave that infrastructure unused until the final patch > in the series enables the whole thing. This temptation should be > avoided if possible; if that series adds regressions, bisection will > finger the last patch as the one which caused the problem, even though > the real bug is elsewhere. Whenever possible, a patch which adds new > code should make that code active immediately." > > So this patch currently is in the right place in the patch set unless > I miss something.
You're giving undue weight to that guidance. It is far more important that you do not enable features that don't work!
Ben.
-- Ben Hutchings, Staff Engineer, Solarflare Not speaking for my employer; that's the marketing department's job. They asked us to note that Solarflare product names are trademarked.
| |