On 17/10/2007 09:03, John Baldwin wrote:
On Tuesday 16 October 2007 06:46:58 pm Constantine A. Murenin wrote:
On 16/10/2007 18:09, John Baldwin wrote:
On Tuesday 16 October 2007 05:46:18 pm Constantine A. Murenin wrote:
On 16/10/2007, John Baldwin <[EMAIL PROTECTED]> wrote:
On Tuesday 16 October 2007 12:33:11 pm Alexander Leidinger wrote:
Constantine asked for review several times on -current. He got some
reviews several times for commits to perforce. He incorporated
suggestions from those reviews, or explained why it is like it is and
why he can not switch (with no replies with suggestions how to solve
the problems he sees with the suggestions). Now you come and ask why
nobody pointed out some flaws before (without telling us which
technical flaws you talk about).
At least from my point of view this is not quite accurate as pretty much
all
my feedback to the p4 commits was ignored with basically "Well, I don't
like
doing it that way". Specifically, with regards to creating dynamic sysctl
trees, Constantine feels that sysctl_add_oid(9) is a hack rather than
recognizing that this is a feature of FreeBSD's sysctl system despite
repeated e-mails on the subject.
Dear John,
I have specifically addressed this concern of yours just several weeks ago.
Please see the following message, which you've never replied to:
http://lists.freebsd.org/pipermail/p4-projects/2007-September/021121.html
I've used the documented parts of the FreeBSD's sysctl interface to
preserve 100% userland compatibility with OpenBSD.
FreeBSD already provides an interface for creating dynamic sysctl trees that
is simpler than having to simulate a pseudo-tree via the .oid_handler
routine. In some cases (such as kern.proc) FreeBSD still uses a function
handler rather than giving each process its own sysctl node. However, in
other cases (generally more recent ones, and ones not as large/performance
impacting) such as dev.* or the recent proposal to give ifnet's their own
nodes under 'net.if' or the like, sysctl_add_oid(9) is used.
Which one is simpler is questionable. Take one more look at the number
of different macros that are available in the sysctl(9) and friends, and
then compare with four functions of the sensors framework:
* sensor_attach(9) to attach a sensor to a sensordev tree
* sensordev_install(9) to register the sensordev tree with sysctl(9)
and the same with the "de" prefix
sensordev_install(9) acts similarly to the sysctl(9) ctx concept, but is
geared towards the sensors approach
If you want to make sure you don't attract any new contributors, then
certainly the bunch of the sysctl(9) macros are simpler. ;)
I never said to not have sensor_*() routines. I just think that the sensor
implementation should make use of dynamic sysctls to build the sysctl
interface rather than treating dynamic sysctls as a temporary hack and adding
a duplicate interface for walking the hw.sensors tree.
I guess I see where you're coming from here. You are concerned that the
hack to register sensor_attach'ed sensors with the sysctl(8) magic all
at once may conflict with additional drivers, for example, if
sensor_attach is called after sensordev_install.
OpenBSD 4.2 has more than 50 drivers using this framework, and all of
them satisfy the assertion that the above doesn't happen -- no
sensor_attach calls are ever made after sensordev_install. On a similar
note, the sensor_detach function is redundant -- the few drivers that
actually use it won't notice any difference if it is removed in its
entirety -- sensordev_deinstall automatically removes all sensors of a
given sensordev. (Granted, this might be documented a bit more
explicitly, but this has not been a problem so far.)
In other words, it is usually the devices that hold sensors that are
hotpluggable, not the individual sensors themselves. So in the realm of
the sensor framework, the sysctl magic for sysctl(8) is not considered
to be a temporary hack.
As to the process of walking sysctl trees being undocumented, it is simply
missing a wrapper routine ala sysctlbyname(3) and a manpage. You could
easily provide one and thus provide a facility for enumerating many different
things than having several one-off oid_handler routines to enumerate
subtrees. However, it is not some "backdoor" hack interface anymore than
sysctlbyname(3) is. They are both equally hackish or non-hackish (depending
on your point of view).
This interface is about having a special tree for the hardware sensors,
with special rules regarding the namespace. What you suggest is that I
try to abandon this, but then why do you need this framework to start with?
I suggest you think in layered abstractions. sysctl is a tool for creating
a namespace that the kernel exports to userland. Let sysctl manage the
namespace and the sensor code just populate nodes in the namespace and manage
sensors.
You are missing some point here. With this interface, sysctl is simply
used as a transfer mechanism to export two structures:
* struct sensordev, which defines how further sensors on a given device
can be addressed and how many sensors of each type a given device has
(this is something that would not be possible to export via the sysctl
magic alone, BTW)
* struct sensor, which provides readings of individual sensors on a
given sensor device
The above gives this framework the potential to be source-code
compatible with any kind of sysctl interface, be that original
non-dynamic sysctl, FreeBSD's dynamic sysctl, or possibly even NetBSD's
dynamic sysctl. It also provides additional benefits -- the whole tree
of a given sensordev can be traversed with completely ignoring sensors
of certain types. What you are suggesting is that a FreeBSD-only
sysctl-specific functions are used instead, also limiting the additional
functionality provided by sensordev structure, but I see no technical
benefit in this. (I hope my explanation above about the specific hack
in relation to OpenBSD's existing drivers addresses your concern for good.)
Thus the two-layered version of the framework (sensordev/sensor) defines
its own namespace. Granted, on OpenBSD this is much more valuable than
it is on FreeBSD, since FreeBSD already has dynamic sysctl mechanism
(which is quite polluted by irrelevant and unused values, IMHO).
However, many people are quite certain that the sensor framework is
nonetheless valuable even on FreeBSD, too. It provides something that
the plain FreeBSD dynamic sysctl interface doesn't provide -- simplicity
of use for sensor-like devices and a common namespace with clearly
defined restrictions.
Cheers,
Constantine.
_______________________________________________
cvs-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/cvs-all
To unsubscribe, send any mail to "[EMAIL PROTECTED]"
