Hi, Hannes:
On 06/07/2013 04:28 AM, Jörn Engel wrote:
On Thu, 6 June 2013 22:39:14 +0200, Hannes Reinecke wrote:
+ spin_unlock_irqrestore(&sdev->list_lock, flags);
+ SCSI_LOG_ERROR_RECOVERY(3,
+ scmd_printk(KERN_INFO, scmd,
+ "aborting command %p\n", scmd));
+ rtn = scsi_try_to_abort_cmd(shost->hostt, scmd);
+ if (rtn == SUCCESS || rtn == FAST_IO_FAIL) {
+ if (((scmd->request->cmd_flags& REQ_FAILFAST_DEV) ||
Am I being stupid again or should this be negated?
Knowing you I would think the former; where do you see the negation?
If REQ_FAILFAST_DEV is set, this runs scsi_queue_insert(), which I
would expect it should run scsi_finish_command().
I also think (scmd->request->cmd_flags & REQ_FAILFAST_DEV) and
(scmd->request->cmd_type == REQ_TYPE_BLOCK_PC) should be negated.
I'm confused why not use !scsi_noretry_cmd(scmd) directly as your
former patch here?
+ (scmd->request->cmd_type == REQ_TYPE_BLOCK_PC))&&
+ (++scmd->retries<= scmd->allowed)) {
+ SCSI_LOG_ERROR_RECOVERY(3,
+ scmd_printk(KERN_WARNING, scmd,
+ "retry aborted command\n"));
+
+ scsi_queue_insert(scmd, SCSI_MLQUEUE_EH_RETRY);
+ } else {
+ SCSI_LOG_ERROR_RECOVERY(3,
+ scmd_printk(KERN_WARNING, scmd,
+ "fast fail aborted command\n"));
+ scmd->result |= DID_TRANSPORT_FAILFAST<< 16;
+ scsi_finish_command(scmd);
+ }
+ } else {
+ if (!scsi_eh_scmd_add(scmd, 0)) {
+ SCSI_LOG_ERROR_RECOVERY(3,
+ scmd_printk(KERN_WARNING, scmd,
+ "terminate aborted command\n"));
+ scmd->result |= DID_TIME_OUT<< 16;
+ scsi_finish_command(scmd);
+ }
+ }
+ spin_lock_irqsave(&sdev->list_lock, flags);
+ }
+ spin_unlock_irqrestore(&sdev->list_lock, flags);
...
+/**
+ * scsi_abort_command - schedule a command abort
+ * @scmd: scmd to abort.
+ *
+ * We only need to abort commands after a command timeout
+ */
+void
+scsi_abort_command(struct scsi_cmnd *scmd)
+{
+ unsigned long flags;
+ int kick_worker = 0;
+ struct scsi_device *sdev = scmd->device;
+
+ spin_lock_irqsave(&sdev->list_lock, flags);
+ if (list_empty(&sdev->eh_abort_list))
+ kick_worker = 1;
+ list_add(&scmd->eh_entry,&sdev->eh_abort_list);
+ SCSI_LOG_ERROR_RECOVERY(3,
+ scmd_printk(KERN_INFO, scmd, "adding to eh_abort_list\n"));
+ spin_unlock_irqrestore(&sdev->list_lock, flags);
+ if (kick_worker)
+ schedule_work(&sdev->abort_work);
+}
+EXPORT_SYMBOL_GPL(scsi_abort_command);
Should the name of function above be more ideographic/understandable?
For example, scsi_abort_scmd_add? I was bewildered among functions
named scsi_abort_eh_cmnd, scsi_eh_abort_cmds...
Thanks,
Ren
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
