Hello,
I've made some changes to devparam.
1. dev_paramset and aux_paramsets are merged. So, there are now only
two categories of paramesets bus paramsets and dev paramsets. Because
bus paramset is used before a driver's probe() function is invoked and
there's no function call to pass bus paramset, merging bus paramset
into dev paramsets wouldn't work very well. This merging also
simplified devparam implementation a bit.
2. There's no need to explicitly initialize/release setdefs anymore;
therefore, devparam_bus_init/release(), devparam_setdef_init/release()
aren't needed anymore.
3. devparm related fields in struct bus_type, device and
device_driver are renamed. They're simpler now.
4. sysfs directory "params" renamed to "parameters"
The updated document and dptest modules are available at the
following URLs.
http://home-tj.org/devparam/devparam.txt
http://home-tj.org/devparam/dptest.tar.gz
And here are my opinions about issues to be resolved. Mostly they
are repeatitions from my previous mails but I tried to add some more
support for my opinions. :-) As soon as the following issues are
resolved in one way or the other, I'll post the updated patches.
1. vector
I've grepped kernel source and found a few constants which can be
removed if converted to use vector. The first one is ACPI_MAX_HANDLES
defined and used in include/acpi/acpi_bus.h. The second one is
USB_MAXINTERFACES in include/linux/usb.h and the last one
CONFIG_DLCI_MAX in drivers/net/wan/sdla.c.
Constructor/destructor and expanding/shrinking in the middle can be
chopped out if the current implementation of vector is too chunky, but
devparam implementation needs some form of dynamically expandable data
structure which can be indexed and hard coding it into devparam
doesn't look very attractive. And dynamically expandable vectors are
useful, really!
2. module_param_*_ranged() stuff
I've looked at a number of places where module param is used, and
most parameters need range checking. Only a few parameters actually
accept full range of the designated type. For example, maxbatch in
kernel/rcupdate.c is currently of type int but values smaller than 1
doesn't make much sense. All ports array parameter in
net/ipv4/netfilter/*.c should be in the range of 0-65535 and most
device driver parameters need range-checking too. I think
incorporating range into moduleparam can reduce duplicate codes and
improve consistency regarding handling of out-of-range parameters.
Yes, the number of functions in module/devparam is increased but
they're straightforward and it's not like actual code is bloated.
3. In array parameters, discarding the number of set array elements
instead of using param_array's max field to store it when nump isn't
supplied.
I think that it's fair and clear to require the user to supply nump
field to param_array() when the number of set array element matters
either to the driver or the user space reading the value off sysfs
node. If nump is there, number of set elements is recorded and thus
reflected in the sysfs node. If nump is NULL, number of set element
isn't recorded and sysfs node just shows all the elements.
Also, it's impossible to maintain the same semantics in devparam.
Wrapping arguments are transient in devparam and cannot carry/maintain
any dynamic information.
4. param_array/arr()
I also think using NULL is fine. But the problem is that currently
there's no way to specify NULL in the same field in
DEVICE_PARAM_ARRAY(). I think it's better to maintain interface
consistency between moduleparam and devparam than to be more compact
in moduleparam.
Except for param_array/arr(), currently the only difference in
interface is module_param_string() and
DEVICE_PARAM_STRING_NAMED()/DEVICE_PARAM_STRING(). I think
module_param_array() is a bit weird in that it differs from all other
module_param_*() macros, but they can be coerced either way, either
moduleparam or devparam part can be changed.
Also, if it's decided that the number of set array elements is thrown
away, the different macros should make the distinction clearer.
Thanks.
--
tejun
On Wed, 2004-10-27 at 15:32 +0900, Tejun Heo wrote:
> 4. sysfs directory "params" renamed to "parameters"
Probably a good idea, but I'm reluctant to endorse userspace-visible
breakage. We've already done that once 8)
> 2. module_param_*_ranged() stuff
>
> I've looked at a number of places where module param is used, and
> most parameters need range checking. Only a few parameters actually
> accept full range of the designated type. For example, maxbatch in
> kernel/rcupdate.c is currently of type int but values smaller than 1
> doesn't make much sense. All ports array parameter in
> net/ipv4/netfilter/*.c should be in the range of 0-65535 and most
> device driver parameters need range-checking too. I think
> incorporating range into moduleparam can reduce duplicate codes and
> improve consistency regarding handling of out-of-range parameters.
>
> Yes, the number of functions in module/devparam is increased but
> they're straightforward and it's not like actual code is bloated.
Well, netfilter ports should be a u16, so it's naturally contrained
(although 0 would be very unusual). If max_batch were unsigned, rcu
would be fixed too (although we have 0 again as the odd case).
I don't really want the churn of making them all take range args
and adding "#define PARAM_NO_RANGE 0, -1". Perhaps doubling the number
of macros is the right approach.
> 3. In array parameters, discarding the number of set array elements
> instead of using param_array's max field to store it when nump isn't
> supplied.
>
> I think that it's fair and clear to require the user to supply nump
> field to param_array() when the number of set array element matters
> either to the driver or the user space reading the value off sysfs
> node.
Fortunately sprintf("%s") handles NULL, which was my main concern, so
this is OK IMHO.
I think my bigger question is: is this going in the right direction?
Should we instead be doing something like eg. "block/hda/dev=3:0", and
trying to connect the pipes so the driver's sysfs entries are
initialized by the parameter parsing code?
Should we be using module parameters to control device settings at all?
I don't know the answers here, but I share your discomfort over the
current module parameter <-> device parameter mapping.
(Hoping Greg KH will have an opinion here...)
Thanks!
Rusty.
--