Re: [PATCH/RESEND v2 1/2] Hard disk S3 resume time optimization

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

 



On Mon, Jan 13, 2014 at 3:51 PM, Todd E Brandt
<todd.e.brandt@xxxxxxxxxxxxxxx> wrote:
> On Mon, Jan 13, 2014 at 12:37:01PM -0800, Dan Williams wrote:
>> On Mon, Jan 13, 2014 at 12:06 PM, Todd E Brandt
>> > Actually there's one other reason. In the ata_port_request_pm function it
>> > checks to see if there's a previous resume operation pending, and if so
>> > it calls ata_port_wait_eh in order to wait for it to complete before
>> > issuing the new suspend. If you just use the (int*)async parameter it
>> > will return immediately and defer to the caller to try again, like is does
>> > with SAS. But in our case we *don't* try again, so it would result in the
>> > resume being skipped. There needs to be a new case where the caller wants
>> > the call to be asynchronous, and it wants ata_port_request_pm to do its
>> > own waiting, but doesn't care about the return value. Thus the additional
>> > parameter.
>>
>> I think that is specifically for the libata case of a suspend request
>> arriving while an async resume is still in flight.  Given libata
>
> Accoring to the comments it's for a previous resume, not a previous suspend.
>
>         /* Previous resume operation might still be in
>          * progress.  Wait for PM_PENDING to clear.
>          */
>         if (ap->pflags & ATA_PFLAG_PM_PENDING) {
>                 if (async) {
>                         *async = -EAGAIN;
>                         return 0;
>                 }
>                 ata_port_wait_eh(ap);
>                 WARN_ON(ap->pflags & ATA_PFLAG_PM_PENDING);
>         }

Right, but that should only happen when ata_port_request_pm() is
called for 'suspend' and 'resume' is in flight.  I think it's a core
device-power-management bug if ->resume() is called twice without an
intervening ->suspend().

> I'm going to assume that it was added for a reason, so I've structured my
> patch in such a way that it doesn't alter the existing logic. Removing that
> particular check would be a completely different discussion and is out of
> the scope of this patch.

You're right that logic should stay, I don't think removing it was
ever proposed?
--
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[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