Skip to contentSkip to conversation

sysfs: cannot create duplicate filename

PATCH In review Sebastian Ott 2 participants 12 messages Apr 11, 2011 – Apr 12, 2011 Driver core

lore
Share & tools
lore

Sebastian Ott

author
#0 lore raw

Hi,

i've seen this warning which looks to be caused by a race between device_add and driver_register

[   80.893594] sysfs: cannot create duplicate filename '/bus/ccw/drivers/qeth/0.0.b57d'
[   80.893611] ------------[ cut here ]------------
[   80.893614] WARNING: at /home/autobuild/BUILD/linux-2.6.38.2-20110404/fs/sysfs/dir.c:455
[   80.893617] Modules linked in: qeth ccwgroup
[   80.893623] Modules linked in: qeth ccwgroup
[   80.893629] CPU: 1 Not tainted 2.6.38.2-48.x.20110404-s390xdefault #1
[   80.893632] Process kworker/u:1 (pid: 25, task: 000000007e6c5a40, ksp: 000000007e6cb980)
[   80.893635] Krnl PSW : 0704000180000000 00000000002b676c (sysfs_add_one+0xd0/0xe8)
[   80.893643]            R:0 T:1 IO:1 EX:1 Key:0 M:1 W:0 P:0 AS:0 CC:0 PM:0 EA:3
[   80.893647] Krnl GPRS: 00000000000000bd 0000000000000000 000000000000005e 0000000000000001
[   80.893651]            0000000000589daa 00000000005a1248 0000000000000000 0000000000000000
[   80.893654]            0000000000000000 0000000000000001 000000007865c000 000000007e6cbbd8
[   80.893657]            000000007c704398 00000000ffffffef 00000000002b6768 000000007e6cbb00
[   80.893668] Krnl Code: 00000000002b675c: c0200020041d        larl    %r2,6b6f96
[   80.893672]            00000000002b6762: c0e500169afb        brasl   %r14,589d58
[   80.893676]            00000000002b6768: a7f40001            brc     15,2b676a
[   80.893680]           >00000000002b676c: b904002a            lgr     %r2,%r10
[   80.893684]            00000000002b6770: c0e5fffb6236        brasl   %r14,222bdc
[   80.893687]            00000000002b6776: a7f4ffad            brc     15,2b66d0
[   80.893692]            00000000002b677a: e320c0480004        lg      %r2,72(%r12)
[   80.893695]            00000000002b6780: a7f4ffec            brc     15,2b6758
[   80.893699] Call Trace:
[   80.893701] ([<00000000002b6768>] sysfs_add_one+0xcc/0xe8)
[   80.893705]  [<00000000002b7046>] sysfs_do_create_link+0xda/0x268
[   80.893708]  [<0000000000409f26>] driver_sysfs_add+0x66/0xcc
[   80.893713]  [<000000000040a0a2>] device_bind_driver+0x26/0x48
[   80.893717]  [<000000000040a110>] device_attach+0x4c/0xd4
[   80.893720]  [<0000000000409734>] bus_probe_device+0x4c/0x5c
[   80.893724]  [<0000000000406c52>] device_add+0x61e/0x73c
[   80.893728]  [<00000000004631a6>] ccw_device_todo+0x31a/0x380
[   80.893733]  [<000000000015f5b6>] process_one_work+0x1f6/0x4f0
[   80.893739]  [<0000000000163358>] worker_thread+0x17c/0x370
[   80.893742]  [<00000000001690ca>] kthread+0xa6/0xb0
[   80.893747]  [<000000000058f5f2>] kernel_thread_starter+0x6/0xc
[   80.893752]  [<000000000058f5ec>] kernel_thread_starter+0x0/0xc
[   80.893756] 4 locks held by kworker/u:1/25:
[   80.893758]  #0:  (cio){++++.+}, at: [<000000000015f524>] process_one_work+0x164/0x4f0
[   80.893766]  #1:  ((&cdev->private->todo_work)){+.+.+.}, at: [<000000000015f524>] process_one_work+0x164/0x4f0
[   80.893773]  #2:  (&__lockdep_no_validate__){+.+.+.}, at: [<000000000040a0fc>] device_attach+0x38/0xd4
[   80.893780]  #3:  (sysfs_mutex){+.+.+.}, at: [<00000000002b7030>] sysfs_do_create_link+0xc4/0x268
[   80.893787] Last Breaking-Event-Address:
[   80.893790]  [<00000000002b6768>] sysfs_add_one+0xcc/0xe8
[   80.893795] ---[ end trace ded2f91fcf2c6165 ]---

* device_add attached the device to the bus /*break*/
* driver_register walks the list of devices and tries to bind unbound devices
* /*continue*/ device_add calls device_attach which gets confused that the device is already bound to a driver

to fix this we could:
* hold the device lock in device_add (from bus_add_device to bus_probe_device) ..but this means we have to extract the lock from the inside of some of these functions and i'm not sure about holding the lock while the blocking_notifier_call_chain thing..

* add a bus mutex, preventing concurrent registrations of devices and drivers to a bus

* change device_attach to detect an already bound device..something like the following:

regards,
Sebastian
-----

Add a check to device_attach to identify an already bound device.

Signed-off-by Sebastian Ott <[email protected]>
---
 drivers/base/dd.c |    5 +++++
 1 file changed, 5 insertions(+)
drivers/base/dd.c +5 -0
@@ -245,6 +245,10 @@ int device_attach(struct device *dev)
 
 	device_lock(dev);
 	if (dev->driver) {
+		if (klist_node_attached(&dev->p->knode_driver)) {
+			ret = 1;
+			goto out_unlock;
+		}
 		ret = device_bind_driver(dev);
 		if (ret == 0)
 			ret = 1;

@@ -257,6 +261,7 @@ int device_attach(struct device *dev)
 		ret = bus_for_each_drv(dev->bus, NULL, dev, __device_attach);
 		pm_runtime_put_sync(dev);
 	}
+out_unlock:
 	device_unlock(dev);
 	return ret;
 }
#1 lore raw
Quoted 6 lines · Sebastian Ott

Hi,

i've seen this warning which looks to be caused by a race between device_add and driver_register

[   80.893594] sysfs: cannot create duplicate filename '/bus/ccw/drivers/qeth/0.0.b57d'

Isn't the problem here the fact that you are creating 2 directories of the same name?

Quoted 44 lines
[   80.893611] ------------[ cut here ]------------
[   80.893614] WARNING: at /home/autobuild/BUILD/linux-2.6.38.2-20110404/fs/sysfs/dir.c:455
[   80.893617] Modules linked in: qeth ccwgroup
[   80.893623] Modules linked in: qeth ccwgroup
[   80.893629] CPU: 1 Not tainted 2.6.38.2-48.x.20110404-s390xdefault #1
[   80.893632] Process kworker/u:1 (pid: 25, task: 000000007e6c5a40, ksp: 000000007e6cb980)
[   80.893635] Krnl PSW : 0704000180000000 00000000002b676c (sysfs_add_one+0xd0/0xe8)
[   80.893643]            R:0 T:1 IO:1 EX:1 Key:0 M:1 W:0 P:0 AS:0 CC:0 PM:0 EA:3
[   80.893647] Krnl GPRS: 00000000000000bd 0000000000000000 000000000000005e 0000000000000001
[   80.893651]            0000000000589daa 00000000005a1248 0000000000000000 0000000000000000
[   80.893654]            0000000000000000 0000000000000001 000000007865c000 000000007e6cbbd8
[   80.893657]            000000007c704398 00000000ffffffef 00000000002b6768 000000007e6cbb00
[   80.893668] Krnl Code: 00000000002b675c: c0200020041d        larl    %r2,6b6f96
[   80.893672]            00000000002b6762: c0e500169afb        brasl   %r14,589d58
[   80.893676]            00000000002b6768: a7f40001            brc     15,2b676a
[   80.893680]           >00000000002b676c: b904002a            lgr     %r2,%r10
[   80.893684]            00000000002b6770: c0e5fffb6236        brasl   %r14,222bdc
[   80.893687]            00000000002b6776: a7f4ffad            brc     15,2b66d0
[   80.893692]            00000000002b677a: e320c0480004        lg      %r2,72(%r12)
[   80.893695]            00000000002b6780: a7f4ffec            brc     15,2b6758
[   80.893699] Call Trace:
[   80.893701] ([<00000000002b6768>] sysfs_add_one+0xcc/0xe8)
[   80.893705]  [<00000000002b7046>] sysfs_do_create_link+0xda/0x268
[   80.893708]  [<0000000000409f26>] driver_sysfs_add+0x66/0xcc
[   80.893713]  [<000000000040a0a2>] device_bind_driver+0x26/0x48
[   80.893717]  [<000000000040a110>] device_attach+0x4c/0xd4
[   80.893720]  [<0000000000409734>] bus_probe_device+0x4c/0x5c
[   80.893724]  [<0000000000406c52>] device_add+0x61e/0x73c
[   80.893728]  [<00000000004631a6>] ccw_device_todo+0x31a/0x380
[   80.893733]  [<000000000015f5b6>] process_one_work+0x1f6/0x4f0
[   80.893739]  [<0000000000163358>] worker_thread+0x17c/0x370
[   80.893742]  [<00000000001690ca>] kthread+0xa6/0xb0
[   80.893747]  [<000000000058f5f2>] kernel_thread_starter+0x6/0xc
[   80.893752]  [<000000000058f5ec>] kernel_thread_starter+0x0/0xc
[   80.893756] 4 locks held by kworker/u:1/25:
[   80.893758]  #0:  (cio){++++.+}, at: [<000000000015f524>] process_one_work+0x164/0x4f0
[   80.893766]  #1:  ((&cdev->private->todo_work)){+.+.+.}, at: [<000000000015f524>] process_one_work+0x164/0x4f0
[   80.893773]  #2:  (&__lockdep_no_validate__){+.+.+.}, at: [<000000000040a0fc>] device_attach+0x38/0xd4
[   80.893780]  #3:  (sysfs_mutex){+.+.+.}, at: [<00000000002b7030>] sysfs_do_create_link+0xc4/0x268
[   80.893787] Last Breaking-Event-Address:
[   80.893790]  [<00000000002b6768>] sysfs_add_one+0xcc/0xe8
[   80.893795] ---[ end trace ded2f91fcf2c6165 ]---
Quoted 5 lines

* device_add attached the device to the bus /*break*/
* driver_register walks the list of devices and tries to bind unbound devices
* /*continue*/ device_add calls device_attach which gets confused that the device is already bound to a driver

Why would your bus code ever allow this to happen? It's the caller's responsiblity to do things in the correct order, right?

Quoted 11 lines

to fix this we could:
* hold the device lock in device_add (from bus_add_device to bus_probe_device) ..but this means we have to extract the lock from the inside of some of these functions and i'm not sure about holding the lock while the blocking_notifier_call_chain thing..

* add a bus mutex, preventing concurrent registrations of devices and drivers to a bus

* change device_attach to detect an already bound device..something like the following:

How about fix your caller code?

thanks,

greg k-h

author
#2 lore raw
Quoted 10 lines · Greg KH
Quoted 6 lines · Sebastian Ott

Hi,

i've seen this warning which looks to be caused by a race between device_add and driver_register

[   80.893594] sysfs: cannot create duplicate filename '/bus/ccw/drivers/qeth/0.0.b57d'

Isn't the problem here the fact that you are creating 2 directories of the same name?

I'm sure this isn't the case here. The bus code just calls device_add and at the same time on a different thread a module is loaded which registers a driver at the bus.

I was able to reproduce this with a module which creates a dummy bus and registers drivers and devices on this bus on 2 different workqueues.

Quoted 46 lines
Quoted 46 lines

[   80.893611] ------------[ cut here ]------------
[   80.893614] WARNING: at /home/autobuild/BUILD/linux-2.6.38.2-20110404/fs/sysfs/dir.c:455
[   80.893617] Modules linked in: qeth ccwgroup
[   80.893623] Modules linked in: qeth ccwgroup
[   80.893629] CPU: 1 Not tainted 2.6.38.2-48.x.20110404-s390xdefault #1
[   80.893632] Process kworker/u:1 (pid: 25, task: 000000007e6c5a40, ksp: 000000007e6cb980)
[   80.893635] Krnl PSW : 0704000180000000 00000000002b676c (sysfs_add_one+0xd0/0xe8)
[   80.893643]            R:0 T:1 IO:1 EX:1 Key:0 M:1 W:0 P:0 AS:0 CC:0 PM:0 EA:3
[   80.893647] Krnl GPRS: 00000000000000bd 0000000000000000 000000000000005e 0000000000000001
[   80.893651]            0000000000589daa 00000000005a1248 0000000000000000 0000000000000000
[   80.893654]            0000000000000000 0000000000000001 000000007865c000 000000007e6cbbd8
[   80.893657]            000000007c704398 00000000ffffffef 00000000002b6768 000000007e6cbb00
[   80.893668] Krnl Code: 00000000002b675c: c0200020041d        larl    %r2,6b6f96
[   80.893672]            00000000002b6762: c0e500169afb        brasl   %r14,589d58
[   80.893676]            00000000002b6768: a7f40001            brc     15,2b676a
[   80.893680]           >00000000002b676c: b904002a            lgr     %r2,%r10
[   80.893684]            00000000002b6770: c0e5fffb6236        brasl   %r14,222bdc
[   80.893687]            00000000002b6776: a7f4ffad            brc     15,2b66d0
[   80.893692]            00000000002b677a: e320c0480004        lg      %r2,72(%r12)
[   80.893695]            00000000002b6780: a7f4ffec            brc     15,2b6758
[   80.893699] Call Trace:
[   80.893701] ([<00000000002b6768>] sysfs_add_one+0xcc/0xe8)
[   80.893705]  [<00000000002b7046>] sysfs_do_create_link+0xda/0x268
[   80.893708]  [<0000000000409f26>] driver_sysfs_add+0x66/0xcc
[   80.893713]  [<000000000040a0a2>] device_bind_driver+0x26/0x48
[   80.893717]  [<000000000040a110>] device_attach+0x4c/0xd4
[   80.893720]  [<0000000000409734>] bus_probe_device+0x4c/0x5c
[   80.893724]  [<0000000000406c52>] device_add+0x61e/0x73c
[   80.893728]  [<00000000004631a6>] ccw_device_todo+0x31a/0x380
[   80.893733]  [<000000000015f5b6>] process_one_work+0x1f6/0x4f0
[   80.893739]  [<0000000000163358>] worker_thread+0x17c/0x370
[   80.893742]  [<00000000001690ca>] kthread+0xa6/0xb0
[   80.893747]  [<000000000058f5f2>] kernel_thread_starter+0x6/0xc
[   80.893752]  [<000000000058f5ec>] kernel_thread_starter+0x0/0xc
[   80.893756] 4 locks held by kworker/u:1/25:
[   80.893758]  #0:  (cio){++++.+}, at: [<000000000015f524>] process_one_work+0x164/0x4f0
[   80.893766]  #1:  ((&cdev->private->todo_work)){+.+.+.}, at: [<000000000015f524>] process_one_work+0x164/0x4f0
[   80.893773]  #2:  (&__lockdep_no_validate__){+.+.+.}, at: [<000000000040a0fc>] device_attach+0x38/0xd4
[   80.893780]  #3:  (sysfs_mutex){+.+.+.}, at: [<00000000002b7030>] sysfs_do_create_link+0xc4/0x268
[   80.893787] Last Breaking-Event-Address:
[   80.893790]  [<00000000002b6768>] sysfs_add_one+0xcc/0xe8
[   80.893795] ---[ end trace ded2f91fcf2c6165 ]---
Quoted 8 lines
Quoted 5 lines

* device_add attached the device to the bus /*break*/
* driver_register walks the list of devices and tries to bind unbound devices
* /*continue*/ device_add calls device_attach which gets confused that the device is already bound to a driver

Why would your bus code ever allow this to happen? It's the caller's responsiblity to do things in the correct order, right?

I don't think the bus code which calls device_register can (or should) prevent drivers from beeing registered at this bus at the same time.

Quoted 19 lines
Quoted 7 lines

to fix this we could:
* hold the device lock in device_add (from bus_add_device to bus_probe_device) ..but this means we have to extract the lock from the inside of some of these functions and i'm not sure about holding the lock while the blocking_notifier_call_chain thing..

Quoted 5 lines

* add a bus mutex, preventing concurrent registrations of devices and drivers to a bus

* change device_attach to detect an already bound device..something like the following:

How about fix your caller code?

thanks,

greg k-h

#3 lore raw
Quoted 10 lines · Sebastian Ott
Quoted 8 lines · Greg KH
Quoted 6 lines · Sebastian Ott

Hi,

i've seen this warning which looks to be caused by a race between device_add and driver_register

[   80.893594] sysfs: cannot create duplicate filename '/bus/ccw/drivers/qeth/0.0.b57d'
Quoted 8 lines
Quoted 2 lines

Isn't the problem here the fact that you are creating 2 directories of the same name?

I'm sure this isn't the case here. The bus code just calls device_add and at the same time on a different thread a module is loaded which registers a driver at the bus.

I was able to reproduce this with a module which creates a dummy bus and registers drivers and devices on this bus on 2 different workqueues.

That makes sense, as no bus should be doing this on multiple "threads". What real-life bus does this today?

Quoted 50 lines
Quoted 50 lines
Quoted 49 lines
[   80.893611] ------------[ cut here ]------------
[   80.893614] WARNING: at /home/autobuild/BUILD/linux-2.6.38.2-20110404/fs/sysfs/dir.c:455
[   80.893617] Modules linked in: qeth ccwgroup
[   80.893623] Modules linked in: qeth ccwgroup
[   80.893629] CPU: 1 Not tainted 2.6.38.2-48.x.20110404-s390xdefault #1
[   80.893632] Process kworker/u:1 (pid: 25, task: 000000007e6c5a40, ksp: 000000007e6cb980)
[   80.893635] Krnl PSW : 0704000180000000 00000000002b676c (sysfs_add_one+0xd0/0xe8)
[   80.893643]            R:0 T:1 IO:1 EX:1 Key:0 M:1 W:0 P:0 AS:0 CC:0 PM:0 EA:3
[   80.893647] Krnl GPRS: 00000000000000bd 0000000000000000 000000000000005e 0000000000000001
[   80.893651]            0000000000589daa 00000000005a1248 0000000000000000 0000000000000000
[   80.893654]            0000000000000000 0000000000000001 000000007865c000 000000007e6cbbd8
[   80.893657]            000000007c704398 00000000ffffffef 00000000002b6768 000000007e6cbb00
[   80.893668] Krnl Code: 00000000002b675c: c0200020041d        larl    %r2,6b6f96
[   80.893672]            00000000002b6762: c0e500169afb        brasl   %r14,589d58
[   80.893676]            00000000002b6768: a7f40001            brc     15,2b676a
[   80.893680]           >00000000002b676c: b904002a            lgr     %r2,%r10
[   80.893684]            00000000002b6770: c0e5fffb6236        brasl   %r14,222bdc
[   80.893687]            00000000002b6776: a7f4ffad            brc     15,2b66d0
[   80.893692]            00000000002b677a: e320c0480004        lg      %r2,72(%r12)
[   80.893695]            00000000002b6780: a7f4ffec            brc     15,2b6758
[   80.893699] Call Trace:
[   80.893701] ([<00000000002b6768>] sysfs_add_one+0xcc/0xe8)
[   80.893705]  [<00000000002b7046>] sysfs_do_create_link+0xda/0x268
[   80.893708]  [<0000000000409f26>] driver_sysfs_add+0x66/0xcc
[   80.893713]  [<000000000040a0a2>] device_bind_driver+0x26/0x48
[   80.893717]  [<000000000040a110>] device_attach+0x4c/0xd4
[   80.893720]  [<0000000000409734>] bus_probe_device+0x4c/0x5c
[   80.893724]  [<0000000000406c52>] device_add+0x61e/0x73c
[   80.893728]  [<00000000004631a6>] ccw_device_todo+0x31a/0x380
[   80.893733]  [<000000000015f5b6>] process_one_work+0x1f6/0x4f0
[   80.893739]  [<0000000000163358>] worker_thread+0x17c/0x370
[   80.893742]  [<00000000001690ca>] kthread+0xa6/0xb0
[   80.893747]  [<000000000058f5f2>] kernel_thread_starter+0x6/0xc
[   80.893752]  [<000000000058f5ec>] kernel_thread_starter+0x0/0xc
[   80.893756] 4 locks held by kworker/u:1/25:
[   80.893758]  #0:  (cio){++++.+}, at: [<000000000015f524>] process_one_work+0x164/0x4f0
[   80.893766]  #1:  ((&cdev->private->todo_work)){+.+.+.}, at: [<000000000015f524>] process_one_work+0x164/0x4f0
[   80.893773]  #2:  (&__lockdep_no_validate__){+.+.+.}, at: [<000000000040a0fc>] device_attach+0x38/0xd4
[   80.893780]  #3:  (sysfs_mutex){+.+.+.}, at: [<00000000002b7030>] sysfs_do_create_link+0xc4/0x268
[   80.893787] Last Breaking-Event-Address:
[   80.893790]  [<00000000002b6768>] sysfs_add_one+0xcc/0xe8
[   80.893795] ---[ end trace ded2f91fcf2c6165 ]---

* device_add attached the device to the bus /*break*/
* driver_register walks the list of devices and tries to bind unbound devices
* /*continue*/ device_add calls device_attach which gets confused that the device is already bound to a driver

Quoted 4 lines
Quoted 2 lines

Why would your bus code ever allow this to happen? It's the caller's responsiblity to do things in the correct order, right?

I don't think the bus code which calls device_register can (or should) prevent drivers from beeing registered at this bus at the same time.

Why not? That's the way all kernel subsystems work today that I know of. Has this changed?

thanks,

greg k-h

author
#4 lore raw
Quoted 17 lines · Greg KH
Quoted 16 lines · Sebastian Ott
Quoted 10 lines · Greg KH
Quoted 6 lines · Sebastian Ott

Hi,

i've seen this warning which looks to be caused by a race between device_add and driver_register

[   80.893594] sysfs: cannot create duplicate filename '/bus/ccw/drivers/qeth/0.0.b57d'

Isn't the problem here the fact that you are creating 2 directories of the same name?

I'm sure this isn't the case here. The bus code just calls device_add and at the same time on a different thread a module is loaded which registers a driver at the bus.

Quoted 5 lines
Quoted 2 lines

I was able to reproduce this with a module which creates a dummy bus and registers drivers and devices on this bus on 2 different workqueues.

That makes sense, as no bus should be doing this on multiple "threads". What real-life bus does this today?

A bus that will recognize and register a lot of devices, after the first uevent is presented to userspace, a module will be loaded registering a driver from a different thread. I don't think thats uncommon.

Quoted 51 lines
Quoted 51 lines
Quoted 51 lines
Quoted 50 lines
[   80.893611] ------------[ cut here ]------------
[   80.893614] WARNING: at /home/autobuild/BUILD/linux-2.6.38.2-20110404/fs/sysfs/dir.c:455
[   80.893617] Modules linked in: qeth ccwgroup
[   80.893623] Modules linked in: qeth ccwgroup
[   80.893629] CPU: 1 Not tainted 2.6.38.2-48.x.20110404-s390xdefault #1
[   80.893632] Process kworker/u:1 (pid: 25, task: 000000007e6c5a40, ksp: 000000007e6cb980)
[   80.893635] Krnl PSW : 0704000180000000 00000000002b676c (sysfs_add_one+0xd0/0xe8)
[   80.893643]            R:0 T:1 IO:1 EX:1 Key:0 M:1 W:0 P:0 AS:0 CC:0 PM:0 EA:3
[   80.893647] Krnl GPRS: 00000000000000bd 0000000000000000 000000000000005e 0000000000000001
[   80.893651]            0000000000589daa 00000000005a1248 0000000000000000 0000000000000000
[   80.893654]            0000000000000000 0000000000000001 000000007865c000 000000007e6cbbd8
[   80.893657]            000000007c704398 00000000ffffffef 00000000002b6768 000000007e6cbb00
[   80.893668] Krnl Code: 00000000002b675c: c0200020041d        larl    %r2,6b6f96
[   80.893672]            00000000002b6762: c0e500169afb        brasl   %r14,589d58
[   80.893676]            00000000002b6768: a7f40001            brc     15,2b676a
[   80.893680]           >00000000002b676c: b904002a            lgr     %r2,%r10
[   80.893684]            00000000002b6770: c0e5fffb6236        brasl   %r14,222bdc
[   80.893687]            00000000002b6776: a7f4ffad            brc     15,2b66d0
[   80.893692]            00000000002b677a: e320c0480004        lg      %r2,72(%r12)
[   80.893695]            00000000002b6780: a7f4ffec            brc     15,2b6758
[   80.893699] Call Trace:
[   80.893701] ([<00000000002b6768>] sysfs_add_one+0xcc/0xe8)
[   80.893705]  [<00000000002b7046>] sysfs_do_create_link+0xda/0x268
[   80.893708]  [<0000000000409f26>] driver_sysfs_add+0x66/0xcc
[   80.893713]  [<000000000040a0a2>] device_bind_driver+0x26/0x48
[   80.893717]  [<000000000040a110>] device_attach+0x4c/0xd4
[   80.893720]  [<0000000000409734>] bus_probe_device+0x4c/0x5c
[   80.893724]  [<0000000000406c52>] device_add+0x61e/0x73c
[   80.893728]  [<00000000004631a6>] ccw_device_todo+0x31a/0x380
[   80.893733]  [<000000000015f5b6>] process_one_work+0x1f6/0x4f0
[   80.893739]  [<0000000000163358>] worker_thread+0x17c/0x370
[   80.893742]  [<00000000001690ca>] kthread+0xa6/0xb0
[   80.893747]  [<000000000058f5f2>] kernel_thread_starter+0x6/0xc
[   80.893752]  [<000000000058f5ec>] kernel_thread_starter+0x0/0xc
[   80.893756] 4 locks held by kworker/u:1/25:
[   80.893758]  #0:  (cio){++++.+}, at: [<000000000015f524>] process_one_work+0x164/0x4f0
[   80.893766]  #1:  ((&cdev->private->todo_work)){+.+.+.}, at: [<000000000015f524>] process_one_work+0x164/0x4f0
[   80.893773]  #2:  (&__lockdep_no_validate__){+.+.+.}, at: [<000000000040a0fc>] device_attach+0x38/0xd4
[   80.893780]  #3:  (sysfs_mutex){+.+.+.}, at: [<00000000002b7030>] sysfs_do_create_link+0xc4/0x268
[   80.893787] Last Breaking-Event-Address:
[   80.893790]  [<00000000002b6768>] sysfs_add_one+0xcc/0xe8
[   80.893795] ---[ end trace ded2f91fcf2c6165 ]---

* device_add attached the device to the bus /*break*/
* driver_register walks the list of devices and tries to bind unbound devices
* /*continue*/ device_add calls device_attach which gets confused that the device is already bound to a driver

Quoted 7 lines
Quoted 4 lines
Quoted 2 lines

Why would your bus code ever allow this to happen? It's the caller's responsiblity to do things in the correct order, right?

I don't think the bus code which calls device_register can (or should) prevent drivers from beeing registered at this bus at the same time.

Why not? That's the way all kernel subsystems work today that I know of. Has this changed?

What about an exported bus_type? At all time a driver for this bus can be registered, the bus code has no chance to prevent or serialize this.

Quoted 5 lines

thanks,

greg k-h

#5 lore raw
Quoted 21 lines · Sebastian Ott
Quoted 17 lines · Greg KH
Quoted 16 lines · Sebastian Ott
Quoted 10 lines · Greg KH
Quoted 6 lines · Sebastian Ott

Hi,

i've seen this warning which looks to be caused by a race between device_add and driver_register

[   80.893594] sysfs: cannot create duplicate filename '/bus/ccw/drivers/qeth/0.0.b57d'

Isn't the problem here the fact that you are creating 2 directories of the same name?

I'm sure this isn't the case here. The bus code just calls device_add and at the same time on a different thread a module is loaded which registers a driver at the bus.

Quoted 8 lines
Quoted 5 lines
Quoted 2 lines

I was able to reproduce this with a module which creates a dummy bus and registers drivers and devices on this bus on 2 different workqueues.

That makes sense, as no bus should be doing this on multiple "threads". What real-life bus does this today?

A bus that will recognize and register a lot of devices, after the first uevent is presented to userspace, a module will be loaded registering a driver from a different thread. I don't think thats uncommon.

But again, what kernel code today does this? I think they all have locks to keep this from happening, right?

Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
Quoted 5 lines

* device_add attached the device to the bus /*break*/
* driver_register walks the list of devices and tries to bind unbound devices
* /*continue*/ device_add calls device_attach which gets confused that the device is already bound to a driver

Why would your bus code ever allow this to happen? It's the caller's responsiblity to do things in the correct order, right?

Quoted 7 lines
Quoted 5 lines
Quoted 2 lines

I don't think the bus code which calls device_register can (or should) prevent drivers from beeing registered at this bus at the same time.

Why not? That's the way all kernel subsystems work today that I know of. Has this changed?

What about an exported bus_type? At all time a driver for this bus can be registered, the bus code has no chance to prevent or serialize this.

No, the bus core is the one that should be binding the bus type to the driver and doing the registering. No individual driver should ever be messing with a bus_type at all.

Now perhaps platform devices are, and if so, we might want to resolve this, but no "real" bus should ever be doing this.

thanks,

greg k-h

author
#6 lore raw
Quoted 25 lines · Greg KH
Quoted 24 lines · Sebastian Ott
Quoted 20 lines · Greg KH
Quoted 18 lines · Sebastian Ott
Quoted 10 lines · Greg KH
Quoted 6 lines · Sebastian Ott

Hi,

i've seen this warning which looks to be caused by a race between device_add and driver_register

[   80.893594] sysfs: cannot create duplicate filename '/bus/ccw/drivers/qeth/0.0.b57d'

Isn't the problem here the fact that you are creating 2 directories of the same name?

I'm sure this isn't the case here. The bus code just calls device_add and at the same time on a different thread a module is loaded which registers a driver at the bus.

I was able to reproduce this with a module which creates a dummy bus and registers drivers and devices on this bus on 2 different workqueues.

Quoted 8 lines
Quoted 5 lines
Quoted 2 lines

That makes sense, as no bus should be doing this on multiple "threads". What real-life bus does this today?

A bus that will recognize and register a lot of devices, after the first uevent is presented to userspace, a module will be loaded registering a driver from a different thread. I don't think thats uncommon.

But again, what kernel code today does this? I think they all have locks to keep this from happening, right?

I couldn't find a bus who protects device_register against driver_register and I don't think this is something which should be handled by every individual bus but from within the driver core.

Quoted 28 lines
Quoted 9 lines
Quoted 9 lines
Quoted 9 lines
Quoted 9 lines
Quoted 6 lines

* device_add attached the device to the bus /*break*/
* driver_register walks the list of devices and tries to bind unbound devices
* /*continue*/ device_add calls device_attach which gets confused that the device is already bound to a driver

Why would your bus code ever allow this to happen? It's the caller's responsiblity to do things in the correct order, right?

Quoted 7 lines
Quoted 5 lines
Quoted 2 lines

I don't think the bus code which calls device_register can (or should) prevent drivers from beeing registered at this bus at the same time.

Why not? That's the way all kernel subsystems work today that I know of. Has this changed?

What about an exported bus_type? At all time a driver for this bus can be registered, the bus code has no chance to prevent or serialize this.

No, the bus core is the one that should be binding the bus type to the driver and doing the registering. No individual driver should ever be messing with a bus_type at all.

Now perhaps platform devices are, and if so, we might want to resolve this, but no "real" bus should ever be doing this.

thanks,

greg k-h

#7 lore raw
Quoted 35 lines · Sebastian Ott
Quoted 31 lines · Greg KH
Quoted 29 lines · Sebastian Ott
Quoted 17 lines · Greg KH
Quoted 16 lines · Sebastian Ott
Quoted 10 lines · Greg KH
Quoted 6 lines · Sebastian Ott

Hi,

i've seen this warning which looks to be caused by a race between device_add and driver_register

[   80.893594] sysfs: cannot create duplicate filename '/bus/ccw/drivers/qeth/0.0.b57d'

Isn't the problem here the fact that you are creating 2 directories of the same name?

I'm sure this isn't the case here. The bus code just calls device_add and at the same time on a different thread a module is loaded which registers a driver at the bus.

Quoted 5 lines
Quoted 2 lines

I was able to reproduce this with a module which creates a dummy bus and registers drivers and devices on this bus on 2 different workqueues.

That makes sense, as no bus should be doing this on multiple "threads". What real-life bus does this today?

A bus that will recognize and register a lot of devices, after the first uevent is presented to userspace, a module will be loaded registering a driver from a different thread. I don't think thats uncommon.

Quoted 5 lines
Quoted 2 lines

But again, what kernel code today does this? I think they all have locks to keep this from happening, right?

I couldn't find a bus who protects device_register against driver_register and I don't think this is something which should be handled by every individual bus but from within the driver core.

How did you cause the oops in this original message? What type of bus was it on? And did your patch solve the issue?

thanks,

greg k-h

author
#8 lore raw
Quoted 36 lines · Greg KH
Quoted 35 lines · Sebastian Ott
Quoted 31 lines · Greg KH
Quoted 29 lines · Sebastian Ott
Quoted 17 lines · Greg KH
Quoted 16 lines · Sebastian Ott
Quoted 10 lines · Greg KH
Quoted 6 lines · Sebastian Ott

Hi,

i've seen this warning which looks to be caused by a race between device_add and driver_register

[   80.893594] sysfs: cannot create duplicate filename '/bus/ccw/drivers/qeth/0.0.b57d'

Isn't the problem here the fact that you are creating 2 directories of the same name?

I'm sure this isn't the case here. The bus code just calls device_add and at the same time on a different thread a module is loaded which registers a driver at the bus.

Quoted 5 lines
Quoted 2 lines

I was able to reproduce this with a module which creates a dummy bus and registers drivers and devices on this bus on 2 different workqueues.

That makes sense, as no bus should be doing this on multiple "threads". What real-life bus does this today?

A bus that will recognize and register a lot of devices, after the first uevent is presented to userspace, a module will be loaded registering a driver from a different thread. I don't think thats uncommon.

Quoted 8 lines
Quoted 5 lines
Quoted 2 lines

But again, what kernel code today does this? I think they all have locks to keep this from happening, right?

I couldn't find a bus who protects device_register against driver_register and I don't think this is something which should be handled by every individual bus but from within the driver core.

How did you cause the oops in this original message? What type of bus was it on? And did your patch solve the issue?

I've seen this warning on a system which had a lot of devices attached. Those devices also responded _very_ slow. So it took some time to register all these deivces (qeth) on the bus (ccw). During this time the qeth module got loaded and registered the qeth driver.

Since i've never seen this warning and those devices normally don't take so much time to respond I wrote the beforementioned module to reproduce the race and make sure that this is no driver issue.

With this module the warning could be triggered after a few cycles. With the patch applied, I did not see the warning again even after

Quoted 1 line

10.000 cycles.

regards,
Sebastian

#9 lore raw
Quoted 54 lines · Sebastian Ott
Quoted 36 lines · Greg KH
Quoted 35 lines · Sebastian Ott
Quoted 31 lines · Greg KH
Quoted 29 lines · Sebastian Ott
Quoted 17 lines · Greg KH
Quoted 16 lines · Sebastian Ott
Quoted 10 lines · Greg KH
Quoted 6 lines · Sebastian Ott

Hi,

i've seen this warning which looks to be caused by a race between device_add and driver_register

[   80.893594] sysfs: cannot create duplicate filename '/bus/ccw/drivers/qeth/0.0.b57d'

Isn't the problem here the fact that you are creating 2 directories of the same name?

I'm sure this isn't the case here. The bus code just calls device_add and at the same time on a different thread a module is loaded which registers a driver at the bus.

Quoted 5 lines
Quoted 2 lines

I was able to reproduce this with a module which creates a dummy bus and registers drivers and devices on this bus on 2 different workqueues.

That makes sense, as no bus should be doing this on multiple "threads". What real-life bus does this today?

A bus that will recognize and register a lot of devices, after the first uevent is presented to userspace, a module will be loaded registering a driver from a different thread. I don't think thats uncommon.

Quoted 8 lines
Quoted 5 lines
Quoted 2 lines

But again, what kernel code today does this? I think they all have locks to keep this from happening, right?

I couldn't find a bus who protects device_register against driver_register and I don't think this is something which should be handled by every individual bus but from within the driver core.

How did you cause the oops in this original message? What type of bus was it on? And did your patch solve the issue?

I've seen this warning on a system which had a lot of devices attached. Those devices also responded _very_ slow. So it took some time to register all these deivces (qeth) on the bus (ccw). During this time the qeth module got loaded and registered the qeth driver.

Quoted 7 lines

Since i've never seen this warning and those devices normally don't take so much time to respond I wrote the beforementioned module to reproduce the race and make sure that this is no driver issue.

With this module the warning could be triggered after a few cycles. With the patch applied, I did not see the warning again even after

Quoted 1 line

10.000 cycles.

Ok, fair enough. Care to resend the patch again so I can review it once more?

thanks,

greg k-h

author
#10 lore raw

PATCH 1/1 driver-core: fix race between device_register and driver_register

When a device is registered to a bus it will be a) added to the list of devices of the bus and b) bind to a driver (if one matches). As a result of a driver being registered at this bus between a) and b) this device could already be bound to a driver. This leads to a warning and incorrect refcounting.
To fix this add a check to device_attach to identify an already bound device.

Signed-off-by Sebastian Ott <[email protected]>
---
 drivers/base/dd.c |    5 +++++
 1 file changed, 5 insertions(+)
drivers/base/dd.c +5 -0
@@ -245,6 +245,10 @@ int device_attach(struct device *dev)
 
 	device_lock(dev);
 	if (dev->driver) {
+		if (klist_node_attached(&dev->p->knode_driver)) {
+			ret = 1;
+			goto out_unlock;
+		}
 		ret = device_bind_driver(dev);
 		if (ret == 0)
 			ret = 1;

@@ -257,6 +261,7 @@ int device_attach(struct device *dev)
 		ret = bus_for_each_drv(dev->bus, NULL, dev, __device_attach);
 		pm_runtime_put_sync(dev);
 	}
+out_unlock:
 	device_unlock(dev);
 	return ret;
 }
#11 lore raw

Re: [PATCH] driver-core: fix race between device_register and driver_register

Quoted 10 lines · Sebastian Ott

When a device is registered to a bus it will be a) added to the list of devices of the bus and b) bind to a driver (if one matches). As a result of a driver being registered at this bus between a) and b) this device could already be bound to a driver. This leads to a warning and incorrect refcounting.
To fix this add a check to device_attach to identify an already bound device.

Signed-off-by Sebastian Ott <[email protected]>

Thanks, I'll queue this up for .40.

greg k-h