Subject: envsys(4) questions and a proposed extension.
To: None <firstname.lastname@example.org>
From: Luke Mewburn <lukem@NetBSD.org>
Date: 10/09/2005 00:10:26
Content-Type: text/plain; charset=us-ascii
I've been working with envsys(4) recently - both as a userland
consumer and a kernel provider, and I have some questions about
it as well as a proposed extension.
(a) The envsys ioctls are documented in envsys(4) as:
Yet the implementation in <sys/envsys.h> has:
The _temp_ types are equivalent to _tre_ and _basic_
For consistency with the documentation and with "sanity",
I think it would be better if the header file was updated
to use envsys_tre_data_t and envsys_basic_info_t (and the
minor typo in the man page fixed)
(b) The kernel currently assumes that that the sme_gtredata
and sme_streinfo members of 'struct sysmon_envsys' are
not NULL pointers. So, if a kernel provider doesn't
provide these we get a panic deref-ing NULL.
(Look for these members in sys/dev/sysmon/sysmon_envsys.c)
I think that this is suboptimal, and see the following solutions:
1. Check that the function pointer isn't NULL before
using. If NULL, return EINVAL (?) from the ioctl.
This is my preferred solution, from a "don't panic"
PoV, especially since we can have envsys providers
added by LKMs.
2. KASSERT(pointer != NULL)
3. Do nothing.
(c) I have a need to change a sensor (e.g, change the alarm
state, turn on/off a sensor, etc), and I've seen other
comments where people need to change the thresholds
of a fan. The ENVSYS_STREINFO ioctl isn't really
sufficient; I'm currently (ab)using the `rpms' member
of envsys_basic_info_t but that's an ugly hack.
I propose adding a new ioctl
and add support in various envsys providers for this if
appropriate (via a new sme_stredata function pointer).
This ioctl could allow the cur, min, max, and avg members
to be modified if the appropriate ENVSYS_xxxVALID flag is
set in the struct passed into the ioctl.
For each member changed the appropriate bit would be set
on the return of ioctl.
Thoughts on the idea?
I think that this should be optional and so the kernel wouldn't
panic if a driver decided not to implement the new sme_stredata
function pointer, a la (b)(1) above.
Would we need to bump either the kernel's version or the
envsys API version if we added ENVSYS_STREDATA ?
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.1 (NetBSD)
-----END PGP SIGNATURE-----