lkml.org 
[lkml]   [2004]   [May]   [4]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH] allow drivers to claim the lapic NMI watchdog HW
John Levon writes:
> 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?

Thing is, using discrete states makes the code for the checks
and state changes more verbose. However, I can easily hide the
representation behind macros with understandable names.

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

Yep, those are nicer names.

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

A little, yes.

I'll implement these changes this evening and post an updated patch tomorrow.

/Mikael
-
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/

\
 
 \ /
  Last update: 2005-03-22 14:02    [W:0.141 / U:0.084 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site