Re: [PATCH] expand /proc/interrupts to include missing vectors, v3

[Joe Korty <joe.korty@ccur.com>]

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@ccur.com> 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-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/