On Tue, Jul 31, 2007 at 07:02:01PM +0200, Andi Kleen wrote: Hi Andi, Thanks for the review. I implemented many of your suggestions and for the rest, here mention why not, in case you want to respond further. Regards, Joe > Joe Korty <joe.korty@xxxxxxxx> writes: > > A threshold interrupt occurs when ECC memory correction > > is occuring at too high a frequency. > > It's configurable and the default is off. Also > it's only on AMD hardware. v4 now has a comment to the Documentation section noting this. > > Thresholds are used > > by the ECC hardware as occasional ECC failures are part > > of normal operation, Occasional ECC _corrections_ are normal (due to stray alpha particles) but ECC _failures_ are not. Document corrected. > > irq_exit(); > > + __get_cpu_var(irq_stat).irq_spur_counts++; > > Wouldn't it be safer on preemptible kernels to have that inside > the irq_exit? Although irq_exit() releases the preemption block, it doesn't seem to release the APIC interrupt block, at least for i386. And as an interrupt block also blocks preemption and process migration, it seems that it would be safe to do the increments after the irq_exit(). But I've moved them all inside in v4, just in case I am wrong, or this changes in the future (eg, PREEMPT_RT). > > + seq_printf(p, "RES: "); > > I think it would be better to use 5-6 char identifiers > even when it whacks the columns a bit; otherwise nobody > will know what it means. e.g. SCHED here. v3 addresses this. The normally empty 'description' column at the end of each line now holds a description of each vector. The three-character line-prefix names are there only to make the new lines match the syntax and format of the other lines in /proc/interrupts. > Also there you should update proc(5) and send a patch > to the manpage maintainer. Will do. Thanks, Joe PS: also fixed up the whitespace. - To unsubscribe from this list: send the line "unsubscribe linux-rt-users" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
