Commit 3dff5721 authored by Dan Williams's avatar Dan Williams Committed by James Bottomley

[SCSI] libsas: close error handling vs sas_ata_task_done() race

Since sas_ata does not implement ->freeze(), completions for scmds and
internal commands can still arrive concurrent with
ata_scsi_cmd_error_handler() and sas_ata_post_internal() respectively.
By the time either of those is called libata has committed to completing
the qc, and the ATA_PFLAG_FROZEN flag tells sas_ata_task_done() it has
lost the race.

In the sas_ata_post_internal() case we take on the additional
responsibility of freeing the sas_task to close the race with
sas_ata_task_done() freeing the the task while sas_ata_post_internal()
is in the process of invoking ->lldd_abort_task().
Signed-off-by: default avatarDan Williams <dan.j.williams@intel.com>
Signed-off-by: default avatarJames Bottomley <JBottomley@Parallels.com>
parent e500a34b
...@@ -100,15 +100,31 @@ static void sas_ata_task_done(struct sas_task *task) ...@@ -100,15 +100,31 @@ static void sas_ata_task_done(struct sas_task *task)
enum ata_completion_errors ac; enum ata_completion_errors ac;
unsigned long flags; unsigned long flags;
struct ata_link *link; struct ata_link *link;
struct ata_port *ap;
if (!qc) if (!qc)
goto qc_already_gone; goto qc_already_gone;
dev = qc->ap->private_data; ap = qc->ap;
dev = ap->private_data;
sas_ha = dev->port->ha; sas_ha = dev->port->ha;
link = &dev->sata_dev.ap->link; link = &ap->link;
spin_lock_irqsave(ap->lock, flags);
/* check if we lost the race with libata/sas_ata_post_internal() */
if (unlikely(ap->pflags & ATA_PFLAG_FROZEN)) {
spin_unlock_irqrestore(ap->lock, flags);
if (qc->scsicmd)
goto qc_already_gone;
else {
/* if eh is not involved and the port is frozen then the
* ata internal abort process has taken responsibility
* for this sas_task
*/
return;
}
}
spin_lock_irqsave(dev->sata_dev.ap->lock, flags);
if (stat->stat == SAS_PROTO_RESPONSE || stat->stat == SAM_STAT_GOOD || if (stat->stat == SAS_PROTO_RESPONSE || stat->stat == SAM_STAT_GOOD ||
((stat->stat == SAM_STAT_CHECK_CONDITION && ((stat->stat == SAM_STAT_CHECK_CONDITION &&
dev->sata_dev.command_set == ATAPI_COMMAND_SET))) { dev->sata_dev.command_set == ATAPI_COMMAND_SET))) {
...@@ -143,7 +159,7 @@ static void sas_ata_task_done(struct sas_task *task) ...@@ -143,7 +159,7 @@ static void sas_ata_task_done(struct sas_task *task)
if (qc->scsicmd) if (qc->scsicmd)
ASSIGN_SAS_TASK(qc->scsicmd, NULL); ASSIGN_SAS_TASK(qc->scsicmd, NULL);
ata_qc_complete(qc); ata_qc_complete(qc);
spin_unlock_irqrestore(dev->sata_dev.ap->lock, flags); spin_unlock_irqrestore(ap->lock, flags);
qc_already_gone: qc_already_gone:
list_del_init(&task->list); list_del_init(&task->list);
...@@ -325,6 +341,54 @@ static int sas_ata_soft_reset(struct ata_link *link, unsigned int *class, ...@@ -325,6 +341,54 @@ static int sas_ata_soft_reset(struct ata_link *link, unsigned int *class,
return ret; return ret;
} }
/*
* notify the lldd to forget the sas_task for this internal ata command
* that bypasses scsi-eh
*/
static void sas_ata_internal_abort(struct sas_task *task)
{
struct sas_internal *si =
to_sas_internal(task->dev->port->ha->core.shost->transportt);
unsigned long flags;
int res;
spin_lock_irqsave(&task->task_state_lock, flags);
if (task->task_state_flags & SAS_TASK_STATE_ABORTED ||
task->task_state_flags & SAS_TASK_STATE_DONE) {
spin_unlock_irqrestore(&task->task_state_lock, flags);
SAS_DPRINTK("%s: Task %p already finished.\n", __func__,
task);
goto out;
}
task->task_state_flags |= SAS_TASK_STATE_ABORTED;
spin_unlock_irqrestore(&task->task_state_lock, flags);
res = si->dft->lldd_abort_task(task);
spin_lock_irqsave(&task->task_state_lock, flags);
if (task->task_state_flags & SAS_TASK_STATE_DONE ||
res == TMF_RESP_FUNC_COMPLETE) {
spin_unlock_irqrestore(&task->task_state_lock, flags);
goto out;
}
/* XXX we are not prepared to deal with ->lldd_abort_task()
* failures. TODO: lldds need to unconditionally forget about
* aborted ata tasks, otherwise we (likely) leak the sas task
* here
*/
SAS_DPRINTK("%s: Task %p leaked.\n", __func__, task);
if (!(task->task_state_flags & SAS_TASK_STATE_DONE))
task->task_state_flags &= ~SAS_TASK_STATE_ABORTED;
spin_unlock_irqrestore(&task->task_state_lock, flags);
return;
out:
list_del_init(&task->list);
sas_free_task(task);
}
static void sas_ata_post_internal(struct ata_queued_cmd *qc) static void sas_ata_post_internal(struct ata_queued_cmd *qc)
{ {
if (qc->flags & ATA_QCFLAG_FAILED) if (qc->flags & ATA_QCFLAG_FAILED)
...@@ -332,10 +396,12 @@ static void sas_ata_post_internal(struct ata_queued_cmd *qc) ...@@ -332,10 +396,12 @@ static void sas_ata_post_internal(struct ata_queued_cmd *qc)
if (qc->err_mask) { if (qc->err_mask) {
/* /*
* Find the sas_task and kill it. By this point, * Find the sas_task and kill it. By this point, libata
* libata has decided to kill the qc, so we needn't * has decided to kill the qc and has frozen the port.
* bother with sas_ata_task_done. But we still * In this state sas_ata_task_done() will no longer free
* ought to abort the task. * the sas_task, so we need to notify the lldd (via
* ->lldd_abort_task) that the task is dead and free it
* ourselves.
*/ */
struct sas_task *task = qc->lldd_task; struct sas_task *task = qc->lldd_task;
unsigned long flags; unsigned long flags;
...@@ -348,7 +414,7 @@ static void sas_ata_post_internal(struct ata_queued_cmd *qc) ...@@ -348,7 +414,7 @@ static void sas_ata_post_internal(struct ata_queued_cmd *qc)
spin_unlock_irqrestore(&task->task_state_lock, flags); spin_unlock_irqrestore(&task->task_state_lock, flags);
task->uldd_task = NULL; task->uldd_task = NULL;
__sas_task_abort(task); sas_ata_internal_abort(task);
} }
} }
} }
......
...@@ -956,49 +956,6 @@ void sas_shutdown_queue(struct sas_ha_struct *sas_ha) ...@@ -956,49 +956,6 @@ void sas_shutdown_queue(struct sas_ha_struct *sas_ha)
spin_unlock_irqrestore(&core->task_queue_lock, flags); spin_unlock_irqrestore(&core->task_queue_lock, flags);
} }
/*
* Call the LLDD task abort routine directly. This function is intended for
* use by upper layers that need to tell the LLDD to abort a task.
*/
int __sas_task_abort(struct sas_task *task)
{
struct sas_internal *si =
to_sas_internal(task->dev->port->ha->core.shost->transportt);
unsigned long flags;
int res;
spin_lock_irqsave(&task->task_state_lock, flags);
if (task->task_state_flags & SAS_TASK_STATE_ABORTED ||
task->task_state_flags & SAS_TASK_STATE_DONE) {
spin_unlock_irqrestore(&task->task_state_lock, flags);
SAS_DPRINTK("%s: Task %p already finished.\n", __func__,
task);
return 0;
}
task->task_state_flags |= SAS_TASK_STATE_ABORTED;
spin_unlock_irqrestore(&task->task_state_lock, flags);
if (!si->dft->lldd_abort_task)
return -ENODEV;
res = si->dft->lldd_abort_task(task);
spin_lock_irqsave(&task->task_state_lock, flags);
if ((task->task_state_flags & SAS_TASK_STATE_DONE) ||
(res == TMF_RESP_FUNC_COMPLETE))
{
spin_unlock_irqrestore(&task->task_state_lock, flags);
task->task_done(task);
return 0;
}
if (!(task->task_state_flags & SAS_TASK_STATE_DONE))
task->task_state_flags &= ~SAS_TASK_STATE_ABORTED;
spin_unlock_irqrestore(&task->task_state_lock, flags);
return -EAGAIN;
}
/* /*
* Tell an upper layer that it needs to initiate an abort for a given task. * Tell an upper layer that it needs to initiate an abort for a given task.
* This should only ever be called by an LLDD. * This should only ever be called by an LLDD.
...@@ -1097,7 +1054,6 @@ EXPORT_SYMBOL_GPL(sas_slave_configure); ...@@ -1097,7 +1054,6 @@ EXPORT_SYMBOL_GPL(sas_slave_configure);
EXPORT_SYMBOL_GPL(sas_change_queue_depth); EXPORT_SYMBOL_GPL(sas_change_queue_depth);
EXPORT_SYMBOL_GPL(sas_change_queue_type); EXPORT_SYMBOL_GPL(sas_change_queue_type);
EXPORT_SYMBOL_GPL(sas_bios_param); EXPORT_SYMBOL_GPL(sas_bios_param);
EXPORT_SYMBOL_GPL(__sas_task_abort);
EXPORT_SYMBOL_GPL(sas_task_abort); EXPORT_SYMBOL_GPL(sas_task_abort);
EXPORT_SYMBOL_GPL(sas_phy_reset); EXPORT_SYMBOL_GPL(sas_phy_reset);
EXPORT_SYMBOL_GPL(sas_phy_enable); EXPORT_SYMBOL_GPL(sas_phy_enable);
......
...@@ -662,7 +662,6 @@ void sas_unregister_dev(struct asd_sas_port *port, struct domain_device *); ...@@ -662,7 +662,6 @@ void sas_unregister_dev(struct asd_sas_port *port, struct domain_device *);
void sas_init_dev(struct domain_device *); void sas_init_dev(struct domain_device *);
void sas_task_abort(struct sas_task *); void sas_task_abort(struct sas_task *);
int __sas_task_abort(struct sas_task *);
int sas_eh_device_reset_handler(struct scsi_cmnd *cmd); int sas_eh_device_reset_handler(struct scsi_cmnd *cmd);
int sas_eh_bus_reset_handler(struct scsi_cmnd *cmd); int sas_eh_bus_reset_handler(struct scsi_cmnd *cmd);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment