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 [off-list ref] writes:

quoted

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.

quoted

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.

quoted

 	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).

quoted

+		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.