Re: Fwd: Waking up from resume locks up on sr device

[Hannes Reinecke]

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@xxxxxxx                              +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