[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <1327445577.2568.8.camel@bwh-desktop>
Date: Tue, 24 Jan 2012 22:52:57 +0000
From: Ben Hutchings <bhutchings@...arflare.com>
To: "Yu, Fenghua" <fenghua.yu@...el.com>
CC: Ingo Molnar <mingo@...e.hu>, Thomas Gleixner <tglx@...utronix.de>,
H Peter Anvin <hpa@...or.com>,
Linus Torvalds <torvalds@...ux-foundation.org>,
Andrew Morton <akpm@...ux-foundation.org>,
"Mallick, Asit K" <asit.k.mallick@...el.com>,
"Luck, Tony" <tony.luck@...el.com>,
"Van De Ven, Arjan" <arjan.van.de.ven@...el.com>,
"Siddha, Suresh B" <suresh.b.siddha@...el.com>,
"Brown, Len" <len.brown@...el.com>,
Randy Dunlap <rdunlap@...otime.net>,
"Srivatsa S. Bhat" <srivatsa.bhat@...ux.vnet.ibm.com>,
Konrad Rzeszutek Wilk <konrad.wilk@...cle.com>,
Peter Zijlstra <peterz@...radead.org>,
Chen Gong <gong.chen@...ux.intel.com>,
linux-kernel <linux-kernel@...r.kernel.org>,
linux-pm <linux-pm@...r.kernel.org>, x86 <x86@...nel.org>
Subject: RE: [PATCH v5 03/12] x86/topology.c: Support functions for CPU0
online/offline
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@...el.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.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists