Re: [PATCH v4 01/23] ata: libata-core: Fix ata_port_request_pm() locking

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, Sep 20, 2023 at 10:54:17PM +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 scheduled
> 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. The two WARN_ON() macros checking
> that the ATA_PFLAG_PM_PENDING flag was cleared are removed as the first
> call is racy and the second one done without holding the port lock.
> 
> Fixes: 5ef41082912b ("ata: add ata port system PM callbacks")
> Cc: stable@xxxxxxxxxxxxxxx
> Signed-off-by: Damien Le Moal <dlemoal@xxxxxxxxxx>
> Reviewed-by: Hannes Reinecke <hare@xxxxxxx>
> Tested-by: Chia-Lin Kao (AceLan) <acelan.kao@xxxxxxxxxxxxx>
> ---
>  drivers/ata/libata-core.c | 18 +++++++++---------
>  1 file changed, 9 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
> index 0072e0f9ad39..732f3d0b4fd9 100644
> --- a/drivers/ata/libata-core.c
> +++ b/drivers/ata/libata-core.c
> @@ -5037,17 +5037,19 @@ 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);
> -		WARN_ON(ap->pflags & ATA_PFLAG_PM_PENDING);
> +		spin_lock_irqsave(ap->lock, flags);
>  	}
>  
> -	/* 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) {
> @@ -5059,10 +5061,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);
> -	}
>  }
>  
>  /*
> -- 
> 2.41.0
> 

Reviewed-by: Niklas Cassel <niklas.cassel@xxxxxxx>



[Index of Archives]     [Linux Filesystems]     [Linux SCSI]     [Linux RAID]     [Git]     [Kernel Newbies]     [Linux Newbie]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Samba]     [Device Mapper]

  Powered by Linux