RE: [PATCH] tick: add check for the existence of broadcast clockevent device

[Tang, Feng]

>-----Original Message-----
>From: Thomas Gleixner [mailto:tglx@xxxxxxxxxxxxx]
>Sent: 2009年6月8日 15:00
>To: Tang, Feng
>Cc: mingo@xxxxxxx; linux-kernel@xxxxxxxxxxxxxxx; Li, Shaohua; Pan, Jacob jun
>Subject: Re: [PATCH] tick: add check for the existence of broadcast clock event
>device
>
>On Mon, 8 Jun 2009, Feng Tang wrote:
>> On Mon, 8 Jun 2009 14:33:14 +0800 Thomas Gleixner <tglx@xxxxxxxxxxxxx> wrote:
>> > On Mon, 8 Jun 2009, Feng Tang wrote:
>> > > Our apbt driver is pretty similar with HPET's, including its cpu
>> > > hotplug notifier. But our platform only has 2 available apbt to
>> > > use, otherwise we will configure it just like HPET, using one timer
>> > > as bc and others for per-cpu ones, then it won't hit this case
>> > >
>> > > There are 2 situations, one is for the normal boot, apbt0 will be
>> > > inited first and registered to OS as cpu0's timer, then tsc/lapic
>> > > is calculated based on it, and apbt1 is registered later in a
>> > > fs_initcall() (just like hpet.c does) after basic kernel core is
>> > > up. so the sequence is: apbt0 --> lapic0 --> lapic1 --> apbt1
>> >
>> > Hmm, I do not like that at all. That explicitely relies on CPU0 doing
>> > some work which will kick CPU1. That's fragile as hell.
>>
>> I understand the concern. apbt0 is inited in a very early boot phase when
>> the cpu1 is not up yet, and os don't even know wether there is a cpu1, that's
>> why we registered apbt1 in fs_initcall(). If we explicitly setup apbt1 when
>> OS brings up cpu1, it is a little brutal and not generic as only our platform
>> has apbt, and I guess cpu hotplug maintainer won't like it :p
>
>Why is that a problem ? You already have a special case for apbt0 in
>the early setup code. So where is the problem when you have an apbt1
>init call on CPU1 _before_ the local APIC is initialized on CPU1.

Yes, your proposal makes good sense. But our apbt0 initialization is an x86 legacy one like HPET and is put into the same position where hpet_enable() exists, it keeps the impact to x86 arch dependent code be minimal, if we put the apbt1 init code into x86's cpu initialization code, I think it will cause much more complains, that's why we chose to follow hpet's method to use delayed init.

The reason we proposed to add pointer check is there are already such checks in the broadcast code, adding these won't do harm to existing code. 

Thanks,
Feng
>
>That's definitely saner than relying on magic IPI wakeups.
>
>Thanks,
>
>	tglx
㈤旃??????+-遍荻?w??笔???dz罐??骅w*jg??????/??罐????璀??摺?囤??????:+v???佶>W?贽i?xPj???-?+?d?