EISA & sysfs.

EISA & sysfs.

[Dave Jones]

I'm somewhat puzzled about the case where we have a driver
that can work on EISA bus, as well as others, when modprobe'd on
a system that doesn't have an EISA bus.

It seems we do a probe really early on to see if we actually
have an eisa bus, but if a driver later calls eisa_driver_register()
we still do lots of hoop jumping through sysfs/kobjects
before deciding that we don't have the device.

Wouldn't it make sense to have eisa_driver_register() check that the
root EISA bus actually got registered, and if not, -ENODEV
immediately ?

		Dave

Re: EISA & sysfs.

[Marc Zyngier]

>>>>> "Dave" == Dave Jones <davej@redhat.com> writes:

Dave> Wouldn't it make sense to have eisa_driver_register() check that the
Dave> root EISA bus actually got registered, and if not, -ENODEV
Dave> immediately ?

Most of the time, the bus driver kicks in *after* the device driver is
registered to the EISA framework (eisa is second to last in the driver
list, so if the driver is built-in, it is guaranted to init before the
root driver has a chance to discover the bus).

So, returning -ENODEV immediatly in this case prevents you from using
any built-in EISA driver. A possible solution to this problem would be
to move eisa just after the pci init (and even that would cause some
trouble, because the virtual_root driver would register before the
parisc root driver has a chance to be probed...).

So yes, this sucks, but I can't come up with a better solution...

Regards,

	M.
-- 
Places change, faces change. Life is so very strange.

Re: EISA & sysfs.

[Dave Jones]

On Wed, Feb 18, 2004 at 10:42:49AM +0100, Marc Zyngier wrote:

 > Dave> Wouldn't it make sense to have eisa_driver_register() check that the
 > Dave> root EISA bus actually got registered, and if not, -ENODEV
 > Dave> immediately ?
 > 
 > Most of the time, the bus driver kicks in *after* the device driver is
 > registered to the EISA framework (eisa is second to last in the driver
 > list, so if the driver is built-in, it is guaranted to init before the
 > root driver has a chance to discover the bus).

sounds like the initcall needs a different priority.

 > So, returning -ENODEV immediatly in this case prevents you from using
 > any built-in EISA driver. A possible solution to this problem would be
 > to move eisa just after the pci init (and even that would cause some
 > trouble, because the virtual_root driver would register before the
 > parisc root driver has a chance to be probed...).
 > 
 > So yes, this sucks, but I can't come up with a better solution...

This problem is not just cosmetic btw, it kills boxes.
For example, hp100 is a net driver that supports multiple busses.
Trying to modprobe it on a kernel that supports EISA on a box that
doesn't gets a hung modprobe. Backtrace shows..

 modprobe      D 00000082     0 23407  15920                     (NOTLB)
        c1fe3f2c 00000082 c031ba08 00000082 ffffffff c031bb90 cc318219 00000092
        c6ee8ca0 c6ee8cc0 c111acc0 0040603e cc532c37 00000092 c267eca0 c267ee70
        c6c6375c 00000001 000005a0 c035ffcc ffffffff c7c5e644 c267eca0 c01d4583
 Call Trace:
  [<c01d4583>] rwsem_down_write_failed+0x141/0x15c
  [<c01d3742>] .text.lock.kobject+0x36/0x74
  [<c01d339a>] kobject_register+0x19/0x39
  [<c0221a5c>] bus_add_driver+0x2e/0x83
  [<c02622d4>] eisa_driver_register+0xf/0x19
  [<c786e91e>] hp100_module_init+0x12/0x2e [hp100]
  [<c013c0c0>] sys_init_module+0x14e/0x25e
  [<c010b697>] syscall_call+0x7/0xb

I've seen same exactly the same behaviour with quite a few other modules.
For my 'modprobe/rmmod script-o-death', I just ended up disabling EISA
in that test tree, as it was too painful to hit this issue over and over,
but its a real situation that could bite users of for eg, vendor kernels.

		Dave

Re: EISA & sysfs.

[Marc Zyngier]

>>>>> "Dave" == Dave Jones <davej@redhat.com> writes:

Dave> This problem is not just cosmetic btw, it kills boxes.
Dave> For example, hp100 is a net driver that supports multiple busses.
Dave> Trying to modprobe it on a kernel that supports EISA on a box that
Dave> doesn't gets a hung modprobe. Backtrace shows..

[...]

It looks like hp100 is at least broken wrt probing, since it lacks a
EISA-ID table terminator. What about changing it to :

static struct eisa_device_id hp100_eisa_tbl[] = {
        { "HWPF180" }, /* HP J2577 rev A */
        { "HWP1920" }, /* HP 27248B */
        { "HWP1940" }, /* HP J2577 */
        { "HWP1990" }, /* HP J2577 */
        { "CPX0301" }, /* ReadyLink ENET100-VG4 */
        { "CPX0401" }, /* FreedomLine 100/VG */
        { "" },        /* THIS ENTRY IS MANDATORY !!! */
};

Dave> I've seen same exactly the same behaviour with quite a few other
Dave> modules.  For my 'modprobe/rmmod script-o-death', I just ended
Dave> up disabling EISA in that test tree, as it was too painful to
Dave> hit this issue over and over, but its a real situation that
Dave> could bite users of for eg, vendor kernels.

What are the other modules ? I'd like to reproduce the problem (I have
no PCI hp100 to check if the above fixes the problem).

Regards,

	M.
-- 
Places change, faces change. Life is so very strange.

Re: EISA & sysfs.

[Dave Jones]

On Wed, Feb 18, 2004 at 04:24:15PM +0100, Marc Zyngier wrote:

 > It looks like hp100 is at least broken wrt probing, since it lacks a
 > EISA-ID table terminator. What about changing it to :
 > 
 > static struct eisa_device_id hp100_eisa_tbl[] = {
 >         { "HWPF180" }, /* HP J2577 rev A */
 >         { "HWP1920" }, /* HP 27248B */
 >         { "HWP1940" }, /* HP J2577 */
 >         { "HWP1990" }, /* HP J2577 */
 >         { "CPX0301" }, /* ReadyLink ENET100-VG4 */
 >         { "CPX0401" }, /* FreedomLine 100/VG */
 >         { "" },        /* THIS ENTRY IS MANDATORY !!! */
 > };

I'll give it a try in a few minutes..

 > Dave> I've seen same exactly the same behaviour with quite a few other
 > Dave> modules.  For my 'modprobe/rmmod script-o-death', I just ended
 > Dave> up disabling EISA in that test tree, as it was too painful to
 > Dave> hit this issue over and over, but its a real situation that
 > Dave> could bite users of for eg, vendor kernels.
 > 
 > What are the other modules ?

See the 'modprobe script-o-doom' I posted last night. Just loading and
unloading various modules causes crashes.

 > I'd like to reproduce the problem (I have
 > no PCI hp100 to check if the above fixes the problem).

Me either. This is the case that crashes. I'll bet it works fine
if you actually have hardware, as that was the case that got tested
when the driver got changed last.

		Dave

Re: EISA & sysfs.

[Dave Jones]

On Wed, Feb 18, 2004 at 04:24:15PM +0100, Marc Zyngier wrote:

 > EISA-ID table terminator. What about changing it to :
 > 
 > static struct eisa_device_id hp100_eisa_tbl[] = {
 >         { "HWPF180" }, /* HP J2577 rev A */
 >         { "HWP1920" }, /* HP 27248B */
 >         { "HWP1940" }, /* HP J2577 */
 >         { "HWP1990" }, /* HP J2577 */
 >         { "CPX0301" }, /* ReadyLink ENET100-VG4 */
 >         { "CPX0401" }, /* FreedomLine 100/VG */
 >         { "" },        /* THIS ENTRY IS MANDATORY !!! */
 > };

ok, that stops it hanging at least, it now barfs a little failure
message, and a calltrace.  This seems awfully verbose for a failure
path that isn't unreasonable IMO.

kernel: kobject_register failed for hp100 (-17)
kernel: Call Trace:
kernel:  [<c01d3662>] kobject_register+0x31/0x39
kernel:  [<c0221d0c>] bus_add_driver+0x2e/0x83
kernel:  [<c01db6db>] pci_register_driver+0x6b/0x87
kernel:  [<c78078ad>] hp100_module_init+0x12/0x22 [hp100]
kernel:  [<c013c0fc>] sys_init_module+0x14e/0x25e
kernel:  [<c010b697>] syscall_call+0x7/0xb


Something also seems awry someplace else..

(15:54:51:root@mindphaser:linux-2.6.2)# cat /proc/modules  | grep hp100
(15:54:55:root@mindphaser:linux-2.6.2)# rmmod hp100
ERROR: Module hp100 does not exist in /proc/modules
(15:55:18:root@mindphaser:linux-2.6.2)# modprobe hp100
FATAL: Module hp100 already in kernel.
(15:55:25:root@mindphaser:linux-2.6.2)# cat /proc/modules | grep hp100
(15:55:33:root@mindphaser:linux-2.6.2)#

Odd.

	Dave

Re: EISA & sysfs.

[Marc Zyngier]

>>>>> "Dave" == Dave Jones <davej@redhat.com> writes:

Dave> kernel: kobject_register failed for hp100 (-17)

-17 == -EEXIST.

Looks like the driver was already loaded once, or managed to leave
some sh*t into sysfs.

Dave> Something also seems awry someplace else..

Dave> (15:54:51:root@mindphaser:linux-2.6.2)# cat /proc/modules  | grep hp100
Dave> (15:54:55:root@mindphaser:linux-2.6.2)# rmmod hp100
Dave> ERROR: Module hp100 does not exist in /proc/modules
Dave> (15:55:18:root@mindphaser:linux-2.6.2)# modprobe hp100
Dave> FATAL: Module hp100 already in kernel.
Dave> (15:55:25:root@mindphaser:linux-2.6.2)# cat /proc/modules | grep hp100
Dave> (15:55:33:root@mindphaser:linux-2.6.2)#

Dave> Odd.

I've already seen that with half-initialized modules...

	M.
-- 
Places change, faces change. Life is so very strange.

Re: EISA & sysfs.

[Bartlomiej Zolnierkiewicz]

Hi,

Consider this:
- both CONFIG_EISA and CONFIG_PCI are defined
- hp100_isa_init() returns an error (ie. -ENODEV)
- eisa_driver_register() and pci_module_init() are called
- hp100_module_init() returns -ENODEV,
  module fails to load, so hp100_module_exit() is not called
- &hp100_eisa_driver and &hp100_pci_driver are still happily
  registered with sysfs

This is far too common mistake... :-(

On Wednesday 18 of February 2004 17:12, Marc Zyngier wrote:
> >>>>> "Dave" == Dave Jones <davej@redhat.com> writes:
>
> Dave> kernel: kobject_register failed for hp100 (-17)
>
> -17 == -EEXIST.
>
> Looks like the driver was already loaded once, or managed to leave
> some sh*t into sysfs.
>
> Dave> Something also seems awry someplace else..
>
> Dave> (15:54:51:root@mindphaser:linux-2.6.2)# cat /proc/modules  | grep
> hp100 Dave> (15:54:55:root@mindphaser:linux-2.6.2)# rmmod hp100
> Dave> ERROR: Module hp100 does not exist in /proc/modules
> Dave> (15:55:18:root@mindphaser:linux-2.6.2)# modprobe hp100
> Dave> FATAL: Module hp100 already in kernel.
> Dave> (15:55:25:root@mindphaser:linux-2.6.2)# cat /proc/modules | grep
> hp100 Dave> (15:55:33:root@mindphaser:linux-2.6.2)#
>
> Dave> Odd.
>
> I've already seen that with half-initialized modules...
>
> 	M.

Re: EISA & sysfs.

[Greg KH]

On Wed, Feb 18, 2004 at 03:53:17PM +0000, Dave Jones wrote:
> 
> ok, that stops it hanging at least, it now barfs a little failure
> message, and a calltrace.  This seems awfully verbose for a failure
> path that isn't unreasonable IMO.
> 
> kernel: kobject_register failed for hp100 (-17)
> kernel: Call Trace:
> kernel:  [<c01d3662>] kobject_register+0x31/0x39
> kernel:  [<c0221d0c>] bus_add_driver+0x2e/0x83
> kernel:  [<c01db6db>] pci_register_driver+0x6b/0x87
> kernel:  [<c78078ad>] hp100_module_init+0x12/0x22 [hp100]
> kernel:  [<c013c0fc>] sys_init_module+0x14e/0x25e
> kernel:  [<c010b697>] syscall_call+0x7/0xb

It's not "unresonable", it just means that someone messed up and we are
now letting the user/developer know.  It's normally because someone
named their driver the same name as another one.

Actually, it's nice to see this message get printed out, and not the
"normal" oops that happens around this area for this case.  Something
must be working correctly :)

I'll go fix that printk to put the proper KERN_ level on it...

thanks,

greg k-h