linux-kernel - Re: [RFC V2 1/2] irq: Add a framework to measure interrupt timings

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <alpine.DEB.2.11.1601201847050.3575@nanos>
Date:	Wed, 20 Jan 2016 18:55:48 +0100 (CET)
From:	Thomas Gleixner <tglx@...utronix.de>
To:	Daniel Lezcano <daniel.lezcano@...aro.org>
cc:	peterz@...radead.org, rafael@...nel.org, linux-pm@...r.kernel.org,
	linux-kernel@...r.kernel.org, nicolas.pitre@...aro.org,
	vincent.guittot@...aro.org
Subject: Re: [RFC V2 1/2] irq: Add a framework to measure interrupt timings

On Wed, 20 Jan 2016, Daniel Lezcano wrote:
> +#ifdef CONFIG_IRQ_TIMINGS
> +/**
> + * struct irqt_ops - structure to be used by the subsystem to track
> + *                   irq timings
> + * @alloc:    called when an irqdesc is allocated
> + * @free:     called when an irqdesc is free
> + * @setup:    called when an irq is setup, this is called under lock
> + * @remove:   called when an irq is removed
> + * @handler:  called when an interrupt is handled
> + */
> +struct irqtimings_ops {
> +	int (*alloc)(unsigned int);
> +	void (*free)(unsigned int);
> +	int (*setup)(unsigned int, struct irqaction *act);
> +	void (*remove)(unsigned int, void *dev_id);
> +	irqt_handler_t handler;
> +};
> +
> +/**
> + * This macro *must* be used by the subsystem interested by the irq
> + * timing information.
> + */
> +#define DECLARE_IRQ_TIMINGS(__ops)				\
> +	const struct irqtimings_ops *__irqtimings = __ops;
> +#endif

> @@ -20,6 +20,49 @@ extern bool noirqdebug;
>  
>  extern struct irqaction chained_action;
>  
> +#ifdef CONFIG_IRQ_TIMINGS
> +
> +extern const struct irqtimings_ops *__irqtimings;
> +
> +static inline int alloc_irqtiming(unsigned int irq)
> +{
> +	if (__irqtimings->alloc)
> +		return __irqtimings->alloc(irq);

I really have a hard time to understand that indirection. __irqtimings is
statically allocated and compiled in. There can be only one user for this in
the system ever and that user has all callbacks populated.

Why can't you spare all that pointer muck and simply have:

#ifdef CONFIG_IRQ_TIMINGS
int irqtiming_alloc(usigned int irq);
....
#else
static int irqtiming_alloc(usigned int irq) { return 0; }
...
#endif

and implement those functions in your idle thingy?

Thanks,

	tglx