scsi_add_device/scsi_remove_device oops
I am experiencing an oops when calling both scsi_add_device andscsi_remove_device at the same time. Is the API supposed to be able to
handle this? I can patch around the exact oops I encountered, but it
looks like there would be more problems in this code path as well.
If I am not following the API, I can try putting some more intelligence in ipr so that I don't try to delete a device that hasn't fully been added, but I want to make sure I am fixing this the proper way.
Looking at the backtraces, it looks like slave_configure has already been called on the device and we are constructing the classdev and trying to destroy it at the same time.
Here are the backtraces:
0xc00000033fecb5c0 0xc000000000043098 .do_page_fault +0x43c 0xc00000033fecb6f0 0xc00000000000ad28 DataAccessSLB_common +0x114 --- Exception: 380: at .sysfs_hash_and_remove +0x2c 0xc00000033fecb9e0 0xc00000000011bc4c .sysfs_hash_and_remove +0x2c 0xc00000033fecb9e0 0xc00000000011d644 (lr) .sysfs_remove_link +0x14 0xc00000033fecba70 0xc00000000011d644 .sysfs_remove_link +0x14 0xc00000033fecbaf0 0xc00000000024d978 .class_device_del +0x1ec 0xc00000033fecbba0 0xc00000000024d9d4 .class_device_unregister +0x1c 0xc00000033fecbc30 0xd000000000095818 .scsi_remove_device +0x54 0xc00000033fecbcb0 0xd0000000000e422c .ipr_worker_thread +0x960 0xc00000033fecbdb0 0xc0000000000746ec .worker_thread +0x24c 0xc00000033fecbed0 0xc00000000007a584 .kthread +0x17c 0xc00000033fecbf90 0xc000000000017924 .kernel_thread +0x4c
and
0xc0000000028e7330 0xc00000000005841c .schedule +0xbc 0xc0000000028e7450 0xc00000000005985c .wait_for_completion +0xec 0xc0000000028e7550 0xd000000000092738 .scsi_wait_req +0x88 0xc0000000028e7600 0xd0000000000574b0 .sd_revalidate_disk +0x160 0xc0000000028e7750 0xd000000000058b6c .sd_probe +0x238 0xc0000000028e7800 0xc00000000024c060 .bus_match +0x94 0xc0000000028e7890 0xc00000000024c130 .device_attach +0x6c 0xc0000000028e7920 0xc00000000024c2c4 .bus_add_device +0x98 0xc0000000028e79b0 0xc00000000024a710 .device_add +0xd0 0xc0000000028e7a50 0xd0000000000959b8 .scsi_sysfs_add_sdev +0x8c 0xc0000000028e7b00 0xd000000000093c54 .scsi_probe_and_add_lun +0x87c 0xc0000000028e7c00 0xd000000000094adc .scsi_add_device +0x78 0xc0000000028e7cb0 0xd0000000000e42c0 .ipr_worker_thread +0x9f4 0xc0000000028e7db0 0xc0000000000746ec .worker_thread +0x24c 0xc0000000028e7ed0 0xc00000000007a584 .kthread +0x17c 0xc0000000028e7f90 0xc000000000017924 .kernel_thread +0x4c
Signature · 3 lines
Brian King eServer Storage I/O IBM Linux Technology Center
PATCH 1/1 scsi_remove_device locking
Signature · 4 lines
-- Brian King eServer Storage I/O IBM Linux Technology Center
The following patch fixes an oops I was seeing on a machine with misconfigured scsi cables, but could feasibly happen in other paths. The oops was occurring because scsi_remove_device was getting called for a device before scsi_add_device had fully completed. This resulted in sysfs_remove_link being called with a NULL dentry.
---
linux-2.6.7-bjking1/drivers/scsi/scsi_sysfs.c | 8 +++++++- 1 files changed, 7 insertions(+), 1 deletion(-)
drivers/scsi/scsi_sysfs.c renamed from drivers/scsi/scsi_sysfs.c~scsi_remove_device_locking +7 -1
@@ -524,8 +524,13 @@ int scsi_sysfs_add_sdev(struct scsi_devi**/ void scsi_remove_device (struct scsi_device *sdev) { -if (scsi_device_set_state (sdev,SDEV_CANCEL ) !=0 ) +struct Scsi_Host *shost = sdev->host; +down (&shost->scan_mutex); + +if (scsi_device_set_state (sdev,SDEV_CANCEL ) !=0 ) { +up (&shost->scan_mutex);return ; + }class_device_unregister (&sdev->sdev_classdev);if (sdev->transport_classdev.class)
@@ -537,6 +542,7 @@ void scsi_remove_device(struct scsi_deviif (sdev->host->transportt->cleanup) sdev->host->transportt->cleanup (sdev);put_device (&sdev->sdev_gendev); +up (&shost->scan_mutex); }int scsi_register_driver (struct device_driver *drv)
_
Re: [PATCH] scsi_remove_device locking
Quoted 13 lines · Brian King
{
drivers/scsi/scsi_sysfs.c renamed from drivers/scsi/scsi_sysfs.c~scsi_remove_device_locking +6
-1-if (scsi_device_set_state (sdev,SDEV_CANCEL ) !=0 ) +struct Scsi_Host *shost = sdev->host; +down (&shost->scan_mutex); + +if (scsi_device_set_state (sdev,SDEV_CANCEL ) !=0 ) { +up (&shost->scan_mutex);return ; + }class_device_unregister (&sdev->sdev_classdev);if (sdev->transport_classdev.class)
drivers/scsi/scsi_sysfs.c void scsi_remove_device(struct scsi_devi · line 546
Quoted 5 lines
drivers/scsi/scsi_sysfs.c renamed from drivers/scsi/scsi_sysfs.c~scsi_remove_device_locking +1
-0@@ -537,6 +542,7 @@ void scsi_remove_device(struct scsi_deviif (sdev->host->transportt->cleanup) sdev->host->transportt->cleanup (sdev);put_device (&sdev->sdev_gendev); +up (&shost->scan_mutex); }
IMO, this would be better done as:
{
struct Scsi_Host *shost = sdev->host;
down(&shost->scan_mutex);
if (scsi_device_set_state(sdev, SDEV_CANCEL) != 0)
goto out;...
out:
up(&shost->scan_mutex); }
Signature · 6 lines
"Next the statesmen will invent cheap lies, putting the blame upon the nation that is attacked, and every man will be glad of those conscience-soothing falsities, and will diligently study them, and refuse to examine any refutations of them; and thus he will by and by convince himself that the war is just, and will thank God for the better sleep he enjoys after this process of grotesque self-deception." -- Mark Twain
Re: [PATCH] scsi_remove_device locking
Here is an updated patch with Matthew's suggestion.
Signature · 3 lines
Brian King eServer Storage I/O IBM Linux Technology Center
The following patch fixes an oops I was seeing on a machine with misconfigured scsi cables, but could feasibly happen in other paths. The oops was occurring because scsi_remove_device was getting called for a device before scsi_add_device had fully completed. This resulted in sysfs_remove_link being called with a NULL dentry.
---
linux-2.6.7-bjking1/drivers/scsi/scsi_sysfs.c | 8 +++++++- 1 files changed, 7 insertions(+), 1 deletion(-)
drivers/scsi/scsi_sysfs.c renamed from drivers/scsi/scsi_sysfs.c~scsi_remove_device_locking +7 -1
@@ -524,8 +524,11 @@ int scsi_sysfs_add_sdev(struct scsi_devi**/ void scsi_remove_device (struct scsi_device *sdev) { +struct Scsi_Host *shost = sdev->host; + +down (&shost->scan_mutex);if (scsi_device_set_state (sdev,SDEV_CANCEL ) !=0 ) -return ; + goto out;class_device_unregister (&sdev->sdev_classdev);if (sdev->transport_classdev.class)
@@ -537,6 +540,9 @@ void scsi_remove_device(struct scsi_deviif (sdev->host->transportt->cleanup) sdev->host->transportt->cleanup (sdev);put_device (&sdev->sdev_gendev); + +out: +up (&shost->scan_mutex); }int scsi_register_driver (struct device_driver *drv)
_