| Message ID | 20180614161809.uskdb6unpuk4uzoj@linutronix.de (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
On Thu, Jun 14, 2018 at 9:18 AM, Sebastian Andrzej Siewior <bigeasy@linutronix.de> wrote: > Since commit 312d3e56119a ("[SCSI] libsas: remove ata_port.lock > management duties from lldds") the sas_ata_qc_issue() function unlocks > the ata_port.lock and disables interrupts before doing so. > That lock is always taken with disabled interrupts so at this point, the > interrupts are already disabled. There is no need to disable the > interrupts before the unlock operation because they are already > disabled. > Restoring the interrupt state later does not change anything because > they were disabled and remain disabled. Therefore remove the operations > which do not change the behaviour. > > Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de> Acked-by: Dan Williams <dan.j.williams@intel.com>
nit: scsi: libsas: remove irq save in sas_ata_qc_issue() On 14/06/2018 17:18, Sebastian Andrzej Siewior wrote: > Since commit 312d3e56119a ("[SCSI] libsas: remove ata_port.lock > management duties from lldds") the sas_ata_qc_issue() function unlocks > the ata_port.lock and disables interrupts before doing so. > That lock is always taken with disabled interrupts so at this point, the > interrupts are already disabled. There is no need to disable the > interrupts before the unlock operation because they are already > disabled. > Restoring the interrupt state later does not change anything because > they were disabled and remain disabled. Therefore remove the operations > which do not change the behaviour. > > Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de> Reviewed-by: John Garry <john.garry@huawei.com>
Sebastian, > Since commit 312d3e56119a ("[SCSI] libsas: remove ata_port.lock > management duties from lldds") the sas_ata_qc_issue() function unlocks > the ata_port.lock and disables interrupts before doing so. That lock > is always taken with disabled interrupts so at this point, the > interrupts are already disabled. There is no need to disable the > interrupts before the unlock operation because they are already > disabled. Restoring the interrupt state later does not change > anything because they were disabled and remain disabled. Therefore > remove the operations which do not change the behaviour. Applied to 4.19/scsi-queue. Thank you!
On 2018-06-14 21:59:18 [-0400], Martin K. Petersen wrote: > > Sebastian, Martin, > Applied to 4.19/scsi-queue. Thank you! Thank you. Sebastian
diff --git a/drivers/scsi/libsas/sas_ata.c b/drivers/scsi/libsas/sas_ata.c index ff1d612f6fb9..2ac7395112b4 100644 --- a/drivers/scsi/libsas/sas_ata.c +++ b/drivers/scsi/libsas/sas_ata.c @@ -176,7 +176,6 @@ static void sas_ata_task_done(struct sas_task *task) static unsigned int sas_ata_qc_issue(struct ata_queued_cmd *qc) { - unsigned long flags; struct sas_task *task; struct scatterlist *sg; int ret = AC_ERR_SYSTEM; @@ -187,10 +186,7 @@ static unsigned int sas_ata_qc_issue(struct ata_queued_cmd *qc) struct Scsi_Host *host = sas_ha->core.shost; struct sas_internal *i = to_sas_internal(host->transportt); - /* TODO: audit callers to ensure they are ready for qc_issue to - * unconditionally re-enable interrupts - */ - local_irq_save(flags); + /* TODO: we should try to remove that unlock */ spin_unlock(ap->lock); /* If the device fell off, no sense in issuing commands */ @@ -252,7 +248,6 @@ static unsigned int sas_ata_qc_issue(struct ata_queued_cmd *qc) out: spin_lock(ap->lock); - local_irq_restore(flags); return ret; }
Since commit 312d3e56119a ("[SCSI] libsas: remove ata_port.lock management duties from lldds") the sas_ata_qc_issue() function unlocks the ata_port.lock and disables interrupts before doing so. That lock is always taken with disabled interrupts so at this point, the interrupts are already disabled. There is no need to disable the interrupts before the unlock operation because they are already disabled. Restoring the interrupt state later does not change anything because they were disabled and remain disabled. Therefore remove the operations which do not change the behaviour. Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de> --- v1…v2: Updating comment as suggested by Dan Williams. drivers/scsi/libsas/sas_ata.c | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-)