Re: [PATCH] x86_64: sync_tsc fix the race (so we can boot)
Eric, Latest tree works. YH Booting processor 1/4 APIC 0x1 Initializing CPU#1 masked ExtINT on CPU#1 Calibrating delay using timer specific routine.. 4000.31 BogoMIPS (lpj=8000624) CPU: L1 I Cache: 64K (64 bytes/line), D cache 64K (64 bytes/line) CPU: L2 Cache: 1024K (64 bytes/line) CPU 1(1) -> Node 1 -> Core 0 stepping 0a CPU 1: Syncing TSC to CPU 0. Booting processor 2/4 APIC 0x2 Initializing CPU#2 masked ExtINT on CPU#2 CPU 1: synchronized TSC with CPU 0 (last diff -4 cycles, maxerr 896 cycles) Calibrating delay using timer specific routine.. 4000.28 BogoMIPS (lpj=8000572) CPU: L1 I Cache: 64K (64 bytes/line), D cache 64K (64 bytes/line) CPU: L2 Cache: 1024K (64 bytes/line) CPU 2(1) -> Node 2 -> Core 0 stepping 0a CPU 2: Syncing TSC to CPU 0. Booting processor 3/4 APIC 0x3 Initializing CPU#3 masked ExtINT on CPU#3 CPU 2: synchronized TSC with CPU 0 (last diff -2 cycles, maxerr 909 cycles) Calibrating delay using timer specific routine.. 4000.15 BogoMIPS (lpj=8000317) CPU: L1 I Cache: 64K (64 bytes/line), D cache 64K (64 bytes/line) CPU: L2 Cache: 1024K (64 bytes/line) CPU 3(1) -> Node 3 -> Core 0 stepping 0a CPU 3: Syncing TSC to CPU 0. Brought up 4 CPUs time.c: Using PIT/TSC based timekeeping. testing NMI watchdog ... OK. checking if image is initramfs...<6>CPU 3: synchronized TSC with CPU 0 (last diff -16 cycles, maxerr 1496 cycles) - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] x86_64: sync_tsc fix the race (so we can boot)
I will use linus's latest tree to have a try. YH On 7/29/05, Eric W. Biederman <[EMAIL PROTECTED]> wrote: > yhlu <[EMAIL PROTECTED]> writes: > > > if using you patch, the > > "synchronized TSC with CPU" never come out. > > > > then with your patch, I add back patch that moving set callin_map from > > smp_callin to start_secondary. It told me can not inquire the apic for > > the CPU 12 > > Hmm. You didn't post enough of a boot log for me to see the problem. > Does it boot and you don't see the message or is it something > else. > > > Can we put tsc_sync_wait() back to smp_callin? > > > > So that it will be executed serially and we can get > > "synchronized TSC with CPU"? > > Currently that just seems silly. That code should be async > safe. > > But it sounds like you have some weird bug I don't understand. > > Eric > - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] x86_64: sync_tsc fix the race (so we can boot)
yhlu <[EMAIL PROTECTED]> writes: > if using you patch, the > "synchronized TSC with CPU" never come out. > > then with your patch, I add back patch that moving set callin_map from > smp_callin to start_secondary. It told me can not inquire the apic for > the CPU 12 Hmm. You didn't post enough of a boot log for me to see the problem. Does it boot and you don't see the message or is it something else. > Can we put tsc_sync_wait() back to smp_callin? > > So that it will be executed serially and we can get > "synchronized TSC with CPU"? Currently that just seems silly. That code should be async safe. But it sounds like you have some weird bug I don't understand. Eric - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] x86_64: sync_tsc fix the race (so we can boot)
Can we put tsc_sync_wait() back to smp_callin? So that it will be executed serially and we can get "synchronized TSC with CPU"? YH - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] x86_64: sync_tsc fix the race (so we can boot)
if using you patch, the "synchronized TSC with CPU" never come out. then with your patch, I add back patch that moving set callin_map from smp_callin to start_secondary. It told me can not inquire the apic for the CPU 12 YH Initializing CPU#1 masked ExtINT on CPU#1 Calibrating delay using timer specific routine.. 3600.30 BogoMIPS (lpj=7200601) CPU: L1 I Cache: 64K (64 bytes/line), D cache 64K (64 bytes/line) CPU: L2 Cache: 1024K (64 bytes/line) CPU 1(2) -> Node 0 -> Core 1 stepping 02 CPU 1: Syncing TSC to CPU 0. CPU 1: synchronized TSC with CPU 0 (last diff 0 cycles, maxerr 1415 cycles) On 7/28/05, Eric W. Biederman <[EMAIL PROTECTED]> wrote: > yhlu <[EMAIL PROTECTED]> writes: > > > I have some problem with this patch. > > > > YH > > > > On 7/28/05, yhlu <[EMAIL PROTECTED]> wrote: > >> Do you mean solve the timing problem for 2 way dual core or 4 way > >> single core above? > > As best as I can determine the problem is possible any time > you have more than 2 cpus (from the kernels perspective), > but you have ti hit a fairly narrow window in cpu start up. > > What problem do you have with this patch. > > Eric > - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] x86_64: sync_tsc fix the race (so we can boot)
Andi Kleen <[EMAIL PROTECTED]> writes: > On Wed, Jul 27, 2005 at 10:03:04PM -0600, Eric W. Biederman wrote: >> I believe this patch suffers from apicid versus logical cpu number confusion. >> I copied the basic logic from smp_send_reschedule and I can't find where >> that translates from the logical cpuid to apicid. So it isn't quite >> correct yet. It should be close enough that it shouldn't be too hard >> to finish it up. >> >> More bug fixes after I have slept but I figured I needed to get this >> one out for review. > > Thanks looks good. This should fix the unexplained > hang for various people. Logical<->apicid is actually ok, the low > level _mask function takes care of that (it differs depending on the > APIC mode anyways) I guess my confusion comes from looking at the code in flat_send_IPI_mask. cpus_addr(mask)[0] reduces it to just the first word but I don't see any translation being done. Eric - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] x86_64: sync_tsc fix the race (so we can boot)
Andi Kleen <[EMAIL PROTECTED]> writes: > There are some style problems, but that can be fixed later. > > How did you track that nasty it down? I had a consistent reproducer. I ruled out hangs in the tsc sync code itself, by implementing timeouts in all of the loops. I looked for differences with the ia64 code. I noticed that sometimes the machine would reboot and not just hang. I implemented smp_call_function_single since that was what the ia64 code did. It worked and all of my pieces of evidence just fell together. Eric - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] x86_64: sync_tsc fix the race (so we can boot)
On Wed, Jul 27, 2005 at 10:03:04PM -0600, Eric W. Biederman wrote: > I believe this patch suffers from apicid versus logical cpu number confusion. > I copied the basic logic from smp_send_reschedule and I can't find where > that translates from the logical cpuid to apicid. So it isn't quite > correct yet. It should be close enough that it shouldn't be too hard > to finish it up. > > More bug fixes after I have slept but I figured I needed to get this > one out for review. Thanks looks good. This should fix the unexplained hang for various people. Logical<->apicid is actually ok, the low level _mask function takes care of that (it differs depending on the APIC mode anyways) There are some style problems, but that can be fixed later. How did you track that nasty it down? -Andi - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] x86_64: sync_tsc fix the race (so we can boot)
On Wed, Jul 27, 2005 at 10:03:04PM -0600, Eric W. Biederman wrote: I believe this patch suffers from apicid versus logical cpu number confusion. I copied the basic logic from smp_send_reschedule and I can't find where that translates from the logical cpuid to apicid. So it isn't quite correct yet. It should be close enough that it shouldn't be too hard to finish it up. More bug fixes after I have slept but I figured I needed to get this one out for review. Thanks looks good. This should fix the unexplained hang for various people. Logical-apicid is actually ok, the low level _mask function takes care of that (it differs depending on the APIC mode anyways) There are some style problems, but that can be fixed later. How did you track that nasty it down? -Andi - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] x86_64: sync_tsc fix the race (so we can boot)
Andi Kleen [EMAIL PROTECTED] writes: There are some style problems, but that can be fixed later. How did you track that nasty it down? I had a consistent reproducer. I ruled out hangs in the tsc sync code itself, by implementing timeouts in all of the loops. I looked for differences with the ia64 code. I noticed that sometimes the machine would reboot and not just hang. I implemented smp_call_function_single since that was what the ia64 code did. It worked and all of my pieces of evidence just fell together. Eric - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] x86_64: sync_tsc fix the race (so we can boot)
Andi Kleen [EMAIL PROTECTED] writes: On Wed, Jul 27, 2005 at 10:03:04PM -0600, Eric W. Biederman wrote: I believe this patch suffers from apicid versus logical cpu number confusion. I copied the basic logic from smp_send_reschedule and I can't find where that translates from the logical cpuid to apicid. So it isn't quite correct yet. It should be close enough that it shouldn't be too hard to finish it up. More bug fixes after I have slept but I figured I needed to get this one out for review. Thanks looks good. This should fix the unexplained hang for various people. Logical-apicid is actually ok, the low level _mask function takes care of that (it differs depending on the APIC mode anyways) I guess my confusion comes from looking at the code in flat_send_IPI_mask. cpus_addr(mask)[0] reduces it to just the first word but I don't see any translation being done. Eric - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] x86_64: sync_tsc fix the race (so we can boot)
if using you patch, the synchronized TSC with CPU never come out. then with your patch, I add back patch that moving set callin_map from smp_callin to start_secondary. It told me can not inquire the apic for the CPU 12 YH Initializing CPU#1 masked ExtINT on CPU#1 Calibrating delay using timer specific routine.. 3600.30 BogoMIPS (lpj=7200601) CPU: L1 I Cache: 64K (64 bytes/line), D cache 64K (64 bytes/line) CPU: L2 Cache: 1024K (64 bytes/line) CPU 1(2) - Node 0 - Core 1 stepping 02 CPU 1: Syncing TSC to CPU 0. CPU 1: synchronized TSC with CPU 0 (last diff 0 cycles, maxerr 1415 cycles) On 7/28/05, Eric W. Biederman [EMAIL PROTECTED] wrote: yhlu [EMAIL PROTECTED] writes: I have some problem with this patch. YH On 7/28/05, yhlu [EMAIL PROTECTED] wrote: Do you mean solve the timing problem for 2 way dual core or 4 way single core above? As best as I can determine the problem is possible any time you have more than 2 cpus (from the kernels perspective), but you have ti hit a fairly narrow window in cpu start up. What problem do you have with this patch. Eric - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] x86_64: sync_tsc fix the race (so we can boot)
Can we put tsc_sync_wait() back to smp_callin? So that it will be executed serially and we can get synchronized TSC with CPU? YH - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] x86_64: sync_tsc fix the race (so we can boot)
yhlu [EMAIL PROTECTED] writes: if using you patch, the synchronized TSC with CPU never come out. then with your patch, I add back patch that moving set callin_map from smp_callin to start_secondary. It told me can not inquire the apic for the CPU 12 Hmm. You didn't post enough of a boot log for me to see the problem. Does it boot and you don't see the message or is it something else. Can we put tsc_sync_wait() back to smp_callin? So that it will be executed serially and we can get synchronized TSC with CPU? Currently that just seems silly. That code should be async safe. But it sounds like you have some weird bug I don't understand. Eric - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] x86_64: sync_tsc fix the race (so we can boot)
Eric, Latest tree works. YH Booting processor 1/4 APIC 0x1 Initializing CPU#1 masked ExtINT on CPU#1 Calibrating delay using timer specific routine.. 4000.31 BogoMIPS (lpj=8000624) CPU: L1 I Cache: 64K (64 bytes/line), D cache 64K (64 bytes/line) CPU: L2 Cache: 1024K (64 bytes/line) CPU 1(1) - Node 1 - Core 0 stepping 0a CPU 1: Syncing TSC to CPU 0. Booting processor 2/4 APIC 0x2 Initializing CPU#2 masked ExtINT on CPU#2 CPU 1: synchronized TSC with CPU 0 (last diff -4 cycles, maxerr 896 cycles) Calibrating delay using timer specific routine.. 4000.28 BogoMIPS (lpj=8000572) CPU: L1 I Cache: 64K (64 bytes/line), D cache 64K (64 bytes/line) CPU: L2 Cache: 1024K (64 bytes/line) CPU 2(1) - Node 2 - Core 0 stepping 0a CPU 2: Syncing TSC to CPU 0. Booting processor 3/4 APIC 0x3 Initializing CPU#3 masked ExtINT on CPU#3 CPU 2: synchronized TSC with CPU 0 (last diff -2 cycles, maxerr 909 cycles) Calibrating delay using timer specific routine.. 4000.15 BogoMIPS (lpj=8000317) CPU: L1 I Cache: 64K (64 bytes/line), D cache 64K (64 bytes/line) CPU: L2 Cache: 1024K (64 bytes/line) CPU 3(1) - Node 3 - Core 0 stepping 0a CPU 3: Syncing TSC to CPU 0. Brought up 4 CPUs time.c: Using PIT/TSC based timekeeping. testing NMI watchdog ... OK. checking if image is initramfs...6CPU 3: synchronized TSC with CPU 0 (last diff -16 cycles, maxerr 1496 cycles) - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] x86_64: sync_tsc fix the race (so we can boot)
yhlu <[EMAIL PROTECTED]> writes: > I have some problem with this patch. > > YH > > On 7/28/05, yhlu <[EMAIL PROTECTED]> wrote: >> Do you mean solve the timing problem for 2 way dual core or 4 way >> single core above? As best as I can determine the problem is possible any time you have more than 2 cpus (from the kernels perspective), but you have ti hit a fairly narrow window in cpu start up. What problem do you have with this patch. Eric - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] x86_64: sync_tsc fix the race (so we can boot)
I have some problem with this patch.
YH
On 7/28/05, yhlu <[EMAIL PROTECTED]> wrote:
> Do you mean solve the timing problem for 2 way dual core or 4 way
> single core above?
>
> YH
>
> On 7/27/05, Eric W. Biederman <[EMAIL PROTECTED]> wrote:
> >
> > sync_tsc was using smp_call_function to ask the boot processor
> > to report it's tsc value. smp_call_function performs an IPI_send_allbutself
> > which is a broadcast ipi. There is a window during processor startup during
> > which the target cpu has started and before it has initialized it's
> > interrupt
> > vectors so it can properly process an interrupt. Receveing an interrupt
> > during that window will triple fault the cpu and do other nasty things.
> >
> > Why cli does not protect us from that is beyond me.
> >
> > The simple fix is to match ia64 and provide a smp_call_function_single.
> > Which avoids the broadcast and is more efficient.
> >
> > This certainly fixes the problem of getting stuck on boot which was
> > very easy to trigger on my SMP Hyperthreaded Xeon, and I think
> > it fixes it for the right reasons.
> >
> > I believe this patch suffers from apicid versus logical cpu number
> > confusion.
> > I copied the basic logic from smp_send_reschedule and I can't find where
> > that translates from the logical cpuid to apicid. So it isn't quite
> > correct yet. It should be close enough that it shouldn't be too hard
> > to finish it up.
> >
> > More bug fixes after I have slept but I figured I needed to get this
> > one out for review.
> >
> > Signed-off-by: Eric W. Biederman <[EMAIL PROTECTED]>
> >
> > ---
> >
> > arch/x86_64/kernel/smp.c | 65
> > ++
> > arch/x86_64/kernel/smpboot.c | 21 --
> > include/asm-x86_64/smp.h |2 +
> > 3 files changed, 79 insertions(+), 9 deletions(-)
> >
> > 571d8bdc5a935b85fb0ce5ff3fac9a59a86efe6e
> > diff --git a/arch/x86_64/kernel/smp.c b/arch/x86_64/kernel/smp.c
> > --- a/arch/x86_64/kernel/smp.c
> > +++ b/arch/x86_64/kernel/smp.c
> > @@ -294,6 +294,71 @@ void unlock_ipi_call_lock(void)
> > }
> >
> > /*
> > + * this function sends a 'generic call function' IPI to one other CPU
> > + * in the system.
> > + */
> > +static void __smp_call_function_single (int cpu, void (*func) (void
> > *info), void *info,
> > + int nonatomic, int wait)
> > +{
> > + struct call_data_struct data;
> > + int cpus = 1;
> > +
> > + data.func = func;
> > + data.info = info;
> > + atomic_set(, 0);
> > + data.wait = wait;
> > + if (wait)
> > + atomic_set(, 0);
> > +
> > + call_data =
> > + wmb();
> > + /* Send a message to all other CPUs and wait for them to respond */
> > + send_IPI_mask(cpumask_of_cpu(cpu), CALL_FUNCTION_VECTOR);
> > +
> > + /* Wait for response */
> > + while (atomic_read() != cpus)
> > + cpu_relax();
> > +
> > + if (!wait)
> > + return;
> > +
> > + while (atomic_read() != cpus)
> > + cpu_relax();
> > +}
> > +
> > +/*
> > + * Run a function on another CPU
> > + * The function to run. This must be fast and non-blocking.
> > + * An arbitrary pointer to pass to the function.
> > + * Currently unused.
> > + * If true, wait until function has completed on other CPUs.
> > + * [RETURNS] 0 on success, else a negative status code.
> > + *
> > + * Does not return until the remote CPU is nearly ready to execute
> > + * or is or has executed.
> > + */
> > +
> > +int smp_call_function_single (int cpu, void (*func) (void *info), void
> > *info,
> > + int nonatomic, int wait)
> > +{
> > +
> > + int me = get_cpu(); /* prevent preemption and reschedule on another
> > processor */
> > +
> > + if (cpu == me) {
> > + printk("%s: trying to call self\n", __func__);
> > + put_cpu();
> > + return -EBUSY;
> > + }
> > + spin_lock_bh(_lock);
> > +
> > + __smp_call_function_single(cpu, func,info,nonatomic,wait);
> > +
> > + spin_unlock_bh(_lock);
> > + put_cpu();
> > + return 0;
> > +}
> > +
> > +/*
> > * this function sends a 'generic call function' IPI to all other CPUs
> > * in the system.
> > */
> > diff --git a/arch/x86_64/kernel/smpboot.c b/arch/x86_64/kernel/smpboot.c
> > --- a/arch/x86_64/kernel/smpboot.c
> > +++ b/arch/x86_64/kernel/smpboot.c
> > @@ -229,9 +229,6 @@ static __cpuinit void sync_master(void *
> > {
> > unsigned long flags, i;
> >
> > - if (smp_processor_id() != boot_cpu_id)
> > - return;
> > -
> > go[MASTER] = 0;
> >
> > local_irq_save(flags);
> > @@ -280,7 +277,7 @@ get_delta(long *rt, long *master)
> > return tcenter - best_tm;
> > }
> >
> > -static __cpuinit void sync_tsc(void)
> > +static __cpuinit void sync_tsc(unsigned int master)
> > {
> > int i, done = 0;
> >
Re: [PATCH] x86_64: sync_tsc fix the race (so we can boot)
Do you mean solve the timing problem for 2 way dual core or 4 way
single core above?
YH
On 7/27/05, Eric W. Biederman <[EMAIL PROTECTED]> wrote:
>
> sync_tsc was using smp_call_function to ask the boot processor
> to report it's tsc value. smp_call_function performs an IPI_send_allbutself
> which is a broadcast ipi. There is a window during processor startup during
> which the target cpu has started and before it has initialized it's interrupt
> vectors so it can properly process an interrupt. Receveing an interrupt
> during that window will triple fault the cpu and do other nasty things.
>
> Why cli does not protect us from that is beyond me.
>
> The simple fix is to match ia64 and provide a smp_call_function_single.
> Which avoids the broadcast and is more efficient.
>
> This certainly fixes the problem of getting stuck on boot which was
> very easy to trigger on my SMP Hyperthreaded Xeon, and I think
> it fixes it for the right reasons.
>
> I believe this patch suffers from apicid versus logical cpu number confusion.
> I copied the basic logic from smp_send_reschedule and I can't find where
> that translates from the logical cpuid to apicid. So it isn't quite
> correct yet. It should be close enough that it shouldn't be too hard
> to finish it up.
>
> More bug fixes after I have slept but I figured I needed to get this
> one out for review.
>
> Signed-off-by: Eric W. Biederman <[EMAIL PROTECTED]>
>
> ---
>
> arch/x86_64/kernel/smp.c | 65
> ++
> arch/x86_64/kernel/smpboot.c | 21 --
> include/asm-x86_64/smp.h |2 +
> 3 files changed, 79 insertions(+), 9 deletions(-)
>
> 571d8bdc5a935b85fb0ce5ff3fac9a59a86efe6e
> diff --git a/arch/x86_64/kernel/smp.c b/arch/x86_64/kernel/smp.c
> --- a/arch/x86_64/kernel/smp.c
> +++ b/arch/x86_64/kernel/smp.c
> @@ -294,6 +294,71 @@ void unlock_ipi_call_lock(void)
> }
>
> /*
> + * this function sends a 'generic call function' IPI to one other CPU
> + * in the system.
> + */
> +static void __smp_call_function_single (int cpu, void (*func) (void *info),
> void *info,
> + int nonatomic, int wait)
> +{
> + struct call_data_struct data;
> + int cpus = 1;
> +
> + data.func = func;
> + data.info = info;
> + atomic_set(, 0);
> + data.wait = wait;
> + if (wait)
> + atomic_set(, 0);
> +
> + call_data =
> + wmb();
> + /* Send a message to all other CPUs and wait for them to respond */
> + send_IPI_mask(cpumask_of_cpu(cpu), CALL_FUNCTION_VECTOR);
> +
> + /* Wait for response */
> + while (atomic_read() != cpus)
> + cpu_relax();
> +
> + if (!wait)
> + return;
> +
> + while (atomic_read() != cpus)
> + cpu_relax();
> +}
> +
> +/*
> + * Run a function on another CPU
> + * The function to run. This must be fast and non-blocking.
> + * An arbitrary pointer to pass to the function.
> + * Currently unused.
> + * If true, wait until function has completed on other CPUs.
> + * [RETURNS] 0 on success, else a negative status code.
> + *
> + * Does not return until the remote CPU is nearly ready to execute
> + * or is or has executed.
> + */
> +
> +int smp_call_function_single (int cpu, void (*func) (void *info), void *info,
> + int nonatomic, int wait)
> +{
> +
> + int me = get_cpu(); /* prevent preemption and reschedule on another
> processor */
> +
> + if (cpu == me) {
> + printk("%s: trying to call self\n", __func__);
> + put_cpu();
> + return -EBUSY;
> + }
> + spin_lock_bh(_lock);
> +
> + __smp_call_function_single(cpu, func,info,nonatomic,wait);
> +
> + spin_unlock_bh(_lock);
> + put_cpu();
> + return 0;
> +}
> +
> +/*
> * this function sends a 'generic call function' IPI to all other CPUs
> * in the system.
> */
> diff --git a/arch/x86_64/kernel/smpboot.c b/arch/x86_64/kernel/smpboot.c
> --- a/arch/x86_64/kernel/smpboot.c
> +++ b/arch/x86_64/kernel/smpboot.c
> @@ -229,9 +229,6 @@ static __cpuinit void sync_master(void *
> {
> unsigned long flags, i;
>
> - if (smp_processor_id() != boot_cpu_id)
> - return;
> -
> go[MASTER] = 0;
>
> local_irq_save(flags);
> @@ -280,7 +277,7 @@ get_delta(long *rt, long *master)
> return tcenter - best_tm;
> }
>
> -static __cpuinit void sync_tsc(void)
> +static __cpuinit void sync_tsc(unsigned int master)
> {
> int i, done = 0;
> long delta, adj, adjust_latency = 0;
> @@ -294,9 +291,17 @@ static __cpuinit void sync_tsc(void)
> } t[NUM_ROUNDS] __cpuinitdata;
> #endif
>
> + printk(KERN_INFO "CPU %d: Syncing TSC to CPU %u.\n",
> + smp_processor_id(), master);
> +
> go[MASTER] = 1;
>
> - smp_call_function(sync_master, NULL, 1, 0);
> + /* It is
Re: [PATCH] x86_64: sync_tsc fix the race (so we can boot)
Do you mean solve the timing problem for 2 way dual core or 4 way
single core above?
YH
On 7/27/05, Eric W. Biederman [EMAIL PROTECTED] wrote:
sync_tsc was using smp_call_function to ask the boot processor
to report it's tsc value. smp_call_function performs an IPI_send_allbutself
which is a broadcast ipi. There is a window during processor startup during
which the target cpu has started and before it has initialized it's interrupt
vectors so it can properly process an interrupt. Receveing an interrupt
during that window will triple fault the cpu and do other nasty things.
Why cli does not protect us from that is beyond me.
The simple fix is to match ia64 and provide a smp_call_function_single.
Which avoids the broadcast and is more efficient.
This certainly fixes the problem of getting stuck on boot which was
very easy to trigger on my SMP Hyperthreaded Xeon, and I think
it fixes it for the right reasons.
I believe this patch suffers from apicid versus logical cpu number confusion.
I copied the basic logic from smp_send_reschedule and I can't find where
that translates from the logical cpuid to apicid. So it isn't quite
correct yet. It should be close enough that it shouldn't be too hard
to finish it up.
More bug fixes after I have slept but I figured I needed to get this
one out for review.
Signed-off-by: Eric W. Biederman [EMAIL PROTECTED]
---
arch/x86_64/kernel/smp.c | 65
++
arch/x86_64/kernel/smpboot.c | 21 --
include/asm-x86_64/smp.h |2 +
3 files changed, 79 insertions(+), 9 deletions(-)
571d8bdc5a935b85fb0ce5ff3fac9a59a86efe6e
diff --git a/arch/x86_64/kernel/smp.c b/arch/x86_64/kernel/smp.c
--- a/arch/x86_64/kernel/smp.c
+++ b/arch/x86_64/kernel/smp.c
@@ -294,6 +294,71 @@ void unlock_ipi_call_lock(void)
}
/*
+ * this function sends a 'generic call function' IPI to one other CPU
+ * in the system.
+ */
+static void __smp_call_function_single (int cpu, void (*func) (void *info),
void *info,
+ int nonatomic, int wait)
+{
+ struct call_data_struct data;
+ int cpus = 1;
+
+ data.func = func;
+ data.info = info;
+ atomic_set(data.started, 0);
+ data.wait = wait;
+ if (wait)
+ atomic_set(data.finished, 0);
+
+ call_data = data;
+ wmb();
+ /* Send a message to all other CPUs and wait for them to respond */
+ send_IPI_mask(cpumask_of_cpu(cpu), CALL_FUNCTION_VECTOR);
+
+ /* Wait for response */
+ while (atomic_read(data.started) != cpus)
+ cpu_relax();
+
+ if (!wait)
+ return;
+
+ while (atomic_read(data.finished) != cpus)
+ cpu_relax();
+}
+
+/*
+ * Run a function on another CPU
+ * func The function to run. This must be fast and non-blocking.
+ * info An arbitrary pointer to pass to the function.
+ * nonatomicCurrently unused.
+ * wait If true, wait until function has completed on other CPUs.
+ * [RETURNS] 0 on success, else a negative status code.
+ *
+ * Does not return until the remote CPU is nearly ready to execute func
+ * or is or has executed.
+ */
+
+int smp_call_function_single (int cpu, void (*func) (void *info), void *info,
+ int nonatomic, int wait)
+{
+
+ int me = get_cpu(); /* prevent preemption and reschedule on another
processor */
+
+ if (cpu == me) {
+ printk(%s: trying to call self\n, __func__);
+ put_cpu();
+ return -EBUSY;
+ }
+ spin_lock_bh(call_lock);
+
+ __smp_call_function_single(cpu, func,info,nonatomic,wait);
+
+ spin_unlock_bh(call_lock);
+ put_cpu();
+ return 0;
+}
+
+/*
* this function sends a 'generic call function' IPI to all other CPUs
* in the system.
*/
diff --git a/arch/x86_64/kernel/smpboot.c b/arch/x86_64/kernel/smpboot.c
--- a/arch/x86_64/kernel/smpboot.c
+++ b/arch/x86_64/kernel/smpboot.c
@@ -229,9 +229,6 @@ static __cpuinit void sync_master(void *
{
unsigned long flags, i;
- if (smp_processor_id() != boot_cpu_id)
- return;
-
go[MASTER] = 0;
local_irq_save(flags);
@@ -280,7 +277,7 @@ get_delta(long *rt, long *master)
return tcenter - best_tm;
}
-static __cpuinit void sync_tsc(void)
+static __cpuinit void sync_tsc(unsigned int master)
{
int i, done = 0;
long delta, adj, adjust_latency = 0;
@@ -294,9 +291,17 @@ static __cpuinit void sync_tsc(void)
} t[NUM_ROUNDS] __cpuinitdata;
#endif
+ printk(KERN_INFO CPU %d: Syncing TSC to CPU %u.\n,
+ smp_processor_id(), master);
+
go[MASTER] = 1;
- smp_call_function(sync_master, NULL, 1, 0);
+ /* It is dangerous to broadcast IPI as cpus are coming up,
+* as they
Re: [PATCH] x86_64: sync_tsc fix the race (so we can boot)
I have some problem with this patch.
YH
On 7/28/05, yhlu [EMAIL PROTECTED] wrote:
Do you mean solve the timing problem for 2 way dual core or 4 way
single core above?
YH
On 7/27/05, Eric W. Biederman [EMAIL PROTECTED] wrote:
sync_tsc was using smp_call_function to ask the boot processor
to report it's tsc value. smp_call_function performs an IPI_send_allbutself
which is a broadcast ipi. There is a window during processor startup during
which the target cpu has started and before it has initialized it's
interrupt
vectors so it can properly process an interrupt. Receveing an interrupt
during that window will triple fault the cpu and do other nasty things.
Why cli does not protect us from that is beyond me.
The simple fix is to match ia64 and provide a smp_call_function_single.
Which avoids the broadcast and is more efficient.
This certainly fixes the problem of getting stuck on boot which was
very easy to trigger on my SMP Hyperthreaded Xeon, and I think
it fixes it for the right reasons.
I believe this patch suffers from apicid versus logical cpu number
confusion.
I copied the basic logic from smp_send_reschedule and I can't find where
that translates from the logical cpuid to apicid. So it isn't quite
correct yet. It should be close enough that it shouldn't be too hard
to finish it up.
More bug fixes after I have slept but I figured I needed to get this
one out for review.
Signed-off-by: Eric W. Biederman [EMAIL PROTECTED]
---
arch/x86_64/kernel/smp.c | 65
++
arch/x86_64/kernel/smpboot.c | 21 --
include/asm-x86_64/smp.h |2 +
3 files changed, 79 insertions(+), 9 deletions(-)
571d8bdc5a935b85fb0ce5ff3fac9a59a86efe6e
diff --git a/arch/x86_64/kernel/smp.c b/arch/x86_64/kernel/smp.c
--- a/arch/x86_64/kernel/smp.c
+++ b/arch/x86_64/kernel/smp.c
@@ -294,6 +294,71 @@ void unlock_ipi_call_lock(void)
}
/*
+ * this function sends a 'generic call function' IPI to one other CPU
+ * in the system.
+ */
+static void __smp_call_function_single (int cpu, void (*func) (void
*info), void *info,
+ int nonatomic, int wait)
+{
+ struct call_data_struct data;
+ int cpus = 1;
+
+ data.func = func;
+ data.info = info;
+ atomic_set(data.started, 0);
+ data.wait = wait;
+ if (wait)
+ atomic_set(data.finished, 0);
+
+ call_data = data;
+ wmb();
+ /* Send a message to all other CPUs and wait for them to respond */
+ send_IPI_mask(cpumask_of_cpu(cpu), CALL_FUNCTION_VECTOR);
+
+ /* Wait for response */
+ while (atomic_read(data.started) != cpus)
+ cpu_relax();
+
+ if (!wait)
+ return;
+
+ while (atomic_read(data.finished) != cpus)
+ cpu_relax();
+}
+
+/*
+ * Run a function on another CPU
+ * func The function to run. This must be fast and non-blocking.
+ * info An arbitrary pointer to pass to the function.
+ * nonatomicCurrently unused.
+ * wait If true, wait until function has completed on other CPUs.
+ * [RETURNS] 0 on success, else a negative status code.
+ *
+ * Does not return until the remote CPU is nearly ready to execute func
+ * or is or has executed.
+ */
+
+int smp_call_function_single (int cpu, void (*func) (void *info), void
*info,
+ int nonatomic, int wait)
+{
+
+ int me = get_cpu(); /* prevent preemption and reschedule on another
processor */
+
+ if (cpu == me) {
+ printk(%s: trying to call self\n, __func__);
+ put_cpu();
+ return -EBUSY;
+ }
+ spin_lock_bh(call_lock);
+
+ __smp_call_function_single(cpu, func,info,nonatomic,wait);
+
+ spin_unlock_bh(call_lock);
+ put_cpu();
+ return 0;
+}
+
+/*
* this function sends a 'generic call function' IPI to all other CPUs
* in the system.
*/
diff --git a/arch/x86_64/kernel/smpboot.c b/arch/x86_64/kernel/smpboot.c
--- a/arch/x86_64/kernel/smpboot.c
+++ b/arch/x86_64/kernel/smpboot.c
@@ -229,9 +229,6 @@ static __cpuinit void sync_master(void *
{
unsigned long flags, i;
- if (smp_processor_id() != boot_cpu_id)
- return;
-
go[MASTER] = 0;
local_irq_save(flags);
@@ -280,7 +277,7 @@ get_delta(long *rt, long *master)
return tcenter - best_tm;
}
-static __cpuinit void sync_tsc(void)
+static __cpuinit void sync_tsc(unsigned int master)
{
int i, done = 0;
long delta, adj, adjust_latency = 0;
@@ -294,9 +291,17 @@ static __cpuinit void sync_tsc(void)
} t[NUM_ROUNDS] __cpuinitdata;
#endif
+ printk(KERN_INFO CPU %d: Syncing TSC to CPU
Re: [PATCH] x86_64: sync_tsc fix the race (so we can boot)
yhlu [EMAIL PROTECTED] writes: I have some problem with this patch. YH On 7/28/05, yhlu [EMAIL PROTECTED] wrote: Do you mean solve the timing problem for 2 way dual core or 4 way single core above? As best as I can determine the problem is possible any time you have more than 2 cpus (from the kernels perspective), but you have ti hit a fairly narrow window in cpu start up. What problem do you have with this patch. Eric - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] x86_64: sync_tsc fix the race (so we can boot)
sync_tsc was using smp_call_function to ask the boot processor
to report it's tsc value. smp_call_function performs an IPI_send_allbutself
which is a broadcast ipi. There is a window during processor startup during
which the target cpu has started and before it has initialized it's interrupt
vectors so it can properly process an interrupt. Receveing an interrupt
during that window will triple fault the cpu and do other nasty things.
Why cli does not protect us from that is beyond me.
The simple fix is to match ia64 and provide a smp_call_function_single.
Which avoids the broadcast and is more efficient.
This certainly fixes the problem of getting stuck on boot which was
very easy to trigger on my SMP Hyperthreaded Xeon, and I think
it fixes it for the right reasons.
I believe this patch suffers from apicid versus logical cpu number confusion.
I copied the basic logic from smp_send_reschedule and I can't find where
that translates from the logical cpuid to apicid. So it isn't quite
correct yet. It should be close enough that it shouldn't be too hard
to finish it up.
More bug fixes after I have slept but I figured I needed to get this
one out for review.
Signed-off-by: Eric W. Biederman <[EMAIL PROTECTED]>
---
arch/x86_64/kernel/smp.c | 65 ++
arch/x86_64/kernel/smpboot.c | 21 --
include/asm-x86_64/smp.h |2 +
3 files changed, 79 insertions(+), 9 deletions(-)
571d8bdc5a935b85fb0ce5ff3fac9a59a86efe6e
diff --git a/arch/x86_64/kernel/smp.c b/arch/x86_64/kernel/smp.c
--- a/arch/x86_64/kernel/smp.c
+++ b/arch/x86_64/kernel/smp.c
@@ -294,6 +294,71 @@ void unlock_ipi_call_lock(void)
}
/*
+ * this function sends a 'generic call function' IPI to one other CPU
+ * in the system.
+ */
+static void __smp_call_function_single (int cpu, void (*func) (void *info),
void *info,
+ int nonatomic, int wait)
+{
+ struct call_data_struct data;
+ int cpus = 1;
+
+ data.func = func;
+ data.info = info;
+ atomic_set(, 0);
+ data.wait = wait;
+ if (wait)
+ atomic_set(, 0);
+
+ call_data =
+ wmb();
+ /* Send a message to all other CPUs and wait for them to respond */
+ send_IPI_mask(cpumask_of_cpu(cpu), CALL_FUNCTION_VECTOR);
+
+ /* Wait for response */
+ while (atomic_read() != cpus)
+ cpu_relax();
+
+ if (!wait)
+ return;
+
+ while (atomic_read() != cpus)
+ cpu_relax();
+}
+
+/*
+ * Run a function on another CPU
+ * The function to run. This must be fast and non-blocking.
+ * An arbitrary pointer to pass to the function.
+ * Currently unused.
+ * If true, wait until function has completed on other CPUs.
+ * [RETURNS] 0 on success, else a negative status code.
+ *
+ * Does not return until the remote CPU is nearly ready to execute
+ * or is or has executed.
+ */
+
+int smp_call_function_single (int cpu, void (*func) (void *info), void *info,
+ int nonatomic, int wait)
+{
+
+ int me = get_cpu(); /* prevent preemption and reschedule on another
processor */
+
+ if (cpu == me) {
+ printk("%s: trying to call self\n", __func__);
+ put_cpu();
+ return -EBUSY;
+ }
+ spin_lock_bh(_lock);
+
+ __smp_call_function_single(cpu, func,info,nonatomic,wait);
+
+ spin_unlock_bh(_lock);
+ put_cpu();
+ return 0;
+}
+
+/*
* this function sends a 'generic call function' IPI to all other CPUs
* in the system.
*/
diff --git a/arch/x86_64/kernel/smpboot.c b/arch/x86_64/kernel/smpboot.c
--- a/arch/x86_64/kernel/smpboot.c
+++ b/arch/x86_64/kernel/smpboot.c
@@ -229,9 +229,6 @@ static __cpuinit void sync_master(void *
{
unsigned long flags, i;
- if (smp_processor_id() != boot_cpu_id)
- return;
-
go[MASTER] = 0;
local_irq_save(flags);
@@ -280,7 +277,7 @@ get_delta(long *rt, long *master)
return tcenter - best_tm;
}
-static __cpuinit void sync_tsc(void)
+static __cpuinit void sync_tsc(unsigned int master)
{
int i, done = 0;
long delta, adj, adjust_latency = 0;
@@ -294,9 +291,17 @@ static __cpuinit void sync_tsc(void)
} t[NUM_ROUNDS] __cpuinitdata;
#endif
+ printk(KERN_INFO "CPU %d: Syncing TSC to CPU %u.\n",
+ smp_processor_id(), master);
+
go[MASTER] = 1;
- smp_call_function(sync_master, NULL, 1, 0);
+ /* It is dangerous to broadcast IPI as cpus are coming up,
+* as they may not be ready to accept them. So since
+* we only need to send the ipi to the boot cpu direct
+* the message, and avoid the race.
+*/
+ smp_call_function_single(master, sync_master, NULL, 1, 0);
while (go[MASTER]) /* wait for master to be ready */
no_cpu_relax();
@@ -340,16 +345,14 @@
[PATCH] x86_64: sync_tsc fix the race (so we can boot)
sync_tsc was using smp_call_function to ask the boot processor
to report it's tsc value. smp_call_function performs an IPI_send_allbutself
which is a broadcast ipi. There is a window during processor startup during
which the target cpu has started and before it has initialized it's interrupt
vectors so it can properly process an interrupt. Receveing an interrupt
during that window will triple fault the cpu and do other nasty things.
Why cli does not protect us from that is beyond me.
The simple fix is to match ia64 and provide a smp_call_function_single.
Which avoids the broadcast and is more efficient.
This certainly fixes the problem of getting stuck on boot which was
very easy to trigger on my SMP Hyperthreaded Xeon, and I think
it fixes it for the right reasons.
I believe this patch suffers from apicid versus logical cpu number confusion.
I copied the basic logic from smp_send_reschedule and I can't find where
that translates from the logical cpuid to apicid. So it isn't quite
correct yet. It should be close enough that it shouldn't be too hard
to finish it up.
More bug fixes after I have slept but I figured I needed to get this
one out for review.
Signed-off-by: Eric W. Biederman [EMAIL PROTECTED]
---
arch/x86_64/kernel/smp.c | 65 ++
arch/x86_64/kernel/smpboot.c | 21 --
include/asm-x86_64/smp.h |2 +
3 files changed, 79 insertions(+), 9 deletions(-)
571d8bdc5a935b85fb0ce5ff3fac9a59a86efe6e
diff --git a/arch/x86_64/kernel/smp.c b/arch/x86_64/kernel/smp.c
--- a/arch/x86_64/kernel/smp.c
+++ b/arch/x86_64/kernel/smp.c
@@ -294,6 +294,71 @@ void unlock_ipi_call_lock(void)
}
/*
+ * this function sends a 'generic call function' IPI to one other CPU
+ * in the system.
+ */
+static void __smp_call_function_single (int cpu, void (*func) (void *info),
void *info,
+ int nonatomic, int wait)
+{
+ struct call_data_struct data;
+ int cpus = 1;
+
+ data.func = func;
+ data.info = info;
+ atomic_set(data.started, 0);
+ data.wait = wait;
+ if (wait)
+ atomic_set(data.finished, 0);
+
+ call_data = data;
+ wmb();
+ /* Send a message to all other CPUs and wait for them to respond */
+ send_IPI_mask(cpumask_of_cpu(cpu), CALL_FUNCTION_VECTOR);
+
+ /* Wait for response */
+ while (atomic_read(data.started) != cpus)
+ cpu_relax();
+
+ if (!wait)
+ return;
+
+ while (atomic_read(data.finished) != cpus)
+ cpu_relax();
+}
+
+/*
+ * Run a function on another CPU
+ * func The function to run. This must be fast and non-blocking.
+ * info An arbitrary pointer to pass to the function.
+ * nonatomicCurrently unused.
+ * wait If true, wait until function has completed on other CPUs.
+ * [RETURNS] 0 on success, else a negative status code.
+ *
+ * Does not return until the remote CPU is nearly ready to execute func
+ * or is or has executed.
+ */
+
+int smp_call_function_single (int cpu, void (*func) (void *info), void *info,
+ int nonatomic, int wait)
+{
+
+ int me = get_cpu(); /* prevent preemption and reschedule on another
processor */
+
+ if (cpu == me) {
+ printk(%s: trying to call self\n, __func__);
+ put_cpu();
+ return -EBUSY;
+ }
+ spin_lock_bh(call_lock);
+
+ __smp_call_function_single(cpu, func,info,nonatomic,wait);
+
+ spin_unlock_bh(call_lock);
+ put_cpu();
+ return 0;
+}
+
+/*
* this function sends a 'generic call function' IPI to all other CPUs
* in the system.
*/
diff --git a/arch/x86_64/kernel/smpboot.c b/arch/x86_64/kernel/smpboot.c
--- a/arch/x86_64/kernel/smpboot.c
+++ b/arch/x86_64/kernel/smpboot.c
@@ -229,9 +229,6 @@ static __cpuinit void sync_master(void *
{
unsigned long flags, i;
- if (smp_processor_id() != boot_cpu_id)
- return;
-
go[MASTER] = 0;
local_irq_save(flags);
@@ -280,7 +277,7 @@ get_delta(long *rt, long *master)
return tcenter - best_tm;
}
-static __cpuinit void sync_tsc(void)
+static __cpuinit void sync_tsc(unsigned int master)
{
int i, done = 0;
long delta, adj, adjust_latency = 0;
@@ -294,9 +291,17 @@ static __cpuinit void sync_tsc(void)
} t[NUM_ROUNDS] __cpuinitdata;
#endif
+ printk(KERN_INFO CPU %d: Syncing TSC to CPU %u.\n,
+ smp_processor_id(), master);
+
go[MASTER] = 1;
- smp_call_function(sync_master, NULL, 1, 0);
+ /* It is dangerous to broadcast IPI as cpus are coming up,
+* as they may not be ready to accept them. So since
+* we only need to send the ipi to the boot cpu direct
+* the message, and avoid the race.
+*/
+ smp_call_function_single(master, sync_master, NULL, 1, 0);
while (go[MASTER]) /* wait for
