Re: [PATCH V8 2/3] irq: Track the interrupt timings

[Nicolas Pitre]

On Thu, 23 Mar 2017, Thomas Gleixner wrote:

> On Thu, 23 Mar 2017, Nicolas Pitre wrote:
> 
> > On Thu, 23 Mar 2017, Daniel Lezcano wrote:
> > 
> > > +#define IRQ_TIMINGS_SHIFT	5
> > > +#define IRQ_TIMINGS_SIZE	(1 << IRQ_TIMINGS_SHIFT)
> > > +#define IRQ_TIMINGS_MASK	(IRQ_TIMINGS_SIZE - 1)
> > > +
> > > +struct irq_timing {
> > > +	u32 irq;
> > > +	u64 ts;
> > > +};
> > > +
> > > +struct irq_timings {
> > > +	struct irq_timing values[IRQ_TIMINGS_SIZE]; /* our circular buffer */
> > 
> > This is not very space efficient because of alignment restrictions from 
> > the u64 in struct irq_timing. 25% of the memory is wasted.
> > 
> > You could consider having two arrays instead:
> > 
> > 	u32 irq_values[IRQ_TIMINGS_SIZE];
> > 	u64 ts_values[IRQ_TIMINGS_SIZE];
> 
> For the penalty of dirtying two cachelines instead of one.

Well...

Is there a need for 64 bits of relative time stamps?
And 32 bits of IRQ number?

I'd say that 48 bit time stamp and 16 bit IRQ number is way sufficient.
Who cares if we mispredict an IRQ after 78 hours of idle time?

Hence:

	u64 value = (ts << 16) | irq;


Nicolas