Messages in this thread | | | Date | Mon, 6 Jun 2005 15:43:26 -0700 | From | Ashok Raj <> | Subject | Re: [patch 2/5] try2: x86_64: CPU hotplug support. |
| |
On Mon, Jun 06, 2005 at 03:11:56PM -0700, Andrew Morton wrote: > Ashok Raj <ashok.raj@intel.com> wrote: > > > > Experimental CPU hotplug patch for x86_64 > > What does "experimental" mean?
Well, stictly since these sections are still under CONFIG_EXPERIMENTAL in arch/x86_64/Kconfig. Evolving... ?
> > > + > > + if (!keventd_up() || current_is_keventd()) > > + work.func(work.data); > > + else { > > + schedule_work(&work); > > + wait_for_completion(&c_idle.done); > > + } > > This shouldn't be diddling with workqueue internals. Why is this code > here? If the workqueue API is inadequate then we should prefer to extend > it rather than working around any shortcoming.
This has been around for ages.. even in ia64 code. For forking idle threads we want to do them in clean state so we dont acquire state from threads from where the cpu_up is being invoked. Hence we want them to start from keventd() threads. But when system boot is happening, there is no keventd() yet, hence we need to create them right away.
the other problem we ran into was ACPI code that handles physical cpu hotplug also queues to keventd(), this becomes permanently blocking when called from code already running in kevend().
> > + Dprintk ("do_boot_cpu %d Already started\n", cpu); > > Please try to adopt a consistent coding style.
I was actually trying to be consistent :-), rest of the debug code was under Dprintk() hence didnt want to use a new style. Not sure what you need here exactly. Do you want to convert the rest of the code to not use Dprintk()? or just leave this with a printk? I dont have a particular preference here... i would rather leave it with Dprintk() as the rest of the debug code. > > Using printk("%s", __FUNCTION__); is preferred, as it will still work if > someone later refactors this code into a new function. (It can increase > code size. Or decrease it if the string gets shared. But that's moot if > the code is inside a normally-disabled macro like Dprintk. Whatever that > is.) > > > +static void > > +remove_siblinginfo(int cpu) > > Unneeded newline here.
I can remove.. when iam inside the file, iam used to search for fn;s from start of line.. no biggie.. can revert.
Cheers, ashok - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |