A cleaner load_drivers function.

A cleaner load_drivers function.

[Stamatis Mitrofanis]

Again, sorry for being away for too long.

>>There's not much in this "common logic" anyway. The way the function is 
>>written makes it seem like there is. It's huge and full of unnecessary 
>>things
>>
>Then we'll just disagree.  I've used versions with and without code
>sharing, and I know which style of agent is smaller.  And I've worked
>with systems designed the way you're advocating (no shared code).
>
I'm not advocating no shared code at all, but _minimal_ shared code. 
Because hotplug agents don't _necessarily_ talk "kernel module" the 
load_drivers function seems out of place in a global file like 
hotplug.functions . It may be better placed in a different sourced file 
(say /etc/hotplug/driver.functions).

My problem is that load_drivers contains many things that are 
unnecessary and/or out of place, making it "bloated":

>>So off goes the *_map_functions bloat of load_driver . It can simply use 
>>*modules .
>>
>No it can't.  "usbdevfs" is completely optional, and most systems
>don't have either "usbmodules" or "pcimodules".  The *_map functions
>are the required parts, the rest is optional gravy (mostly there to deal
>with the "coldplug" cases).
>
You didn't notice part of my post:
 >>Shell functions are useful as "emulations" of actual programs. Defining
 >>the meaning of *_map_modules doesn't really do much when there can be a
 >>shell script implementation of *modules (which is already defined to do
 >>the same job).
(I implied that shell functions can be conditionally defined)

Of course, the shell function *modules can use the provided environment 
variables instead of the proc fs entry.

But the best solution to the assorted FIXME is to define (more general) 
*map functions that don't need command-line parameters. *map is defined 
by its agent and is called by load_drivers . (patch on the way)

So, that lessens the "bloat"! It removes that unwanted "case" as the 
FIXME wants to be done.

/****/

What's left in load_modules now (after the "bloat" is removed) is _just 
the following logic_, which is not much (even the current logic isn't).

>>instead of calling the script _after_ the module is loaded...
>>
>>_call_the_script_instead_of_loading_the_module_ ,
>>and if it doesnt exist, do load the kernel module.
>>

To put it more clearly: just load the module unless there exists 
/etc/hotplug/TYPE/DRIVER.agent  to delegate to.

Note the `.agent' extension!

>I've thought about that before, but it's got some problems too.
>
>For one, it'd break existing scripts, and (often worse) knowledge
>that folk have already about using them.For another, there's no
>compelling example motivating such a change -- no problem
>that folk have hit (real-world) that doesn't have a good solution
>within the existing framework.
>

You didn't mention any actual problems with the "new framework". Just 
that "it's a change".

It's easy to add backwards compatibility to this interface, if that's a 
problem. It can even be automated.

IMO, this driver-agent interface is much more forward-looking and 
uniform. You just can't exclude the possibility that a driver may want 
to do something device-specific in user-space _before_ `modprobe'ing.



_______________________________________________
Linux-hotplug-devel mailing list  http://linux-hotplug.sourceforge.net
Linux-hotplug-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel

A cleaner load_drivers function.

[Stamatis Mitrofanis]

Again, sorry for being away for too long.

>>There's not much in this "common logic" anyway. The way the function is 
>>written makes it seem like there is. It's huge and full of unnecessary 
>>things
>>
>Then we'll just disagree.  I've used versions with and without code
>sharing, and I know which style of agent is smaller.  And I've worked
>with systems designed the way you're advocating (no shared code).
>
I'm not advocating no shared code at all, but _minimal_ shared code. 
Because hotplug agents don't _necessarily_ talk "kernel module" the 
load_drivers function seems out of place in a global file like 
hotplug.functions . It may be better placed in a different sourced file 
(say /etc/hotplug/driver.functions).

My problem is that load_drivers contains many things that are 
unnecessary and/or out of place, making it "bloated":

>>So off goes the *_map_functions bloat of load_driver . It can simply use 
>>*modules .
>>
>No it can't.  "usbdevfs" is completely optional, and most systems
>don't have either "usbmodules" or "pcimodules".  The *_map functions
>are the required parts, the rest is optional gravy (mostly there to deal
>with the "coldplug" cases).
>
You didn't notice part of my post:
 >>Shell functions are useful as "emulations" of actual programs. Defining
 >>the meaning of *_map_modules doesn't really do much when there can be a
 >>shell script implementation of *modules (which is already defined to do
 >>the same job).
(I implied that shell functions can be conditionally defined)

Of course, the shell function *modules can use the provided environment 
variables instead of the proc fs entry.

But the best solution to the assorted FIXME is to define (more general) 
*map functions that don't need command-line parameters. *map is defined 
by its agent and is called by load_drivers . (patch on the way)

So, that lessens the "bloat"! It removes that unwanted "case" as the 
FIXME wants to be done.

/****/

What's left in load_modules now (after the "bloat" is removed) is _just 
the following logic_, which is not much (even the current logic isn't).

>>instead of calling the script _after_ the module is loaded...
>>
>>_call_the_script_instead_of_loading_the_module_ ,
>>and if it doesnt exist, do load the kernel module.
>>

To put it more clearly: just load the module unless there exists 
/etc/hotplug/TYPE/DRIVER.agent  to delegate to.

Note the `.agent' extension!

>I've thought about that before, but it's got some problems too.
>
>For one, it'd break existing scripts, and (often worse) knowledge
>that folk have already about using them.For another, there's no
>compelling example motivating such a change -- no problem
>that folk have hit (real-world) that doesn't have a good solution
>within the existing framework.
>

You didn't mention any actual problems with the "new framework". Just 
that "it's a change".

It's easy to add backwards compatibility to this interface, if that's a 
problem. It can even be automated.

IMO, this driver-agent interface is much more forward-looking and 
uniform. You just can't exclude the possibility that a driver may want 
to do something device-specific in user-space _before_ `modprobe'ing.



_______________________________________________
Linux-hotplug-devel mailing list  http://linux-hotplug.sourceforge.net
Linux-hotplug-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel

Re: A cleaner load_drivers function.

[Stamatis Mitrofanis]

Stamatis Mitrofanis wrote:

 > To put it more clearly: just load the module _unless_ there exists
 > /etc/hotplug/TYPE/DRIVER.agent  to delegate to.
 >
 > Note the `.agent' extension!
 >
 > IMO, this driver-agent interface is much more forward-looking and
 > uniform. You just can't exclude the possibility that a driver may want
 > to do something device-specific in user-space _before_ `modprobe'ing.

The driver-specific agents can do some _additional filtering_ this way.
  (I forgot that -- why do I keep forgetting to mention things? :-( )

The input subsystem can be an example. It has many bitmasks, some more
than 32 bits.

ACTION ... add|remove
PRODUCT ... idbus/idvendor/idproduct/idversion
NAME ... Device name from device descriptor.
PHYS ... pci00:07.2/usb0:2.3/input0 (device physical address ID).
EV ... Bitmask (32 bits) for the kinds of events this device supports.
KEY ... 511 bits for the keys&buttons the device has.
REL ... 15 bits for relative axes (x/y/z, mouse-wheel etc.) it provides.
ABS ... 31 bits for absolute axes (x/y/z, throttle, pressure etc.).
LED ... 15 bits for the LEDs on the device (num/caps-lock etc.).
SND ... 7 bits for the characteristic sounds the device can make (beep!).
FF ... 127 bits for supported force-feedback effects!
MSC ... 7 more bits for the rest.

It would be great if the input-driver-specific agents could do some
extra filtering as appropriate.

/****/

Here is a _tiny_ patch that will make load_drivers delegate (full
backward compatibility):

diff -r linux-hotplug/etc/hotplug/hotplug.functions
linux-hotplug2/etc/hotplug/hotplug.functions
123a124,130
  >     # Available driver-specific agent?
  >     if [ -f "/etc/hotplug/$TYPE/$MODULE.agent" -a \
  >          -x "/etc/hotplug/$TYPE/$MODULE.agent" ]; then
  >         "/etc/hotplug/$TYPE/$MODULE.agent"
  >         continue
  >     fi
  >

Now, the driver-specific agent (if available) can handle `add' events
for devices it most likely handles. It can forget about devices it
doesn't really handle.

It would be a good idea to extend the `load_drivers' interface so that
it handles `remove' events as well.




_______________________________________________
Linux-hotplug-devel mailing list  http://linux-hotplug.sourceforge.net
Linux-hotplug-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel