diff options
author | Tejun Heo <htejun@gmail.com> | 2006-07-08 07:17:26 -0400 |
---|---|---|
committer | Jeff Garzik <jeff@garzik.org> | 2006-07-19 14:06:53 -0400 |
commit | 4528e4da79675b4995e085046b8ffbe0415c3261 (patch) | |
tree | 9ddea7e2229a282879212ba9d4ba16f0e0a27955 | |
parent | f5beec49636bf8d5a34065c8ab030cd4ea84516f (diff) |
[PATCH] libata: fix autopsy ehc->i.action and ehc->i.dev handling
Commit 0662c58b3265f52f708a6d59476bc7862b01f9c0 updated
ata_eh_autopsy() to OR determined action to ehc->i.action to preserve
action mask set directly into ehc->i.action by nested functions. This
broke action mask clearing on SENSE_VALID case causing revalidation
and EH complete message on successful ATAPI CC.
This patch removes two local variables - action and failed_dev - which
cache ehc->i.action and ehc->i.dev respectively, and make the function
directly modify ehc->i.* fields to remove aliasing issues.
Signed-off-by: Tejun Heo <htejun@gmail.com>
Signed-off-by: Jeff Garzik <jeff@garzik.org>
-rw-r--r-- | drivers/scsi/libata-eh.c | 29 |
1 files changed, 12 insertions, 17 deletions
diff --git a/drivers/scsi/libata-eh.c b/drivers/scsi/libata-eh.c index 4b6aa30f4d68..36a801c83325 100644 --- a/drivers/scsi/libata-eh.c +++ b/drivers/scsi/libata-eh.c | |||
@@ -1276,8 +1276,6 @@ static int ata_eh_speed_down(struct ata_device *dev, int is_io, | |||
1276 | static void ata_eh_autopsy(struct ata_port *ap) | 1276 | static void ata_eh_autopsy(struct ata_port *ap) |
1277 | { | 1277 | { |
1278 | struct ata_eh_context *ehc = &ap->eh_context; | 1278 | struct ata_eh_context *ehc = &ap->eh_context; |
1279 | unsigned int action = ehc->i.action; | ||
1280 | struct ata_device *failed_dev = NULL; | ||
1281 | unsigned int all_err_mask = 0; | 1279 | unsigned int all_err_mask = 0; |
1282 | int tag, is_io = 0; | 1280 | int tag, is_io = 0; |
1283 | u32 serror; | 1281 | u32 serror; |
@@ -1294,7 +1292,7 @@ static void ata_eh_autopsy(struct ata_port *ap) | |||
1294 | ehc->i.serror |= serror; | 1292 | ehc->i.serror |= serror; |
1295 | ata_eh_analyze_serror(ap); | 1293 | ata_eh_analyze_serror(ap); |
1296 | } else if (rc != -EOPNOTSUPP) | 1294 | } else if (rc != -EOPNOTSUPP) |
1297 | action |= ATA_EH_HARDRESET; | 1295 | ehc->i.action |= ATA_EH_HARDRESET; |
1298 | 1296 | ||
1299 | /* analyze NCQ failure */ | 1297 | /* analyze NCQ failure */ |
1300 | ata_eh_analyze_ncq_error(ap); | 1298 | ata_eh_analyze_ncq_error(ap); |
@@ -1315,7 +1313,7 @@ static void ata_eh_autopsy(struct ata_port *ap) | |||
1315 | qc->err_mask |= ehc->i.err_mask; | 1313 | qc->err_mask |= ehc->i.err_mask; |
1316 | 1314 | ||
1317 | /* analyze TF */ | 1315 | /* analyze TF */ |
1318 | action |= ata_eh_analyze_tf(qc, &qc->result_tf); | 1316 | ehc->i.action |= ata_eh_analyze_tf(qc, &qc->result_tf); |
1319 | 1317 | ||
1320 | /* DEV errors are probably spurious in case of ATA_BUS error */ | 1318 | /* DEV errors are probably spurious in case of ATA_BUS error */ |
1321 | if (qc->err_mask & AC_ERR_ATA_BUS) | 1319 | if (qc->err_mask & AC_ERR_ATA_BUS) |
@@ -1329,11 +1327,11 @@ static void ata_eh_autopsy(struct ata_port *ap) | |||
1329 | /* SENSE_VALID trumps dev/unknown error and revalidation */ | 1327 | /* SENSE_VALID trumps dev/unknown error and revalidation */ |
1330 | if (qc->flags & ATA_QCFLAG_SENSE_VALID) { | 1328 | if (qc->flags & ATA_QCFLAG_SENSE_VALID) { |
1331 | qc->err_mask &= ~(AC_ERR_DEV | AC_ERR_OTHER); | 1329 | qc->err_mask &= ~(AC_ERR_DEV | AC_ERR_OTHER); |
1332 | action &= ~ATA_EH_REVALIDATE; | 1330 | ehc->i.action &= ~ATA_EH_REVALIDATE; |
1333 | } | 1331 | } |
1334 | 1332 | ||
1335 | /* accumulate error info */ | 1333 | /* accumulate error info */ |
1336 | failed_dev = qc->dev; | 1334 | ehc->i.dev = qc->dev; |
1337 | all_err_mask |= qc->err_mask; | 1335 | all_err_mask |= qc->err_mask; |
1338 | if (qc->flags & ATA_QCFLAG_IO) | 1336 | if (qc->flags & ATA_QCFLAG_IO) |
1339 | is_io = 1; | 1337 | is_io = 1; |
@@ -1342,25 +1340,22 @@ static void ata_eh_autopsy(struct ata_port *ap) | |||
1342 | /* enforce default EH actions */ | 1340 | /* enforce default EH actions */ |
1343 | if (ap->pflags & ATA_PFLAG_FROZEN || | 1341 | if (ap->pflags & ATA_PFLAG_FROZEN || |
1344 | all_err_mask & (AC_ERR_HSM | AC_ERR_TIMEOUT)) | 1342 | all_err_mask & (AC_ERR_HSM | AC_ERR_TIMEOUT)) |
1345 | action |= ATA_EH_SOFTRESET; | 1343 | ehc->i.action |= ATA_EH_SOFTRESET; |
1346 | else if (all_err_mask) | 1344 | else if (all_err_mask) |
1347 | action |= ATA_EH_REVALIDATE; | 1345 | ehc->i.action |= ATA_EH_REVALIDATE; |
1348 | 1346 | ||
1349 | /* if we have offending qcs and the associated failed device */ | 1347 | /* if we have offending qcs and the associated failed device */ |
1350 | if (failed_dev) { | 1348 | if (ehc->i.dev) { |
1351 | /* speed down */ | 1349 | /* speed down */ |
1352 | action |= ata_eh_speed_down(failed_dev, is_io, all_err_mask); | 1350 | ehc->i.action |= ata_eh_speed_down(ehc->i.dev, is_io, |
1351 | all_err_mask); | ||
1353 | 1352 | ||
1354 | /* perform per-dev EH action only on the offending device */ | 1353 | /* perform per-dev EH action only on the offending device */ |
1355 | ehc->i.dev_action[failed_dev->devno] |= | 1354 | ehc->i.dev_action[ehc->i.dev->devno] |= |
1356 | action & ATA_EH_PERDEV_MASK; | 1355 | ehc->i.action & ATA_EH_PERDEV_MASK; |
1357 | action &= ~ATA_EH_PERDEV_MASK; | 1356 | ehc->i.action &= ~ATA_EH_PERDEV_MASK; |
1358 | } | 1357 | } |
1359 | 1358 | ||
1360 | /* record autopsy result */ | ||
1361 | ehc->i.dev = failed_dev; | ||
1362 | ehc->i.action |= action; | ||
1363 | |||
1364 | DPRINTK("EXIT\n"); | 1359 | DPRINTK("EXIT\n"); |
1365 | } | 1360 | } |
1366 | 1361 | ||