On 01/13/2013 01:28 PM, Jan Kiszka wrote:
> On 2013-01-12 18:59, Gilles Chanteperdrix wrote:
>> On 01/12/2013 06:45 PM, Jan Kiszka wrote:
>>
>>> On 2013-01-12 18:35, Gilles Chanteperdrix wrote:
>>>> On 01/11/2013 08:45 AM, Jan Kiszka wrote:
>>>>
>>>>> On 2013-01-11 08:37, Jan Kiszka wrote:
>>>>>>> From acc3c57390d275a1b28d66f1ec88c85e0f8c0890 Mon Sep 17 00:00:00 2001
>>>>>>> From: Gilles Chanteperdrix <[email protected]>
>>>>>>> Date: Sat, 29 Dec 2012 17:48:20 +0100
>>>>>>> Subject: [PATCH] x86/ipipe: restore warning with AMD erratum
>>>>>>>
>>>>>>> ---
>>>>>>> arch/x86/kernel/apic/apic.c | 16 ++++++++++------
>>>>>>> 1 files changed, 10 insertions(+), 6 deletions(-)
>>>>>>>
>>>>>>> diff --git a/arch/x86/kernel/apic/apic.c b/arch/x86/kernel/apic/apic.c
>>>>>>> index 7f07610..91531bb 100644
>>>>>>> --- a/arch/x86/kernel/apic/apic.c
>>>>>>> +++ b/arch/x86/kernel/apic/apic.c
>>>>>>> @@ -545,14 +545,18 @@ static void __cpuinit setup_APIC_timer(void)
>>>>>>> memcpy(levt, &lapic_clockevent, sizeof(*levt));
>>>>>>> levt->cpumask = cpumask_of(smp_processor_id());
>>>>>>> #ifdef CONFIG_IPIPE
>>>>>>> - if (!(lapic_clockevent.features & CLOCK_EVT_FEAT_DUMMY))
>>>>>>> + if (!(lapic_clockevent.features & CLOCK_EVT_FEAT_DUMMY)
>>>>>>> + && !cpu_has_amd_erratum(amd_erratum_400))
>>>>>>> levt->ipipe_timer = &__get_cpu_var(lapic_itimer);
>>>>>>> else {
>>>>>>> - printk(KERN_INFO
>>>>>>> - "I-pipe: cannot use LAPIC as a tick device\n");
>>>>>>> - if (cpu_has_amd_erratum(amd_erratum_400))
>>>>>>> - printk(KERN_INFO
>>>>>>> - "I-pipe: disable C1E power state in your
>>>>>>> BIOS\n");
>>>>>>> + static atomic_t printed = ATOMIC_INIT(-1);
>>>>>>> + printk(KERN_INFO
>>>>>>> + "I-pipe: cannot use LAPIC on cpu #%d as a tick
>>>>>>> device\n",
>>>>>>> + smp_processor_id());
>>>>>>> + if (cpu_has_amd_erratum(amd_erratum_400)
>>>>>>> + && atomic_inc_and_test(&printed))
>>>>>>> + printk(KERN_INFO
>>>>>>> + "I-pipe: disable C1E power state in your
>>>>>>> BIOS\n");
>>>>>>
>>>>>> printk_once should do the trick as well.
>>>>>
>>>>> In fact, it should actually do what you likely intended: print on first
>>>>> occurrence, not on second or later. ;)
>>>>
>>>>
>>>> Not quite, printk_once is not protected from the code running
>>>> concurrently on multiple cpus.
>>>
>>> OK.
>>>
>>>> So, atomic_inc_and_test is better, except
>>>> the ATOMIC_INIT needs to be set to 0.
>>>
>>> Nope, -1 is correct. atomic_inc_and_test first increments, then tests. I
>>> missed that it checks against 0, not != 0. So the logic is fine, you
>>> should just include the first printk as well.
>>
>>
>> Yes, I have just realized that looking at the x86 version of
>> atomic_inc_and_test, only the asm-generic version is broken, but I guess
>> nobody uses it:
>>
>> static inline int atomic_add_return(int i, atomic_t *v)
>> {
>> unsigned long flags;
>> int temp;
>>
>> flags = hard_local_irq_save(); /* Don't trace it in an irqsoff handler
>> */
>> temp = v->counter;
>> temp += i;
>> v->counter = temp;
>> hard_local_irq_restore(flags);
>>
>> return temp;
>> }
>>
>> #define atomic_inc_and_test(v) (atomic_inc_return(v) == 0)
>>
>
> Are you referring to lacking SMP support of this version? I guess that's
> intentional (no SMP arch should lack atomic ops).
>
> It is otherwise functionally identical to the x86 code, i.e. it *first*
> increments and *then* tests. Your new version of this patch will
> therefore never print anything due to the 0 initialization - well, until
> the counter overflows. Probably it's better to use test_and_set_bit here
> to avoid all these confusions.
Sorry, fixed, thanks.
--
Gilles.
_______________________________________________
Xenomai mailing list
[email protected]
http://www.xenomai.org/mailman/listinfo/xenomai
