Hi Wim,
All watchdog drivers include:
MODULE_ALIAS_MISCDEV(WATCHDOG_MINOR);
which causes a modalias char-major-10-130 to be added to every watchdog
driver module. As a result, any access to /dev/watchdog on a system with
no watchdog driver loaded and working will result in an attempt to load
several dozen drivers. At best one or two will actually work, the others
will:
* Waste time failing to load.
* Waste memory succeeding to load but not finding any device to bind to.
* Pollute the kernel log.
* Sometimes even load while they should not and break the system. I just
had a report about advantechwdt doing that on some systems.
And the attempt order will presumably be random, so it might as well
load softdog before a hardware-based watchdog which would have been
preferred.
This looks so 90s. Drivers for enumerated devices have hardware-based
modaliases, so char-major-10-130 shouldn't be needed. Other drivers
should certainly not be loaded randomly if they need to poke the
hardware to detect the presence of a supported device.
My opinion is that the char-major-10-130 modalias should ONLY be defined
by user-space, when the user knows he/she needs a watchdog driver which
doesn't support auto-loading via hardware-based auto-loading.
So, can we please get rid of all these
MODULE_ALIAS_MISCDEV(WATCHDOG_MINOR) statements? They do more harm than
good as far as I can see.
Thanks,
--
Jean Delvare
Suse L3
On Wed, Jul 03, 2013 at 02:33:23PM +0200, Jean Delvare wrote:
> Hi Wim,
>
> All watchdog drivers include:
>
> MODULE_ALIAS_MISCDEV(WATCHDOG_MINOR);
>
> which causes a modalias char-major-10-130 to be added to every watchdog
> driver module. As a result, any access to /dev/watchdog on a system with
> no watchdog driver loaded and working will result in an attempt to load
> several dozen drivers. At best one or two will actually work, the others
> will:
>
> * Waste time failing to load.
> * Waste memory succeeding to load but not finding any device to bind to.
> * Pollute the kernel log.
> * Sometimes even load while they should not and break the system. I just
> had a report about advantechwdt doing that on some systems.
>
> And the attempt order will presumably be random, so it might as well
> load softdog before a hardware-based watchdog which would have been
> preferred.
>
> This looks so 90s. Drivers for enumerated devices have hardware-based
> modaliases, so char-major-10-130 shouldn't be needed. Other drivers
> should certainly not be loaded randomly if they need to poke the
> hardware to detect the presence of a supported device.
>
> My opinion is that the char-major-10-130 modalias should ONLY be defined
> by user-space, when the user knows he/she needs a watchdog driver which
> doesn't support auto-loading via hardware-based auto-loading.
>
> So, can we please get rid of all these
> MODULE_ALIAS_MISCDEV(WATCHDOG_MINOR) statements? They do more harm than
> good as far as I can see.
>
Agreed.
Can you submit a set of patches ? I'll be happy to add my Reviewed-by: tag
to it. Of course that won't guarantee acceptance ;).
Thanks,
Guenter
Hi Jean,
> All watchdog drivers include:
>
> MODULE_ALIAS_MISCDEV(WATCHDOG_MINOR);
>
> which causes a modalias char-major-10-130 to be added to every watchdog
> driver module. As a result, any access to /dev/watchdog on a system with
> no watchdog driver loaded and working will result in an attempt to load
> several dozen drivers. At best one or two will actually work, the others
> will:
>
> * Waste time failing to load.
> * Waste memory succeeding to load but not finding any device to bind to.
> * Pollute the kernel log.
> * Sometimes even load while they should not and break the system. I just
> had a report about advantechwdt doing that on some systems.
>
> And the attempt order will presumably be random, so it might as well
> load softdog before a hardware-based watchdog which would have been
> preferred.
>
> This looks so 90s. Drivers for enumerated devices have hardware-based
> modaliases, so char-major-10-130 shouldn't be needed. Other drivers
> should certainly not be loaded randomly if they need to poke the
> hardware to detect the presence of a supported device.
>
> My opinion is that the char-major-10-130 modalias should ONLY be defined
> by user-space, when the user knows he/she needs a watchdog driver which
> doesn't support auto-loading via hardware-based auto-loading.
>
> So, can we please get rid of all these
> MODULE_ALIAS_MISCDEV(WATCHDOG_MINOR) statements? They do more harm than
> good as far as I can see.
You have a valid point: There were we have modaliases and other detection
mechanism we should indeed remove them. The rest should be evaluated
afterwards on a case by case basis. Certain intel based drivers should
(like advantechwdt) should indeed be fixed because they poke in the hardware
directly and can't be really detected.
Kind regards,
Wim.
Hi Wim,
Sorry for the very very late reply :(
Le Friday 05 July 2013 à 22:53 +0200, Wim Van Sebroeck a écrit :
> Hi Jean,
>
> > All watchdog drivers include:
> >
> > MODULE_ALIAS_MISCDEV(WATCHDOG_MINOR);
> >
> > which causes a modalias char-major-10-130 to be added to every watchdog
> > driver module. As a result, any access to /dev/watchdog on a system with
> > no watchdog driver loaded and working will result in an attempt to load
> > several dozen drivers. At best one or two will actually work, the others
> > will:
> >
> > * Waste time failing to load.
> > * Waste memory succeeding to load but not finding any device to bind to.
> > * Pollute the kernel log.
> > * Sometimes even load while they should not and break the system. I just
> > had a report about advantechwdt doing that on some systems.
> >
> > And the attempt order will presumably be random, so it might as well
> > load softdog before a hardware-based watchdog which would have been
> > preferred.
> >
> > This looks so 90s. Drivers for enumerated devices have hardware-based
> > modaliases, so char-major-10-130 shouldn't be needed. Other drivers
> > should certainly not be loaded randomly if they need to poke the
> > hardware to detect the presence of a supported device.
> >
> > My opinion is that the char-major-10-130 modalias should ONLY be defined
> > by user-space, when the user knows he/she needs a watchdog driver which
> > doesn't support auto-loading via hardware-based auto-loading.
> >
> > So, can we please get rid of all these
> > MODULE_ALIAS_MISCDEV(WATCHDOG_MINOR) statements? They do more harm than
> > good as far as I can see.
>
> You have a valid point: There were we have modaliases and other detection
> mechanism we should indeed remove them. The rest should be evaluated
> afterwards on a case by case basis. Certain intel based drivers should
> (like advantechwdt) should indeed be fixed because they poke in the hardware
> directly and can't be really detected.
I would suggest a bigger move and remove _all_
MODULE_ALIAS_MISCDEV(WATCHDOG_MINOR) statements. I just can't find any
value in them.
Either the device is enumerated and the driver already has a module
alias (e.g. PCI, USB etc.) that will get the right driver loaded
automatically.
Or the device is not enumerated and loading its driver will lead to more
or less intrusive hardware poking. Such hardware poking should be
limited to a bare minimum, so the user should really decide which
drivers should be tried and in what order. Trying them all in arbitrary
order can't do any good.
On top of that, loading that many drivers at once bloats the kernel log.
Also many drivers will stay loaded afterward, bloating the output of
"lsmod" and wasting memory.
If defining char-major-10-130 is needed then it should happen in
user-space.
Thanks,
--
Jean Delvare
Suse L3 Support