diff options
author | Christoph Hellwig <hch@lst.de> | 2015-10-08 10:25:41 +0200 |
---|---|---|
committer | Tejun Heo <tj@kernel.org> | 2015-10-12 13:23:13 -0400 |
commit | 2aa8f5d63d9183ab185364d4ce849566ede0f301 (patch) | |
tree | 75e01ce75cac1abcb883847c18500da83bdef645 /drivers/ata | |
parent | 25c7ce7f7b511fc02b6eef95595af2cea80e2612 (diff) | |
download | linux-2aa8f5d63d9183ab185364d4ce849566ede0f301.tar.bz2 |
libata: only call ->done once all per-tag ressources are released
When calling ->done before releasing resources we could run into a
race where the SCSI midlayer sends another command and races with
the resources beeing manipulated. For libata this can't currently
happen as synchronization happens at a higher level, but I'd still
like to fix it to future proof libata and to avoid copy & paste
into SCSI drivers where this pattern has led to reproducible crashes.
Signed-off-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Tejun Heo <tj@kernel.org>
Diffstat (limited to 'drivers/ata')
-rw-r--r-- | drivers/ata/libata-scsi.c | 22 |
1 files changed, 13 insertions, 9 deletions
diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c index ebcba8504ac0..560cedea2b40 100644 --- a/drivers/ata/libata-scsi.c +++ b/drivers/ata/libata-scsi.c @@ -1757,6 +1757,15 @@ nothing_to_do: return 1; } +static void ata_qc_done(struct ata_queued_cmd *qc) +{ + struct scsi_cmnd *cmd = qc->scsicmd; + void (*done)(struct scsi_cmnd *) = qc->scsidone; + + ata_qc_free(qc); + done(cmd); +} + static void ata_scsi_qc_complete(struct ata_queued_cmd *qc) { struct ata_port *ap = qc->ap; @@ -1784,9 +1793,7 @@ static void ata_scsi_qc_complete(struct ata_queued_cmd *qc) if (need_sense && !ap->ops->error_handler) ata_dump_status(ap->print_id, &qc->result_tf); - qc->scsidone(cmd); - - ata_qc_free(qc); + ata_qc_done(qc); } /** @@ -2588,8 +2595,7 @@ static void atapi_sense_complete(struct ata_queued_cmd *qc) ata_gen_passthru_sense(qc); } - qc->scsidone(qc->scsicmd); - ata_qc_free(qc); + ata_qc_done(qc); } /* is it pointless to prefer PIO for "safety reasons"? */ @@ -2684,8 +2690,7 @@ static void atapi_qc_complete(struct ata_queued_cmd *qc) qc->dev->sdev->locked = 0; qc->scsicmd->result = SAM_STAT_CHECK_CONDITION; - qc->scsidone(cmd); - ata_qc_free(qc); + ata_qc_done(qc); return; } @@ -2729,8 +2734,7 @@ static void atapi_qc_complete(struct ata_queued_cmd *qc) cmd->result = SAM_STAT_GOOD; } - qc->scsidone(cmd); - ata_qc_free(qc); + ata_qc_done(qc); } /** * atapi_xlat - Initialize PACKET taskfile |