All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] Fix a bdi reregistration race, v3
@ 2016-03-28 21:29 Bart Van Assche
  2016-05-05 19:58 ` Bart Van Assche
  0 siblings, 1 reply; 6+ messages in thread
From: Bart Van Assche @ 2016-03-28 21:29 UTC (permalink / raw)
  To: James Bottomley, Martin K. Petersen
  Cc: Christoph Hellwig, Hannes Reinecke, linux-scsi

Avoid that the sd driver registers a BDI device with a name that
is still in use. This patch avoids that the following warning gets
triggered:

WARNING: CPU: 7 PID: 203 at fs/sysfs/dir.c:31 sysfs_warn_dup+0x68/0x80()
sysfs: cannot create duplicate filename '/devices/virtual/bdi/8:32'
Workqueue: events_unbound async_run_entry_fn
Call Trace:
[<ffffffff814ff5a4>] dump_stack+0x4c/0x65
[<ffffffff810746ba>] warn_slowpath_common+0x8a/0xc0
[<ffffffff81074736>] warn_slowpath_fmt+0x46/0x50
[<ffffffff81237ca8>] sysfs_warn_dup+0x68/0x80
[<ffffffff81237d8e>] sysfs_create_dir_ns+0x7e/0x90
[<ffffffff81291f58>] kobject_add_internal+0xa8/0x320
[<ffffffff812923a0>] kobject_add+0x60/0xb0
[<ffffffff8138c937>] device_add+0x107/0x5e0
[<ffffffff8138d018>] device_create_groups_vargs+0xd8/0x100
[<ffffffff8138d05c>] device_create_vargs+0x1c/0x20
[<ffffffff8117f233>] bdi_register+0x63/0x2a0
[<ffffffff8117f497>] bdi_register_dev+0x27/0x30
[<ffffffff81281549>] add_disk+0x1a9/0x4e0
[<ffffffffa00c5739>] sd_probe_async+0x119/0x1d0 [sd_mod]
[<ffffffff8109a81a>] async_run_entry_fn+0x4a/0x140
[<ffffffff81091078>] process_one_work+0x1d8/0x7c0
[<ffffffff81091774>] worker_thread+0x114/0x460
[<ffffffff81097878>] kthread+0xf8/0x110
[<ffffffff8150801f>] ret_from_fork+0x3f/0x70

Signed-off-by: Bart Van Assche <bart.vanassche@sandisk.com>
Cc: Christoph Hellwig <hch@lst.de>
Cc: Hannes Reinecke <hare@suse.de>
Cc: <stable@vger.kernel.org>
---
 drivers/scsi/scsi_sysfs.c | 30 +++++++++++++++++++++++++++++-
 1 file changed, 29 insertions(+), 1 deletion(-)

diff --git a/drivers/scsi/scsi_sysfs.c b/drivers/scsi/scsi_sysfs.c
index 2b642b1..567f2a0 100644
--- a/drivers/scsi/scsi_sysfs.c
+++ b/drivers/scsi/scsi_sysfs.c
@@ -1272,9 +1272,26 @@ int scsi_sysfs_add_sdev(struct scsi_device *sdev)
 	return error;
 }
 
+static int scsi_return_dev(struct device *dev, void *data)
+{
+	struct device **childp = data;
+
+	*childp = dev;
+	return 0;
+}
+
+/* Caller must call put_device() if this function does not return NULL. */
+static struct device *scsi_get_child_dev(struct device *dev)
+{
+	struct device *child = NULL;
+
+	device_for_each_child(dev, &child, scsi_return_dev);
+	return get_device(child);
+}
+
 void __scsi_remove_device(struct scsi_device *sdev)
 {
-	struct device *dev = &sdev->sdev_gendev;
+	struct device *dev = &sdev->sdev_gendev, *sdkp = NULL;
 
 	/*
 	 * This cleanup path is not reentrant and while it is impossible
@@ -1289,6 +1306,7 @@ void __scsi_remove_device(struct scsi_device *sdev)
 			return;
 
 		bsg_unregister_queue(sdev->request_queue);
+		sdkp = scsi_get_child_dev(dev);
 		device_unregister(&sdev->sdev_dev);
 		transport_remove_device(dev);
 		scsi_dh_remove_device(sdev);
@@ -1305,6 +1323,16 @@ void __scsi_remove_device(struct scsi_device *sdev)
 	blk_cleanup_queue(sdev->request_queue);
 	cancel_work_sync(&sdev->requeue_work);
 
+	/*
+	 * blk_cleanup_queue() unregisters the BDI device. The name of the
+	 * BDI device is derived from the dev_t of the /dev/sd<n> device.
+	 * Keep a reference to the /dev/sd<n> device until the BDI device
+	 * has been unregistered to avoid that a BDI device with the same
+	 * name gets registered before blk_cleanup_queue() has finished.
+	 */
+	if (sdkp)
+		put_device(sdkp);
+
 	if (sdev->host->hostt->slave_destroy)
 		sdev->host->hostt->slave_destroy(sdev);
 	transport_destroy_device(dev);
-- 
2.7.3


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] Fix a bdi reregistration race, v3
  2016-03-28 21:29 [PATCH] Fix a bdi reregistration race, v3 Bart Van Assche
@ 2016-05-05 19:58 ` Bart Van Assche
  2016-05-05 20:40   ` Joe Lawrence
  0 siblings, 1 reply; 6+ messages in thread
From: Bart Van Assche @ 2016-05-05 19:58 UTC (permalink / raw)
  To: James Bottomley, Martin K. Petersen
  Cc: Christoph Hellwig, Hannes Reinecke, linux-scsi

On 03/28/2016 02:29 PM, Bart Van Assche wrote:
> Avoid that the sd driver registers a BDI device with a name that
> is still in use. This patch avoids that the following warning gets
> triggered:
>
> [ ... ]

(replying to my own e-mail)

If anyone could review this patch that would be very welcome.

Thanks,

Bart.


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] Fix a bdi reregistration race, v3
  2016-05-05 19:58 ` Bart Van Assche
@ 2016-05-05 20:40   ` Joe Lawrence
  2016-05-09 13:50     ` Joe Lawrence
  0 siblings, 1 reply; 6+ messages in thread
From: Joe Lawrence @ 2016-05-05 20:40 UTC (permalink / raw)
  To: Bart Van Assche, James Bottomley, Martin K. Petersen
  Cc: Christoph Hellwig, Hannes Reinecke, linux-scsi

On 05/05/2016 03:58 PM, Bart Van Assche wrote:
> On 03/28/2016 02:29 PM, Bart Van Assche wrote:
>> Avoid that the sd driver registers a BDI device with a name that
>> is still in use. This patch avoids that the following warning gets
>> triggered:
>>
>> [ ... ]
> 
> (replying to my own e-mail)
> 
> If anyone could review this patch that would be very welcome.

Hi Bart,

I *think* I may be hitting this same problem running some tests here at Stratus -- I've got slub_debugging and other kernel debugging turned on while repeatedly hotplugging controllers at various timings during probe.  It takes hours to hit, but I've seen it twice in the past week.  I've got some other testing on my plate at the moment, but when I get a chance I can give your v3 a spin.

Here's an example (sorry for the wide width) ...

------------[ cut here ]------------
WARNING: CPU: 5 PID: 30702 at fs/sysfs/dir.c:31 sysfs_warn_dup+0x64/0x80
sysfs: cannot create duplicate filename '/devices/virtual/bdi/65:0'
Modules linked in: btrfs xor raid6_pq msdos ext4 jbd2 mbcache matroxfb(OE) ccmod(POE) ftmod(OE) videosw(OE) ipmi_devintf fuse xt_CHECKSUM ipt_MASQUERADE nf_nat_masquerade_ipv4 tun ip6t_rpfilter ip6t_REJECT nf_reject_ipv6 ipt_REJECT nf_reject_ipv4 xt_conntrack bonding ebtable_nat ebtable_broute bridge stp llc ebtable_filter ebtables ip6table_nat nf_conntrack_ipv6 nf_defrag_ipv6 nf_nat_ipv6 ip6table_mangle ip6table_security ip6table_raw ip6table_filter ip6_tables iptable_nat nf_conntrack_ipv4 nf_defrag_ipv4 nf_nat_ipv4 nf_nat nf_conntrack iptable_mangle iptable_security iptable_raw iptable_filter vfat fat x86_pkg_temp_thermal intel_powerclamp coretemp crct10dif_pclmul crc32_pclmul ghash_clmulni_intel aesni_intel glue_helper lrw gf128mul ablk_helper cryptd dm_service_time pcspkr ses enclosure
  sg wmi nfsd auth_rpcgss nfs_acl lockd grace sunrpc dm_multipath ip_tables xfs libcrc32c raid1 sr_mod cdrom crc32c_intel qla2xxx(OE) scsi_transport_fc ixgbe(OE) mdio igb(OE) ptp pps_core mpt3sas(OE) 
raid_class i2c_algo_bit sra_sense(OE) i2c_core sd_mod(OE) scsi_transport_sas dca scsi_hbas(OE) fjes ipmi_msghandler usb_storage dm_mirror dm_region_hash dm_log dm_mod [last unloaded: ipmi_devintf]
CPU: 5 PID: 30702 Comm: kworker/u97:7 Tainted: P        W  OE   4.6.0-rc6+ #37
Hardware name: Stratus ftServer 6800/G7LYY, BIOS BIOS Version 8.1:64 02/03/2016
Workqueue: events_unbound async_run_entry_fn
 0000000000000286 00000000e136f6f8 ffff88038ae03a50 ffffffff8134359f
 ffff88038ae03aa0 0000000000000000 ffff88038ae03a90 ffffffff8108af31
 0000001f5bdb0398 ffff880067e0e7e8 ffff88058f92def0 ffff88084bd2c008
Call Trace:
 [<ffffffff8134359f>] dump_stack+0x63/0x84
 [<ffffffff8108af31>] __warn+0xd1/0xf0
 [<ffffffff8108afaf>] warn_slowpath_fmt+0x5f/0x80
 [<ffffffff81297604>] sysfs_warn_dup+0x64/0x80
 [<ffffffff812976ee>] sysfs_create_dir_ns+0x7e/0x90
 [<ffffffff8134653a>] kobject_add_internal+0xaa/0x320
 [<ffffffff8146bf93>] ? device_private_init+0x23/0x70
 [<ffffffff813469e5>] kobject_add+0x75/0xd0
 [<ffffffff816a9cd2>] ? mutex_lock+0x12/0x2f
 [<ffffffff8146c105>] device_add+0x125/0x610
 [<ffffffff8146c7e8>] device_create_groups_vargs+0xd8/0x100
 [<ffffffff8146c82c>] device_create_vargs+0x1c/0x20
 [<ffffffff811b209c>] bdi_register+0x8c/0x180
 [<ffffffff811b21b7>] bdi_register_dev+0x27/0x30
 [<ffffffff81327e7f>] add_disk+0x17f/0x4a0
 [<ffffffff8147b955>] ? update_autosuspend+0x55/0x60
 [<ffffffff8147ba14>] ? __pm_runtime_use_autosuspend+0x54/0x70
 [<ffffffffa00a1a25>] sd_probe_async+0x115/0x1d0 [sd_mod]
 [<ffffffff810ad80a>] async_run_entry_fn+0x4a/0x140
 [<ffffffff810a464e>] process_one_work+0x16e/0x420
 [<ffffffff810a4f45>] worker_thread+0x125/0x4b0
 [<ffffffff816a780d>] ? __schedule+0x2ad/0x8a0
 [<ffffffff810a4e20>] ? rescuer_thread+0x380/0x380
 [<ffffffff810aa9b8>] kthread+0xd8/0xf0
 [<ffffffff816abb42>] ret_from_fork+0x22/0x40
 [<ffffffff810aa8e0>] ? kthread_park+0x60/0x60
---[ end trace 353add4e78cb2a75 ]---
------------[ cut here ]------------
WARNING: CPU: 5 PID: 30702 at lib/kobject.c:240 kobject_add_internal+0x262/0x320
kobject_add_internal failed for 65:0 with -EEXIST, don't try to register things with the same name in the same directory.
Modules linked in: btrfs xor raid6_pq msdos ext4 jbd2 mbcache matroxfb(OE) ccmod(POE) ftmod(OE) videosw(OE) ipmi_devintf fuse xt_CHECKSUM ipt_MASQUERADE nf_nat_masquerade_ipv4 tun ip6t_rpfilter ip6t_REJECT nf_reject_ipv6 ipt_REJECT nf_reject_ipv4 xt_conntrack bonding ebtable_nat ebtable_broute bridge stp llc ebtable_filter ebtables ip6table_nat nf_conntrack_ipv6 nf_defrag_ipv6 nf_nat_ipv6 ip6table_mangle ip6table_security ip6table_raw ip6table_filter ip6_tables iptable_nat nf_conntrack_ipv4 nf_defrag_ipv4 nf_nat_ipv4 nf_nat nf_conntrack iptable_mangle iptable_security iptable_raw iptable_filter vfat fat x86_pkg_temp_thermal intel_powerclamp coretemp crct10dif_pclmul crc32_pclmul ghash_clmulni_intel aesni_intel glue_helper lrw gf128mul ablk_helper cryptd dm_service_time pcspkr ses enclosure
  sg wmi nfsd auth_rpcgss nfs_acl lockd grace sunrpc dm_multipath ip_tables xfs libcrc32c raid1 sr_mod cdrom crc32c_intel qla2xxx(OE) scsi_transport_fc ixgbe(OE) mdio igb(OE) ptp pps_core mpt3sas(OE) 
raid_class i2c_algo_bit sra_sense(OE) i2c_core sd_mod(OE) scsi_transport_sas dca scsi_hbas(OE) fjes ipmi_msghandler usb_storage dm_mirror dm_region_hash dm_log dm_mod [last unloaded: ipmi_devintf]
CPU: 5 PID: 30702 Comm: kworker/u97:7 Tainted: P        W  OE   4.6.0-rc6+ #37
Hardware name: Stratus ftServer 6800/G7LYY, BIOS BIOS Version 8.1:64 02/03/2016
Workqueue: events_unbound async_run_entry_fn
 0000000000000286 00000000e136f6f8 ffff88038ae03aa0 ffffffff8134359f
 ffff88038ae03af0 0000000000000000 ffff88038ae03ae0 ffffffff8108af31
 000000f08129760c ffff880853a4f4f8 ffff88003b529e68 00000000ffffffef
Call Trace:
 [<ffffffff8134359f>] dump_stack+0x63/0x84
 [<ffffffff8108af31>] __warn+0xd1/0xf0
 [<ffffffff8108afaf>] warn_slowpath_fmt+0x5f/0x80
 [<ffffffff813466f2>] kobject_add_internal+0x262/0x320
 [<ffffffff8146bf93>] ? device_private_init+0x23/0x70
 [<ffffffff813469e5>] kobject_add+0x75/0xd0
 [<ffffffff816a9cd2>] ? mutex_lock+0x12/0x2f
 [<ffffffff8146c105>] device_add+0x125/0x610
 [<ffffffff8146c7e8>] device_create_groups_vargs+0xd8/0x100
 [<ffffffff8146c82c>] device_create_vargs+0x1c/0x20
 [<ffffffff811b209c>] bdi_register+0x8c/0x180
 [<ffffffff811b21b7>] bdi_register_dev+0x27/0x30
 [<ffffffff81327e7f>] add_disk+0x17f/0x4a0
 [<ffffffff8147b955>] ? update_autosuspend+0x55/0x60
 [<ffffffff8147ba14>] ? __pm_runtime_use_autosuspend+0x54/0x70
 [<ffffffffa00a1a25>] sd_probe_async+0x115/0x1d0 [sd_mod]
 [<ffffffff810ad80a>] async_run_entry_fn+0x4a/0x140
 [<ffffffff810a464e>] process_one_work+0x16e/0x420
 [<ffffffff810a4f45>] worker_thread+0x125/0x4b0
 [<ffffffff816a780d>] ? __schedule+0x2ad/0x8a0
 [<ffffffff810a4e20>] ? rescuer_thread+0x380/0x380
 [<ffffffff810aa9b8>] kthread+0xd8/0xf0
 [<ffffffff816abb42>] ret_from_fork+0x22/0x40
 [<ffffffff810aa8e0>] ? kthread_park+0x60/0x60
---[ end trace 353add4e78cb2a76 ]---
BUG: unable to handle kernel NULL pointer dereference at 0000000000000040
IP: [<ffffffff81297924>] sysfs_do_create_link_sd.isra.2+0x34/0xb0
PGD 0
Oops: 0000 [#1] SMP
Modules linked in: btrfs xor raid6_pq msdos ext4 jbd2 mbcache matroxfb(OE) ccmod(POE) ftmod(OE) videosw(OE) ipmi_devintf fuse xt_CHECKSUM ipt_MASQUERADE nf_nat_masquerade_ipv4 tun ip6t_rpfilter ip6t_REJECT nf_reject_ipv6 ipt_REJECT nf_reject_ipv4 xt_conntrack bonding ebtable_nat ebtable_broute bridge stp llc ebtable_filter ebtables ip6table_nat nf_conntrack_ipv6 nf_defrag_ipv6 nf_nat_ipv6 ip6table_mangle ip6table_security ip6table_raw ip6table_filter ip6_tables iptable_nat nf_conntrack_ipv4 nf_defrag_ipv4 nf_nat_ipv4 nf_nat nf_conntrack iptable_mangle iptable_security iptable_raw iptable_filter vfat fat x86_pkg_temp_thermal intel_powerclamp coretemp crct10dif_pclmul crc32_pclmul ghash_clmulni_intel aesni_intel glue_helper lrw gf128mul ablk_helper cryptd dm_service_time pcspkr ses enclosure
  sg wmi nfsd auth_rpcgss nfs_acl lockd grace sunrpc dm_multipath ip_tables xfs libcrc32c raid1 sr_mod cdrom crc32c_intel qla2xxx(OE) scsi_transport_fc ixgbe(OE) mdio igb(OE) ptp pps_core mpt3sas(OE) 
raid_class i2c_algo_bit sra_sense(OE) i2c_core sd_mod(OE) scsi_transport_sas dca scsi_hbas(OE) fjes ipmi_msghandler usb_storage dm_mirror dm_region_hash dm_log dm_mod [last unloaded: ipmi_devintf]
CPU: 5 PID: 30702 Comm: kworker/u97:7 Tainted: P        W  OE   4.6.0-rc6+ #37
Hardware name: Stratus ftServer 6800/G7LYY, BIOS BIOS Version 8.1:64 02/03/2016
Workqueue: events_unbound async_run_entry_fn
task: ffff88038adfda40 ti: ffff88038ae00000 task.ti: ffff88038ae00000
RIP: 0010:[<ffffffff81297924>]  [<ffffffff81297924>] sysfs_do_create_link_sd.isra.2+0x34/0xb0
RSP: 0018:ffff88038ae03d08  EFLAGS: 00010246
RAX: 0000000000000000 RBX: 0000000000000040 RCX: 0000000000000001
RDX: 0000000000000001 RSI: 0000000000000040 RDI: ffffffff8221adf0
RBP: ffff88038ae03d30 R08: ffff88084d4b91f0 R09: 0000000000000000
R10: ffff8801f1832608 R11: ffff8801f1832f88 R12: ffffffff81a2034e
R13: ffff8808580bae58 R14: 0000000000000001 R15: ffff8804ac9c7038
FS:  0000000000000000(0000) GS:ffff88085f940000(0000) knlGS:0000000000000000
CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 0000000000000040 CR3: 0000001054486000 CR4: 00000000001406e0
Stack:
 ffff8804ac9c6fc8 ffff8804ac9c7048 ffff8808352a0a58 ffff8804ac9c6fd4
 ffff8804ac9c7038 ffff88038ae03d40 ffffffff812979c5 ffff88038ae03da8
 ffffffff81327f15 ffff8804ac9c6fc8 041000008ae03d68 ffff8804ac9c6fc8
Call Trace:
 [<ffffffff812979c5>] sysfs_create_link+0x25/0x40
 [<ffffffff81327f15>] add_disk+0x215/0x4a0
 [<ffffffffa00a1a25>] sd_probe_async+0x115/0x1d0 [sd_mod]
 [<ffffffff810ad80a>] async_run_entry_fn+0x4a/0x140
 [<ffffffff810a464e>] process_one_work+0x16e/0x420
 [<ffffffff810a4f45>] worker_thread+0x125/0x4b0
 [<ffffffff816a780d>] ? __schedule+0x2ad/0x8a0
 [<ffffffff810a4e20>] ? rescuer_thread+0x380/0x380
 [<ffffffff810aa9b8>] kthread+0xd8/0xf0
 [<ffffffff816abb42>] ret_from_fork+0x22/0x40
 [<ffffffff810aa8e0>] ? kthread_park+0x60/0x60
Code: 48 89 e5 41 57 41 56 41 55 41 54 49 89 d4 53 74 73 48 85 ff 49 89 fd 74 6b 48 89 f3 48 c7 c7 f0 ad 21 82 41 89 ce e8 ac 3d 41 00 <48> 8b 1b 48 85 db 74 08 48 89 df e8 ac c1 ff ff 48 c7 c7 f0 ad
RIP  [<ffffffff81297924>] sysfs_do_create_link_sd.isra.2+0x34/0xb0
 RSP <ffff88038ae03d08>
CR2: 0000000000000040
sd 1:0:0:2: [sds] Read Capacity(16) failed: Result: hostbyte=DID_NO_CONNECT driverbyte=DRIVER_OK
sd 1:0:0:2: [sds] Sense not available.
sd 1:0:0:2: [sds] Read Capacity(10) failed: Result: hostbyte=DID_NO_CONNECT driverbyte=DRIVER_OK
sd 1:0:0:2: [sds] Sense not available.
sd 1:0:0:2: [sds] Write Protect is off
sd 1:0:0:2: [sds] Mode Sense: 00 00 00 60
sd 1:0:0:2: [sds] Asking for cache data failed
sd 1:0:0:2: [sds] Assuming drive cache: write through

Regards,

-- Joe

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] Fix a bdi reregistration race, v3
  2016-05-05 20:40   ` Joe Lawrence
@ 2016-05-09 13:50     ` Joe Lawrence
  2016-06-03 23:25       ` Bart Van Assche
  0 siblings, 1 reply; 6+ messages in thread
From: Joe Lawrence @ 2016-05-09 13:50 UTC (permalink / raw)
  To: Bart Van Assche, James Bottomley, Martin K. Petersen
  Cc: Christoph Hellwig, Hannes Reinecke, linux-scsi

On 05/05/2016 04:40 PM, Joe Lawrence wrote:
> On 05/05/2016 03:58 PM, Bart Van Assche wrote:
>> On 03/28/2016 02:29 PM, Bart Van Assche wrote:
>>> Avoid that the sd driver registers a BDI device with a name that
>>> is still in use. This patch avoids that the following warning gets
>>> triggered:
>>>
>>> [ ... ]
>>
>> (replying to my own e-mail)
>>
>> If anyone could review this patch that would be very welcome.
> 
> Hi Bart,
> 
> I *think* I may be hitting this same problem running some tests here at Stratus
> ... snip...


Hi Bart,

Good news = With your v3 patch, I didn't see the "sysfs: cannot create
duplicate filename '/devices/virtual/bdi/65:0'" warning during my
weekend testing (573 surprise disk HBA removals).

Bad news = I still crashed in add_disk > sysfs_create_link >
sysfs_do_create_link_sd on a NULL target_kobj->sd ... unfortunately I
don't have kdump working, so all I have is a serial console output to
work with for now.

Regards,

-- Joe

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] Fix a bdi reregistration race, v3
  2016-05-09 13:50     ` Joe Lawrence
@ 2016-06-03 23:25       ` Bart Van Assche
  2016-06-15 10:49         ` Bart Van Assche
  0 siblings, 1 reply; 6+ messages in thread
From: Bart Van Assche @ 2016-06-03 23:25 UTC (permalink / raw)
  To: Joe Lawrence, James Bottomley, Martin K. Petersen
  Cc: Christoph Hellwig, Hannes Reinecke, linux-scsi

On 05/09/2016 06:50 AM, Joe Lawrence wrote:
> On 05/05/2016 04:40 PM, Joe Lawrence wrote:
>> On 05/05/2016 03:58 PM, Bart Van Assche wrote:
>>> On 03/28/2016 02:29 PM, Bart Van Assche wrote:
>>>> Avoid that the sd driver registers a BDI device with a name that
>>>> is still in use. This patch avoids that the following warning gets
>>>> triggered:
>>>>
>>>> [ ... ]
>>>
>>> (replying to my own e-mail)
>>>
>>> If anyone could review this patch that would be very welcome.
>>
>> I *think* I may be hitting this same problem running some tests here at Stratus
>> ... snip...
> 
> Good news = With your v3 patch, I didn't see the "sysfs: cannot create
> duplicate filename '/devices/virtual/bdi/65:0'" warning during my
> weekend testing (573 surprise disk HBA removals).
> 
> Bad news = I still crashed in add_disk > sysfs_create_link >
> sysfs_do_create_link_sd on a NULL target_kobj->sd ... unfortunately I
> don't have kdump working, so all I have is a serial console output to
> work with for now.

(replying to an e-mail of one month ago)

Hello Joe,

Earlier today I discovered a subtle bug in v3 of this patch. It would
be appreciated if you could give v4 a try. The only substantial 
difference between v3 and v4 is that a "if (dev->class != &sdev_class)"
test has been added.

Thanks,

Bart.


[PATCH] Fix a bdi reregistration race, v4

Avoid that the sd driver registers a BDI device with a name that
is still in use. This patch avoids that the following warning gets
triggered:

WARNING: CPU: 7 PID: 203 at fs/sysfs/dir.c:31 sysfs_warn_dup+0x68/0x80()
sysfs: cannot create duplicate filename '/devices/virtual/bdi/8:32'
Workqueue: events_unbound async_run_entry_fn
Call Trace:
[<ffffffff814ff5a4>] dump_stack+0x4c/0x65
[<ffffffff810746ba>] warn_slowpath_common+0x8a/0xc0
[<ffffffff81074736>] warn_slowpath_fmt+0x46/0x50
[<ffffffff81237ca8>] sysfs_warn_dup+0x68/0x80
[<ffffffff81237d8e>] sysfs_create_dir_ns+0x7e/0x90
[<ffffffff81291f58>] kobject_add_internal+0xa8/0x320
[<ffffffff812923a0>] kobject_add+0x60/0xb0
[<ffffffff8138c937>] device_add+0x107/0x5e0
[<ffffffff8138d018>] device_create_groups_vargs+0xd8/0x100
[<ffffffff8138d05c>] device_create_vargs+0x1c/0x20
[<ffffffff8117f233>] bdi_register+0x63/0x2a0
[<ffffffff8117f497>] bdi_register_dev+0x27/0x30
[<ffffffff81281549>] add_disk+0x1a9/0x4e0
[<ffffffffa00c5739>] sd_probe_async+0x119/0x1d0 [sd_mod]
[<ffffffff8109a81a>] async_run_entry_fn+0x4a/0x140
[<ffffffff81091078>] process_one_work+0x1d8/0x7c0
[<ffffffff81091774>] worker_thread+0x114/0x460
[<ffffffff81097878>] kthread+0xf8/0x110
[<ffffffff8150801f>] ret_from_fork+0x3f/0x70

Signed-off-by: Bart Van Assche <bart.vanassche@sandisk.com>
Cc: Christoph Hellwig <hch@lst.de>
Cc: Hannes Reinecke <hare@suse.de>
Cc: <stable@vger.kernel.org>
---
 drivers/scsi/scsi_sysfs.c | 39 ++++++++++++++++++++++++++++++++++++++-
 1 file changed, 38 insertions(+), 1 deletion(-)

diff --git a/drivers/scsi/scsi_sysfs.c b/drivers/scsi/scsi_sysfs.c
index 0734927..ac10f0c 100644
--- a/drivers/scsi/scsi_sysfs.c
+++ b/drivers/scsi/scsi_sysfs.c
@@ -1273,9 +1273,35 @@ int scsi_sysfs_add_sdev(struct scsi_device *sdev)
 	return error;
 }
 
+/**
+ * scsi_filter_sd - Look up the device structure embedded in a disk structure
+ * @dev: A sdev_gendev device
+ * @data: A struct device pointer
+ *
+ * sdev_gendev devices have two children - the sdev_dev device and for SCSI
+ * disks, the device embedded in a scsi_disk.
+ */
+static int scsi_filter_sd(struct device *dev, void *data)
+{
+	struct device **childp = data;
+
+	if (dev->class != &sdev_class)
+		*childp = dev;
+	return 0;
+}
+
+/* Caller must call put_device() if this function does not return NULL. */
+static struct device *scsi_get_sd(struct device *dev)
+{
+	struct device *child = NULL;
+
+	device_for_each_child(dev, &child, scsi_filter_sd);
+	return get_device(child);
+}
+
 void __scsi_remove_device(struct scsi_device *sdev)
 {
-	struct device *dev = &sdev->sdev_gendev;
+	struct device *dev = &sdev->sdev_gendev, *sdp = NULL;
 
 	/*
 	 * This cleanup path is not reentrant and while it is impossible
@@ -1290,6 +1316,7 @@ void __scsi_remove_device(struct scsi_device *sdev)
 			return;
 
 		bsg_unregister_queue(sdev->request_queue);
+		sdp = scsi_get_sd(dev);
 		device_unregister(&sdev->sdev_dev);
 		transport_remove_device(dev);
 		scsi_dh_remove_device(sdev);
@@ -1306,6 +1333,16 @@ void __scsi_remove_device(struct scsi_device *sdev)
 	blk_cleanup_queue(sdev->request_queue);
 	cancel_work_sync(&sdev->requeue_work);
 
+	/*
+	 * blk_cleanup_queue() unregisters the BDI device. The name of the
+	 * BDI device is derived from the dev_t of the /dev/sd<n> device.
+	 * Keep a reference to the /dev/sd<n> device until the BDI device
+	 * has been unregistered to avoid that a BDI device with the same
+	 * name gets registered before blk_cleanup_queue() has finished.
+	 */
+	if (sdp)
+		put_device(sdp);
+
 	if (sdev->host->hostt->slave_destroy)
 		sdev->host->hostt->slave_destroy(sdev);
 	transport_destroy_device(dev);
-- 
2.8.3


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] Fix a bdi reregistration race, v3
  2016-06-03 23:25       ` Bart Van Assche
@ 2016-06-15 10:49         ` Bart Van Assche
  0 siblings, 0 replies; 6+ messages in thread
From: Bart Van Assche @ 2016-06-15 10:49 UTC (permalink / raw)
  To: Joe Lawrence, James Bottomley, Martin K. Petersen
  Cc: Christoph Hellwig, Hannes Reinecke, linux-scsi

On 06/04/2016 01:25 AM, Bart Van Assche wrote:
> Earlier today I discovered a subtle bug in v3 of this patch. It would
> be appreciated if you could give v4 a try. The only substantial
> difference between v3 and v4 is that a "if (dev->class != &sdev_class)"
> test has been added.

(again replying to my own e-mail)

Sorry but even v4 was not perfect. Will post v5 later today.

Bart.

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2016-06-15 10:49 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-03-28 21:29 [PATCH] Fix a bdi reregistration race, v3 Bart Van Assche
2016-05-05 19:58 ` Bart Van Assche
2016-05-05 20:40   ` Joe Lawrence
2016-05-09 13:50     ` Joe Lawrence
2016-06-03 23:25       ` Bart Van Assche
2016-06-15 10:49         ` Bart Van Assche

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.