Skip to contentSkip to conversation

A cleaner load_drivers function.

DISCUSSION Stamatis Mitrofanis 1 participant 3 messages Jan 4, 2002 – Jan 5, 2002

lore
Share & tools
lore
#0 lore raw

Again, sorry for being away for too long.

Quoted 8 lines
Quoted 4 lines

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":

Quoted 8 lines
Quoted 3 lines

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:

Quoted 4 lines
Quoted 4 lines

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).

Quoted 5 lines
Quoted 5 lines

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!

Quoted 8 lines

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
[email protected]
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel

#1 lore raw

Again, sorry for being away for too long.

Quoted 8 lines
Quoted 4 lines

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":

Quoted 8 lines
Quoted 3 lines

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:

Quoted 4 lines
Quoted 4 lines

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).

Quoted 5 lines
Quoted 5 lines

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!

Quoted 8 lines

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
[email protected]
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel

#2 lore raw
In reply to Stamatis Mitrofanis
Quoted 8 lines · Stamatis Mitrofanis

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

Quoted 7 lines
    # 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
[email protected]
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel