> I also have some plans and ideas about further interface improvements to
> make it more flexible and convenient, and definitely will have to add
> more error checking. However I decided to submit the "bare bones" patch
> to emphasize key points of code changes. I will appreciate any feedback
> and suggestions for the patch.
You should really cc lhcs-devel@xxxxxxxxxxxxxxxxxxxxx,
linux-kernel@xxxxxxxxxxxxxxx, Li Shaohua <shaohua.li@xxxxxxxxx>, and
probably Zwane Mwaikambo. It's also nice to put .patch on the end of
the filename of patches. Also, create those patches with the -p option.
I'll skip the things Ashok Raj already commented on, except to say that
I agree with his comments.
> /* no utility in registering a disabled processor */
> if (processor->flags.enabled == 0)
>+#ifdef CONFIG_HOTPLUG_CPU
>+ printk("proc %d is disabled\n", processor->id);
>+#else
> return 0;
>+#endif
You are going to have to clean up your printks before this patch goes
anywhere. Also with these changes the comment doesn't make any sense.
Can the whole "if(...) return 0" thing just be removed altogether?
>- if (cpu_possible(i)) arch_register_cpu(i);
>+#ifndef CONFIG_HOTPLUG_CPU
>+ if (cpu_possible(i))
>+#endif
>+ arch_register_cpu(i);
If CONFIG_HOTPLUG_CPU is defined the cpus will be possible anyway. This
isn't a performance sensitive path and this diff really uglies up the
code. I'd leave the whole file alone.
>+ if (system_state != SYSTEM_RUNNING) // not the >runtime boot
>+ write_tsc(0, 0);
>+ else
>+ >write_tsc(tsc_values[master]._part.tsc_value_high,
>+ >tsc_values[master]._part.tsc_value_low);
Probably better to do a (system_state < SYSTEM_RUNNING here).
