tech-kern archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

RE: sysctl question

On Sat, 21 Jun 2008, Tim Rightnour wrote:

6. The current code seems to assume that zero is an invalid value
    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

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 |   |
| Network Engineer | 0786 F758 55DE 53BA 7731 | |

Home | Main Index | Thread Index | Old Index