PERFORCE change 126230 for review

[Constantine A. Murenin]

On 26/09/2007 13:25, John Baldwin wrote:
> On Wednesday 26 September 2007 12:26:45 pm Constantine A. Murenin wrote:
> 
>>On 26/09/2007 11:21, John Baldwin wrote:
>>
>>>On Sunday 09 September 2007 02:00:36 pm Constantine A. Murenin wrote:
>>>
>>>
>>>>http://perforce.freebsd.org/chv.cgi?CH=126230
>>>>
>>>>Change 126230 by cnst at dale on 2007/09/09 18:00:16
>>>>
>>>>	put in a hack for supporting "Sysctl internal magic",
>>>>	and now hw.sensors tree magically works in sysctl(8)!
>>>>	
>>>>	dale# sysctl hw.sensors.{lm0.volt{0,1,2,3},cpu{0,1}}
>>>>	hw.sensors.lm0.volt0: 1.23 VDC (VCore)
>>>>	hw.sensors.lm0.volt1: 12.30 VDC (+12V)
>>>>	hw.sensors.lm0.volt2: 3.33 VDC (+3.3V)
>>>>	hw.sensors.lm0.volt3: 3.31 VDC (+3.3V)
>>>>	hw.sensors.cpu0.temp0: 28.00 degC
>>>>	hw.sensors.cpu1.temp0: 28.00 degC
>>>>	dale#
>>>>	
>>>>	(All other utilities continue working using a cross-platform
>>>>	sysctl(3) interface, compatible with OpenBSD.)
>>>>
>>>>+#endif /* !NOSYSCTL8HACK */
>>>>+
>>>>+
>>>>+#ifndef NOSYSCTL8HACK
>>>>+
>>>>+/*
>>>>+ * XXX:
>>>>+ * FreeBSD's sysctl(9) .oid_handler functionality is not accustomed
>>>>+ * for the CTLTYPE_NODE handler to handle the undocumented sysctl
>>>>+ * magic calls.  As soon as such functionality is developed, 
>>>>+ * sysctl_sensors_handler() should be converted to handle all such
>>>>+ * calls, and these sysctl_add_oid(9) calls should be removed 
>>>>+ * "with a big axe".  This whole sysctl_add_oid(9) business is solely
>>>>+ * to please sysctl(8).
>>>
>>>
>>>Actually, sysctl_add_oid(9) is how you should add a tree of sysctl's on 
> 
> the
> 
>>>fly to FreeBSD.  It frees you from having to manually simulate a sysctl
>>>tree yourself and instead focus on just handling the functionality for the
>>>leaf nodes.  If you just gave each sensor its own sysctl ctx and tree most
>>>of your in-kernel code for dealing with 'hw.sensors' would go away as it
>>>would just be a normal node ala 'dev'.
>>
>>If you take a closer look, the code for manually keeping track of 
>>sensors is very straightforward -- mostly a linked list.  sysctl(9) 
>>interface is much more complicated than that.
>>
>>This manual accounting of the sensors is a requirement for the sensors 
>>framework -- it is the basis for the concept of sensor device, which 
>>stores information about individual sensors each device has (similarly 
>>to a sysctl_ctx).  It is also required to generate and keep track of the 
>>numt of each sensor.
>>
>>Using FreeBSD's internal magic does not make the userland applications 
>>portable, because there is no documented userland interface for listing 
>>all leaves of a given sysctl tree.  (If you believe otherwise, a link to 
>>the manual page is welcome. :)
> 
> 
> See device_t's.  Each one has an associated sysctl ctx (and tree) while still 
> having its own internal hierarchy of devices that is used to provide 
> enumeration APIs to userland.  It's fine to use a separate interface other 
> than sysctl to tell userland which sensors exist (or you could have a sysctl 
> whose value was a list of the sensor names, etc.), but sysctl_add_oid(9) 
> means you don't need an actual 'hw.sensors' sysctl proc routine to manage the 
> sysctl namespace for you.

But I why would I have a separate interface for showing sensor hierarchy 
if the sysctl interface can accomplish this purpose with FreeBSD's 
sysctl(9) .oid_handler and the sensordev structure, making the userland 
part of the framework entirely source-code compatible between OpenBSD 
and FreeBSD at the same time?

Also, what is wrong with having a sysctl .oid_handler for managing the 
sysctl namespace?  The .oid_handler functionality has been in the 
FreeBSD tree for ages -- why is it there if not for me to use it as it 
was designed for? :-)

C.