Fix error handler offline behaviour
Subsystems (2)
PATCH 1/1 Fix error handler offline behaviour
No-one seems to have noticed, but there's a bug in our offline handling which can cause the eh to loop forever when it tries to offline a device (basically offline I/O rejections are done in prep, which doesn't work for already prepped commands).
The attached fixes this, and also sweeps offline up into a fairly fully fledged scsi state model.
With this applied I can now offline my root device under load without causing a SCSI hang (ext2 BUG()s rather unhappily but at least that's not a SCSI problem).
James
===== drivers/scsi/dpt_i2o.c 1.35 vs edited =====
drivers/scsi/dpt_i2o.c +4 -4
@@ -592,7 +592,7 @@ unit = d->pI2o_dev->lct_data.tid; len +=sprintf (buffer+len,"\tTID=%d, (Channel=%d, Target=%d, Lun=%d) (%s)\n\n" , unit, (int )d->scsi_channel, (int )d->scsi_id, (int )d->scsi_lun, - d->pScsi_dev->online?"online" :"offline" ); +scsi_device_online (d->pScsi_dev)?"online" :"offline" ); pos = begin + len;/* CHECKPOINT */
@@ -2436,11 +2436,11 @@// We found an old device - check it while (pDev) {if (pDev->scsi_lun == scsi_lun) { -if (pDev->pScsi_dev->online == FALSE) { +if (!scsi_device_online (pDev->pScsi_dev)) {printk (KERN_WARNING "%s: Setting device (%d,%d,%d) back online\n" , pHba->name,bus_no,scsi_id,scsi_lun);if (pDev->pScsi_dev) { - pDev->pScsi_dev->online = TRUE; +scsi_device_set_state (pDev->pScsi_dev,SDEV_RUNNING ); } } d = pDev->pI2o_dev;
@@ -2471,7 +2471,7 @@ pDev->state =DPTI_DEV_OFFLINE ;printk (KERN_WARNING "%s: Device (%d,%d,%d) offline\n" ,pHba->name,pDev->scsi_channel,pDev->scsi_id,pDev->scsi_lun);if (pDev->pScsi_dev) { - pDev->pScsi_dev->online = FALSE; +scsi_device_set_state (pDev->pScsi_dev,SDEV_OFFLINE ); } } }
===== drivers/scsi/scsi_error.c 1.71 vs edited =====
drivers/scsi/scsi_error.c +18 -11
@@ -186,12 +186,16 @@**/ int scsi_block_when_processing_errors (struct scsi_device *sdev) { +int online; +wait_event (sdev->host->host_wait, (!test_bit (SHOST_RECOVERY , &sdev->host->shost_state))); + online =scsi_device_online (sdev); +SCSI_LOG_ERROR_RECOVERY (5 ,printk ("%s: rtn: %d\n" , __FUNCTION__, - sdev->online)); + online)); -return sdev->online; +return online; }#ifdef CONFIG_SCSI_LOGGING
@@ -792,7 +796,8 @@ rtn =scsi_try_to_abort_cmd (scmd);if (rtn ==SUCCESS ) {scsi_eh_eflags_clr (scmd,SCSI_EH_CANCEL_CMD ); -if (!scmd->device->online || !scsi_eh_tur (scmd)) { +if (!scsi_device_online (scmd->device) || + !scsi_eh_tur (scmd)) {scsi_eh_finish_cmd (scmd, done_q); }
@@ -920,7 +925,8 @@ " 0x%p\n", current->comm, sdev));if (!scsi_eh_try_stu (stu_scmd)) { -if (!sdev->online || !scsi_eh_tur (stu_scmd)) { +if (!scsi_device_online (sdev) || + !scsi_eh_tur (stu_scmd)) {list_for_each_safe (lh, lh_sf, work_q) { scmd = list_entry(lh, struct scsi_cmnd, eh_entry);if (scmd->device == sdev)
@@ -974,7 +980,8 @@ sdev)); rtn =scsi_try_bus_device_reset (bdr_scmd);if (rtn ==SUCCESS ) { -if (!sdev->online || !scsi_eh_tur (bdr_scmd)) { +if (!scsi_device_online (sdev) || + !scsi_eh_tur (bdr_scmd)) {list_for_each_safe (lh, lh_sf, work_q) { scmd = list_entry(lh, struct
@@ -1105,7 +1112,7 @@ scmd = list_entry(lh, struct scsi_cmnd, eh_entry);if (channel == scmd->device->channel) -if (!scmd->device->online || +if (!scsi_device_online (scmd->device) || !scsi_eh_tur (scmd))scsi_eh_finish_cmd (scmd, done_q);
@@ -1143,7 +1150,7 @@if (rtn ==SUCCESS ) {list_for_each_safe (lh, lh_sf, work_q) { scmd = list_entry(lh, struct scsi_cmnd, eh_entry); -if (!scmd->device->online || +if (!scsi_device_online (scmd->device) || (!scsi_eh_try_stu (scmd) && !scsi_eh_tur (scmd)) || !scsi_eh_tur (scmd))scsi_eh_finish_cmd (scmd, done_q);
@@ -1178,7 +1185,7 @@ scmd->device->channel, scmd->device->id, scmd->device->lun); - scmd->device->online = FALSE; +scsi_device_set_state (scmd->device,SDEV_OFFLINE );if (scsi_eh_eflags_chk (scmd,SCSI_EH_CANCEL_CMD )) {/* * FIXME: Handle lost cmds.
@@ -1248,7 +1255,7 @@* if the device is offline, then we clearly just pass the result back * up to the top level. */ -if (!scmd->device->online) { +if (!scsi_device_online (scmd->device)) {SCSI_LOG_ERROR_RECOVERY (5 ,printk ("%s: device offline - report" " as SUCCESS\n" , __FUNCTION__));
@@ -1480,7 +1487,7 @@* is no point trying to lock the door of an off-line device. */ shost_for_each_device (sdev, shost) { -if (sdev->online && sdev->locked) +if (scsi_device_online (sdev) && sdev->locked)scsi_eh_lock_door (sdev); }
@@ -1535,7 +1542,7 @@list_for_each_safe (lh, lh_sf, done_q) { scmd = list_entry(lh, struct scsi_cmnd, eh_entry);list_del_init (lh); -if (scmd->device->online && +if (scsi_device_online (scmd->device) && (++scmd->retries < scmd->allowed)) { SCSI_LOG_ERROR_RECOVERY(3, printk("%s: flush" " retry cmd: %p\n",
===== drivers/scsi/scsi_lib.c 1.123 vs edited =====
drivers/scsi/scsi_lib.c +86 -30
170 lines not on this page · raw message
===== drivers/scsi/scsi_scan.c 1.117 vs edited =====
drivers/scsi/scsi_scan.c +1 -2
@@ -205,7 +205,6 @@ sdev->id = id; sdev->lun = lun; sdev->channel = channel; - sdev->online = TRUE; sdev->sdev_state =SDEV_CREATED ;INIT_LIST_HEAD (&sdev->siblings);INIT_LIST_HEAD (&sdev->same_target_siblings);
@@ -552,7 +551,7 @@if (((inq_result[0 ] >>5 ) &7 ) ==1 ) {SCSI_LOG_SCAN_BUS (3 ,printk (KERN_INFO "scsi scan: peripheral" " qualifier of 1, device offlined\n" )); - sdev->online = FALSE; +scsi_device_set_state (sdev,SDEV_OFFLINE ); } sdev->removable = (0x80 & inq_result[1 ]) >>7 ;
===== drivers/scsi/scsi_sysfs.c 1.43 vs edited =====
drivers/scsi/scsi_sysfs.c +70 -4
118 lines not on this page · raw message
===== drivers/scsi/sd.c 1.142 vs edited =====
drivers/scsi/sd.c +5 -5
@@ -245,7 +245,7 @@SCSI_LOG_HLQUEUE (1 ,printk ("sd_init_command: disk=%s, block=%llu, " "count=%d\n" , disk->disk_name, (unsigned long long )block, this_count)); -if (!sdp || !sdp->online || +if (!sdp || !scsi_device_online (sdp) || block + SCpnt->request->nr_sectors >get_capacity (disk)) {SCSI_LOG_HLQUEUE (2 ,printk ("Finishing %ld sectors\n" , SCpnt->request->nr_sectors));
@@ -452,7 +452,7 @@* open actually succeeded. */ retval = -ENXIO ; -if (!sdev->online) +if (!scsi_device_online (sdev)) goto error_out;if (!sdkp->openers++ && sdev->removable) {
@@ -619,7 +619,7 @@* can deal with it then. It is only because of unrecoverable errors * that we would ever take a device offline in the first place. */ -if (!sdp->online) +if (!scsi_device_online (sdp)) goto not_present;/*
@@ -1254,7 +1254,7 @@* If the device is offline, don't try and read capacity or any * of the other niceties. */ -if (!sdp->online) +if (!scsi_device_online (sdp)) goto out; sreq =scsi_allocate_request (sdp,GFP_KERNEL );
@@ -1474,7 +1474,7 @@if (!sdkp)return ;/* this can happen */ -if (!sdp->online || !sdkp->WCE) +if (!scsi_device_online (sdp) || !sdkp->WCE)return ; printk(KERN_NOTICE "Synchronizing SCSI cache for disk %s: ",
===== drivers/scsi/sg.c 1.84 vs edited =====
drivers/scsi/sg.c +1 -1
@@ -2920,7 +2920,7 @@1 , (int ) scsidp->queue_depth, (int ) scsidp->device_busy, - (int) scsidp->online); + (int) scsi_device_online(scsidp)); else seq_printf(s, "-1\t-1\t-1\t-1\t-1\t-1\t-1\t-1\t-1\n");return 0 ;
===== drivers/scsi/sr.c 1.100 vs edited =====
drivers/scsi/sr.c +1 -1
@@ -267,7 +267,7 @@SCSI_LOG_HLQUEUE (1 ,printk ("Doing sr request, dev = %s, block = %d\n" , cd->disk->disk_name, block)); -if (!cd->device || !cd->device->online) { +if (!cd->device || !scsi_device_online (cd->device)) {SCSI_LOG_HLQUEUE (2 ,printk ("Finishing %ld sectors\n" , SCpnt->request->nr_sectors));SCSI_LOG_HLQUEUE (2 ,printk ("Retry with 0x%p\n" , SCpnt));
===== include/scsi/scsi_device.h 1.14 vs edited =====
include/scsi/scsi_device.h +10 -3
@@ -12,7 +12,9 @@/* -* sdev state +* sdev state: If you alter this, you also need to alter scsi_sysfs.c +* (for the ascii descriptions) and the state model enforcer: +* scsi_lib:scsi_device_set_state(). */ enum scsi_device_state {SDEV_CREATED =1 ,/* device created but not added to sysfs
@@ -26,6 +28,8 @@SDEV_QUIESCE ,/* Device quiescent. No block commands * will be accepted, only specials (which * originate in the mid-layer) */ + SDEV_OFFLINE, /* Device offlined (by error handling or + * user request */ }; struct scsi_device {
@@ -67,8 +71,6 @@unsigned char current_tag;/* current tag */ struct scsi_target *sdev_target;/* used only for single_lun */ - unsigned online:1; - unsigned writeable:1; unsigned removable:1; unsigned changed:1; /* Data invalid due to media change */
@@ -177,4 +179,9 @@ enum scsi_device_state state);extern int scsi_device_quiesce (struct scsi_device *sdev);extern void scsi_device_resume (struct scsi_device *sdev); +extern const char *scsi_device_state_name (enum scsi_device_state ); +static int inline scsi_device_online (struct scsi_device *sdev) +{ +return sdev->sdev_state !=SDEV_OFFLINE ; +}#endif /* _SCSI_SCSI_DEVICE_H */