Re: [PATCH] allow drivers to claim the lapic NMI watchdog HW

[John Levon]

On Tue, May 04, 2004 at 04:33:01AM +0200, Mikael Pettersson wrote:

> +/* lapic_nmi_owner:
> + * +1: the lapic NMI hardware is assigned to the lapic NMI watchdog
> + *  0: the lapic NMI hardware is unassigned

If we're going to have a mini state machine, can't we at least use some
defines for each state...

> +		lapic_nmi_owner -= 2; /* +1 -> -1, 0 -> -2 */

...and make this into some readable english via a little helper?

> -EXPORT_SYMBOL(disable_lapic_nmi_watchdog);
> -EXPORT_SYMBOL(enable_lapic_nmi_watchdog);
> +EXPORT_SYMBOL(reassign_lapic_nmi_watchdog);
> +EXPORT_SYMBOL(release_lapic_nmi_watchdog);

I don't like this new naming. Since the patch is really all about
ownership of the local APIC, can't we call it something like

acquire_lapic_nmi()
release_lapic_nmi()

Neither perfctr nor oprofile have anything to do with watchdogs, so
this:

> -	disable_lapic_nmi_watchdog();
> +	if (reassign_lapic_nmi_watchdog() < 0) {

Looks a little weird now.

regards
john
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/