* scsi_add_device/scsi_remove_device oops
@ 2004-07-01 23:29 Brian King
2004-07-02 20:18 ` [PATCH] scsi_remove_device locking Brian King
0 siblings, 1 reply; 4+ messages in thread
From: Brian King @ 2004-07-01 23:29 UTC (permalink / raw)
To: linux-scsi
I am experiencing an oops when calling both scsi_add_device and
scsi_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
--
Brian King
eServer Storage I/O
IBM Linux Technology Center
^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH] scsi_remove_device locking
2004-07-01 23:29 scsi_add_device/scsi_remove_device oops Brian King
@ 2004-07-02 20:18 ` Brian King
2004-07-02 20:23 ` Matthew Wilcox
0 siblings, 1 reply; 4+ messages in thread
From: Brian King @ 2004-07-02 20:18 UTC (permalink / raw)
To: James.Bottomley; +Cc: linux-scsi
[-- Attachment #1: Type: text/plain, Size: 64 bytes --]
--
Brian King
eServer Storage I/O
IBM Linux Technology Center
[-- Attachment #2: scsi_remove_device_locking.patch --]
[-- Type: text/plain, Size: 1467 bytes --]
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.
Signed-off-by: Brian King <brking@us.ibm.com>
---
linux-2.6.7-bjking1/drivers/scsi/scsi_sysfs.c | 8 +++++++-
1 files changed, 7 insertions(+), 1 deletion(-)
diff -puN drivers/scsi/scsi_sysfs.c~scsi_remove_device_locking drivers/scsi/scsi_sysfs.c
--- linux-2.6.7/drivers/scsi/scsi_sysfs.c~scsi_remove_device_locking 2004-07-02 11:34:25.000000000 -0500
+++ linux-2.6.7-bjking1/drivers/scsi/scsi_sysfs.c 2004-07-02 11:37:00.000000000 -0500
@@ -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_devi
if (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)
_
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH] scsi_remove_device locking
2004-07-02 20:18 ` [PATCH] scsi_remove_device locking Brian King
@ 2004-07-02 20:23 ` Matthew Wilcox
2004-07-02 20:34 ` Brian King
0 siblings, 1 reply; 4+ messages in thread
From: Matthew Wilcox @ 2004-07-02 20:23 UTC (permalink / raw)
To: Brian King; +Cc: James.Bottomley, linux-scsi
On Fri, Jul 02, 2004 at 03:18:50PM -0500, Brian King wrote:
> {
> - 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_devi
> if (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);
}
--
"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
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH] scsi_remove_device locking
2004-07-02 20:23 ` Matthew Wilcox
@ 2004-07-02 20:34 ` Brian King
0 siblings, 0 replies; 4+ messages in thread
From: Brian King @ 2004-07-02 20:34 UTC (permalink / raw)
To: Matthew Wilcox; +Cc: James.Bottomley, linux-scsi
[-- Attachment #1: Type: text/plain, Size: 116 bytes --]
Here is an updated patch with Matthew's suggestion.
--
Brian King
eServer Storage I/O
IBM Linux Technology Center
[-- Attachment #2: scsi_remove_device_locking.patch --]
[-- Type: text/plain, Size: 1402 bytes --]
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.
Signed-off-by: Brian King <brking@us.ibm.com>
---
linux-2.6.7-bjking1/drivers/scsi/scsi_sysfs.c | 8 +++++++-
1 files changed, 7 insertions(+), 1 deletion(-)
diff -puN drivers/scsi/scsi_sysfs.c~scsi_remove_device_locking drivers/scsi/scsi_sysfs.c
--- linux-2.6.7/drivers/scsi/scsi_sysfs.c~scsi_remove_device_locking 2004-07-02 11:34:25.000000000 -0500
+++ linux-2.6.7-bjking1/drivers/scsi/scsi_sysfs.c 2004-07-02 15:29:59.000000000 -0500
@@ -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_devi
if (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)
_
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2004-07-02 20:34 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2004-07-01 23:29 scsi_add_device/scsi_remove_device oops Brian King
2004-07-02 20:18 ` [PATCH] scsi_remove_device locking Brian King
2004-07-02 20:23 ` Matthew Wilcox
2004-07-02 20:34 ` Brian King
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox