RE: sysctl question

[Paul Goyette <paul%whooppee.com@localhost>]

On Sat, 21 Jun 2008, Tim Rightnour wrote:

> > 6. The current code seems to assume that zero is an invalid value 
for
> >     the user_crit_{min,max} setting.  Is this assumption valid?  While
> >     I can't imagine anyone actually trying to run their system in sub-
> >     freezing temperatures, the sensors I'm playing with are capable of
> >     registering temperatures from -256 to +255 degC!
>
> Yeah, thats because temperature is internally in microkelvin, it will 
> never fall to zero on a working sensor.

Yeah, I realized that later on.  :)  Having a negative value for degK 
would be a real impossibility.

> > 7. I've noticed that the values for PENVSYS_EVENT_* defined in
> >     sys/power.h have values in increments of 10.  I don't seem to find
> >     any rationale for this.  Would it be acceptable to insert new values
> >     in between existing ones, or should the whole table be renumbered?
> >     (Inserting might make it easier if sorting the work queue is the
> >     right answer to #5).
>
> I suspect it was done this way to allow you to insert things between them as
> you intend to.  You might dig through the CVS logs to see if any mention of
> that was made.

OK, I'll go dig through the logs.

> > 8. If I modify the envsys framework to add the user_warn_{min,max}
> >     stuff, would it even make sense for the sensor's driver to set the
> >     hardware-based thresholds?  It seems to me that if I do that, I'll
> >     end up sending two events for every threshold crossing.  For example
> >     I'd send both a CRITOVER and USER_CRITOVER event if the sensor gets
> >     above the crit_max setting.
>
> That seems broken to me.  I would imagine correct behavior would be 
> that USER overrides the system set/default one.  I do think you should 
> set one by default in the driver, in case the user doesn't bother to 
> wire stuff up properly though.

The current behavior is definitely broken in this respect.  Not only are 
the USER* events totally separate from the system ones, but they are all 
totally independant of each other.  So even with the current set-up it's 
possible to get multiple events from a single sensor refresh.  Consider 
what happens on a rapidly-moving sensor value:

a. At time 0, the sensor is "normal", between crit-min and crit-max.
b. At time 1, the sensor rises above crit-max.  The crit-max event is
   detected and reported.
c. At time 2, the sensor drops rapidly, below crit-min.  Now we get
   two events triggering!  The crit-max event reports that the sensor
   has returned to normal and at the same time the crit-min is reported
   too!

Somehow it seems to me that there should be a _single_ event monitor for 
each sensor.  This single monitor needs to be aware of _all_ the range
boundaries, and manage a single reporting state to avoid the above 
situation.  (The above situation gets aggravated with more boundaries, 
and it's possible for a single transition from above-crit-max to 
below-crit-min to generate three "back to normal" events in addition to 
the correct "below critical" event.)

I haven't solved this multiple-report problem, but I _have_ successfully 
implemented the addition of USER_WARN{MIN,MAX} events.  (I also added a 
new BATT_USERWARN event, just for consistency!)  Including man page 
updates (!), I touched a total of 10 source files.  What would be the 
proper way for me to submit these changes?  Should I file a change- 
request PR?  Or just post the changes here on tech-kern?

And should I submit a separate PR regarding the multiple event report 
problem?



----------------------------------------------------------------------
|   Paul Goyette   | PGP DSS Key fingerprint: |  E-mail addresses:   |
| Customer Service | FA29 0E3B 35AF E8AE 6651 |  paul%whooppee.com@localhost   |
| Network Engineer | 0786 F758 55DE 53BA 7731 | pgoyette%juniper.net@localhost |
----------------------------------------------------------------------