Hi Joel,
Thank you for great suggestions.
> > 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 will go through yours and Ashok's (and others who give me more
feedback hopefully) items one by one and will send cleaned up patches to
the distribution that you mentioned above. I tried sending initial copy
to Zwane bit I think I had the wrong address for him.
> > /* 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?
It looks like I can just remove it, but I thought it would make sense to
keep those printk's for the accounting purposes. I often go through boot
messages checking which processors are physically present and which are
down (being assigned to different partition in ES7000 case), while all
together they total to number of available sockets. Would it be needed
info for debug? Maybe I should do KERNEL_DEBUG on those?
As for the rest of your comments, they were clear to me and I agree with
them completely. I will start making changes right away and send a new
version of the patch as soon as it is ready...
Regards,
--Natalie
