Re: i386 nmi_watchdog: Merge check_nmi_watchdog fixes from x86_64

[Eric W. Biederman]

Andrew Morton <akpm@xxxxxxxx> writes:

> ebiederm@xxxxxxxxxxxx (Eric W. Biederman) wrote:
>>
>> 
>> The per cpu nmi watchdog timer is based on an event counter.  
>> idle cpus don't generate events so the NMI watchdog doesn't fire
>> and the test to see if the watchdog is working fails.
>> 
>> - Add nmi_cpu_busy so idle cpus don't mess up the test.
>> - kmalloc prev_nmi_count to keep kernel stack usage bounded.
>> - Improve the error message on failure so there is enough
>>   information to debug problems.
>> 
>> ...
>>
>>  static int __init check_nmi_watchdog(void)
>>  {
>> -	unsigned int prev_nmi_count[NR_CPUS];
>> +	volatile int endflag = 0;
>
> I don't think this needs to be declared volatile?


Sorry for not replying sooner I just got back from a trip.

I haven't though it through extremely closely but I believe
the stores into that variable in check_nmi_watchdog could 
legitimately be optimized away by the compiler if it doesn't
have a hint.  As the variable is auto and is never used
after the store without volatile it seems a reasonable
assumption that no one else will see the value.

If the variable was static the volatile would clearly be unnecessary
as we have taken the address earlier so at some point the compiler
would be obligated to but with the variable being auto the rules are a
little murky.

Eric
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/