From cb7fc545352947ccb5d746a3baf2e8ae67c694ad Mon Sep 17 00:00:00 2001 From: Jason Yan Date: Wed, 3 Apr 2019 21:43:06 +0800 Subject: [PATCH] ahci: prevent freezing port when EH is running euler inclusion category: bugfix bugzilla: NA CVE: NA --------------------------- Trinity report a warning for this patch: WARNING: CPU: 1 PID: 118 at ../drivers/ata/libata-eh.c:4016 ata_eh_finish+0x15a/0x170 Fixing the race condition between EH and interrupt by making the EH thread re-enter again is a little overkill and IO will get through after the scsi_run_host_queues() and before SHOST_RECOVERY is set agian in scsi_restart_operations(). If EH thread is already running, no need to freeze port and schedule EH again. Fixes: a7d2fef75b83 ("scsi: ata: Fix a race condition between scsi error handler and ahci interrupt") Signed-off-by: Jason Yan Reviewed-by: zhengbin Signed-off-by: Yang Yingliang --- drivers/ata/libahci.c | 12 ++++++++++-- drivers/ata/libata-eh.c | 2 +- drivers/scsi/libsas/sas_scsi_host.c | 4 ++-- drivers/scsi/scsi_error.c | 10 ++++------ drivers/scsi/scsi_lib.c | 3 +-- include/scsi/scsi_host.h | 2 +- 6 files changed, 19 insertions(+), 14 deletions(-) diff --git a/drivers/ata/libahci.c b/drivers/ata/libahci.c index b5f57c69c487..2f5e00c0cf4d 100644 --- a/drivers/ata/libahci.c +++ b/drivers/ata/libahci.c @@ -1805,9 +1805,17 @@ static void ahci_error_intr(struct ata_port *ap, u32 irq_stat) /* okay, let's hand over to EH */ - if (irq_stat & PORT_IRQ_FREEZE) + if (irq_stat & PORT_IRQ_FREEZE) { + /* + * EH already running, this may happen if the port is + * thawed in the EH. But we cannot freeze it again + * otherwise the port will never be thawed. + */ + if (ap->pflags & (ATA_PFLAG_EH_PENDING | + ATA_PFLAG_EH_IN_PROGRESS)) + return; ata_port_freeze(ap); - else if (fbs_need_dec) { + } else if (fbs_need_dec) { ata_link_abort(link); ahci_fbs_dec_intr(ap); } else diff --git a/drivers/ata/libata-eh.c b/drivers/ata/libata-eh.c index e4a781d9b58f..01306c018398 100644 --- a/drivers/ata/libata-eh.c +++ b/drivers/ata/libata-eh.c @@ -974,7 +974,7 @@ void ata_std_end_eh(struct ata_port *ap) { struct Scsi_Host *host = ap->scsi_host; - atomic_dec(&host->host_eh_scheduled); + host->host_eh_scheduled = 0; } EXPORT_SYMBOL(ata_std_end_eh); diff --git a/drivers/scsi/libsas/sas_scsi_host.c b/drivers/scsi/libsas/sas_scsi_host.c index 96f0ac4c913f..4eeaafbe0504 100644 --- a/drivers/scsi/libsas/sas_scsi_host.c +++ b/drivers/scsi/libsas/sas_scsi_host.c @@ -492,7 +492,7 @@ int sas_eh_abort_handler(struct scsi_cmnd *cmd) spin_lock_irqsave(host->host_lock, flags); /* We cannot do async aborts for SATA devices */ - if (dev_is_sata(dev) && !atomic_read(&host->host_eh_scheduled)) { + if (dev_is_sata(dev) && !host->host_eh_scheduled) { spin_unlock_irqrestore(host->host_lock, flags); return FAILED; } @@ -792,7 +792,7 @@ void sas_scsi_recover_host(struct Scsi_Host *shost) /* check if any new eh work was scheduled during the last run */ spin_lock_irq(&ha->lock); if (ha->eh_active == 0) { - atomic_set(&shost->host_eh_scheduled, 0); + shost->host_eh_scheduled = 0; retry = false; } spin_unlock_irq(&ha->lock); diff --git a/drivers/scsi/scsi_error.c b/drivers/scsi/scsi_error.c index 55f12ecaf400..c736d61b1648 100644 --- a/drivers/scsi/scsi_error.c +++ b/drivers/scsi/scsi_error.c @@ -88,7 +88,7 @@ void scsi_schedule_eh(struct Scsi_Host *shost) if (scsi_host_set_state(shost, SHOST_RECOVERY) == 0 || scsi_host_set_state(shost, SHOST_CANCEL_RECOVERY) == 0) { - atomic_inc(&shost->host_eh_scheduled); + shost->host_eh_scheduled++; scsi_eh_wakeup(shost); } @@ -2029,7 +2029,7 @@ static void scsi_restart_operations(struct Scsi_Host *shost) * pending commands complete. */ spin_lock_irqsave(shost->host_lock, flags); - if (atomic_read(&shost->host_eh_scheduled)) + if (shost->host_eh_scheduled) if (scsi_host_set_state(shost, SHOST_RECOVERY)) WARN_ON(scsi_host_set_state(shost, SHOST_CANCEL_RECOVERY)); spin_unlock_irqrestore(shost->host_lock, flags); @@ -2165,8 +2165,7 @@ int scsi_error_handler(void *data) if (kthread_should_stop()) break; - if ((shost->host_failed == 0 && - atomic_read(&shost->host_eh_scheduled) == 0) || + if ((shost->host_failed == 0 && shost->host_eh_scheduled == 0) || shost->host_failed != scsi_host_busy(shost)) { SCSI_LOG_ERROR_RECOVERY(1, shost_printk(KERN_INFO, shost, @@ -2180,8 +2179,7 @@ int scsi_error_handler(void *data) SCSI_LOG_ERROR_RECOVERY(1, shost_printk(KERN_INFO, shost, "scsi_eh_%d: waking up %d/%d/%d\n", - shost->host_no, - atomic_read(&shost->host_eh_scheduled), + shost->host_no, shost->host_eh_scheduled, shost->host_failed, scsi_host_busy(shost))); diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c index 37237010953f..4a3ef1a70ed7 100644 --- a/drivers/scsi/scsi_lib.c +++ b/drivers/scsi/scsi_lib.c @@ -348,8 +348,7 @@ static void scsi_dec_host_busy(struct Scsi_Host *shost) atomic_dec(&shost->host_busy); if (unlikely(scsi_host_in_recovery(shost))) { spin_lock_irqsave(shost->host_lock, flags); - if (shost->host_failed || - atomic_read(&shost->host_eh_scheduled)) + if (shost->host_failed || shost->host_eh_scheduled) scsi_eh_wakeup(shost); spin_unlock_irqrestore(shost->host_lock, flags); } diff --git a/include/scsi/scsi_host.h b/include/scsi/scsi_host.h index 5f0fa2296866..5c1df26e1d4f 100644 --- a/include/scsi/scsi_host.h +++ b/include/scsi/scsi_host.h @@ -563,7 +563,7 @@ struct Scsi_Host { unsigned int host_failed; /* commands that failed. protected by host_lock */ - atomic_t host_eh_scheduled; /* EH scheduled without command */ + unsigned int host_eh_scheduled; /* EH scheduled without command */ unsigned int host_no; /* Used for IOCTL_GET_IDLUN, /proc/scsi et al. */ -- GitLab