| Message ID | 20230911040217.253905-2-dlemoal@kernel.org |
|---|---|
| State | New |
| Headers | show |
| Series | Fix libata suspend/resume handling and code cleanup | expand |
On 9/11/23 06:01, Damien Le Moal wrote: > The function ata_port_request_pm() checks the port flag > ATA_PFLAG_PM_PENDING and calls ata_port_wait_eh() if this flag is set to > ensure that power management operations for a port are not secheduled > simultaneously. However, this flag check is done without holding the > port lock. > > Fix this by taking the port lock on entry to the function and checking > the flag under this lock. The lock is released and re-taken if > ata_port_wait_eh() needs to be called. > > Fixes: 5ef41082912b ("ata: add ata port system PM callbacks") > Cc: stable@vger.kernel.org > Signed-off-by: Damien Le Moal <dlemoal@kernel.org> > --- > drivers/ata/libata-core.c | 17 +++++++++-------- > 1 file changed, 9 insertions(+), 8 deletions(-) > Reviewed-by: Hannes Reinecke <hare@suse.de> Cheers, Hannes
On Mon, Sep 11, 2023 at 01:01:59PM +0900, Damien Le Moal wrote: > The function ata_port_request_pm() checks the port flag > ATA_PFLAG_PM_PENDING and calls ata_port_wait_eh() if this flag is set to > ensure that power management operations for a port are not secheduled > simultaneously. However, this flag check is done without holding the > port lock. > > Fix this by taking the port lock on entry to the function and checking > the flag under this lock. The lock is released and re-taken if > ata_port_wait_eh() needs to be called. > > Fixes: 5ef41082912b ("ata: add ata port system PM callbacks") > Cc: stable@vger.kernel.org > Signed-off-by: Damien Le Moal <dlemoal@kernel.org> Tested-by: Chia-Lin Kao (AceLan) <acelan.kao@canonical.com>
diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c index 74314311295f..c4898483d716 100644 --- a/drivers/ata/libata-core.c +++ b/drivers/ata/libata-core.c @@ -5040,17 +5040,20 @@ static void ata_port_request_pm(struct ata_port *ap, pm_message_t mesg, struct ata_link *link; unsigned long flags; - /* Previous resume operation might still be in - * progress. Wait for PM_PENDING to clear. + spin_lock_irqsave(ap->lock, flags); + + /* + * A previous PM operation might still be in progress. Wait for + * ATA_PFLAG_PM_PENDING to clear. */ if (ap->pflags & ATA_PFLAG_PM_PENDING) { + spin_unlock_irqrestore(ap->lock, flags); ata_port_wait_eh(ap); + spin_lock_irqsave(ap->lock, flags); WARN_ON(ap->pflags & ATA_PFLAG_PM_PENDING); } - /* request PM ops to EH */ - spin_lock_irqsave(ap->lock, flags); - + /* Request PM operation to EH */ ap->pm_mesg = mesg; ap->pflags |= ATA_PFLAG_PM_PENDING; ata_for_each_link(link, ap, HOST_FIRST) { @@ -5062,10 +5065,8 @@ static void ata_port_request_pm(struct ata_port *ap, pm_message_t mesg, spin_unlock_irqrestore(ap->lock, flags); - if (!async) { + if (!async) ata_port_wait_eh(ap); - WARN_ON(ap->pflags & ATA_PFLAG_PM_PENDING); - } } /*
The function ata_port_request_pm() checks the port flag ATA_PFLAG_PM_PENDING and calls ata_port_wait_eh() if this flag is set to ensure that power management operations for a port are not secheduled simultaneously. However, this flag check is done without holding the port lock. Fix this by taking the port lock on entry to the function and checking the flag under this lock. The lock is released and re-taken if ata_port_wait_eh() needs to be called. Fixes: 5ef41082912b ("ata: add ata port system PM callbacks") Cc: stable@vger.kernel.org Signed-off-by: Damien Le Moal <dlemoal@kernel.org> --- drivers/ata/libata-core.c | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-)