linux-kernel - Re: Fwd: Waking up from resume locks up on sr device

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <37ed36f0-6f72-115c-85fb-62ef5ad72e76@suse.de>
Date:   Wed, 14 Jun 2023 08:57:12 +0200
From:   Hannes Reinecke <hare@...e.de>
To:     Damien Le Moal <dlemoal@...nel.org>,
        Joe Breuer <linux-kernel@...reuer.net>,
        Bart Van Assche <bvanassche@....org>,
        Bagas Sanjaya <bagasdotme@...il.com>,
        Pavel Machek <pavel@....cz>
Cc:     "Rafael J. Wysocki" <rafael@...nel.org>,
        Len Brown <len.brown@...el.com>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Kees Cook <keescook@...omium.org>,
        Tony Luck <tony.luck@...el.com>,
        "Guilherme G. Piccoli" <gpiccoli@...lia.com>,
        Thorsten Leemhuis <linux@...mhuis.info>,
        "James E.J. Bottomley" <jejb@...ux.ibm.com>,
        "Martin K. Petersen" <martin.petersen@...cle.com>,
        Phillip Potter <phil@...lpotter.co.uk>,
        Linux Power Management <linux-pm@...r.kernel.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Linux Hardening <linux-hardening@...r.kernel.org>,
        Linux Regressions <regressions@...ts.linux.dev>,
        Linux SCSI <linux-scsi@...r.kernel.org>,
        Alan Stern <stern@...land.harvard.edu>,
        Dan Williams <dan.j.williams@...el.com>,
        Hannes Reinecke <hare@...e.com>,
        Adrian Hunter <adrian.hunter@...el.com>,
        Martin Kepplinger <martin.kepplinger@...i.sm>,
        Kai-Heng Feng <kai.heng.feng@...onical.com>
Subject: Re: Fwd: Waking up from resume locks up on sr device

On 6/14/23 06:49, Damien Le Moal wrote:
> On 6/11/23 18:05, Joe Breuer wrote:
>> I'm the reporter of this issue.
>>
>> I just tried this patch against 6.3.4, and it completely fixes my
>> suspend/resume issue.
>>
>> The optical drive stays usable after resume, even suspending/resuming
>> during playback of CDDA content works flawlessly and playback resumes
>> seamlessly after system resume.
>>
>> So, from my perspective: Good one!
> 
> In place of Bart's fix, could you please try this patch ?
> 
> diff --git a/drivers/ata/libata-eh.c b/drivers/ata/libata-eh.c
> index b80e68000dd3..a81eb4f882ab 100644
> --- a/drivers/ata/libata-eh.c
> +++ b/drivers/ata/libata-eh.c
> @@ -4006,9 +4006,32 @@ static void ata_eh_handle_port_resume(struct
> ata_port *ap)
>          /* tell ACPI that we're resuming */
>          ata_acpi_on_resume(ap);
> 
> -       /* update the flags */
>          spin_lock_irqsave(ap->lock, flags);
> +
> +       /* Update the flags */
>          ap->pflags &= ~(ATA_PFLAG_PM_PENDING | ATA_PFLAG_SUSPENDED);
> +
> +       /*
> +        * Resuming the port will trigger a rescan of the ATA device(s)
> +        * connected to it. Before scheduling the rescan, make sure that
> +        * the associated scsi device(s) are fully resumed as well.
> +        */
> +       ata_for_each_link(link, ap, HOST_FIRST) {
> +               ata_for_each_dev(dev, link, ENABLED) {
> +                       struct scsi_device *sdev = dev->sdev;
> +
> +                       if (!sdev)
> +                               continue;
> +                       if (scsi_device_get(sdev))
> +                               continue;
> +
> +                       spin_unlock_irqrestore(ap->lock, flags);
> +                       device_pm_wait_for_dev(&ap->tdev,
> +                                              &sdev->sdev_gendev);
> +                       scsi_device_put(sdev);
> +                       spin_lock_irqsave(ap->lock, flags);
> +               }
> +       }
>          spin_unlock_irqrestore(ap->lock, flags);
>   }
>   #endif /* CONFIG_PM */
> 
> Thanks !
> 
Well; not sure if that'll work out.
The whole reason why we initial a rescan is that we need to check if the 
ports are still connected, and whether the devices react.
So we can't iterate the ports here as this is the very thing which gets 
checked during EH.

We really should claim resume to be finished as soon as we can talk with 
the HBA, and kick off EH asynchronously to let it finish the job after 
resume has completed.

Cheers,

Hannes
-- 
Dr. Hannes Reinecke                Kernel Storage Architect
hare@...e.de                              +49 911 74053 688
SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), Geschäftsführer: Ivo Totev, Andrew
Myers, Andrew McDonald, Martje Boudien Moerman