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

[Dan Williams <dan.j.williams@intel.com>]

On Mon, Jan 13, 2014 at 12:06 PM, Todd E Brandt
<todd.e.brandt@linux.intel.com> wrote:
> On Fri, Jan 10, 2014 at 07:13:11PM -0800, Dan Williams wrote:
>> On Fri, Jan 10, 2014 at 6:13 PM, Phillip Susi <psusi@ubuntu.com> wrote:
>> > -----BEGIN PGP SIGNED MESSAGE-----
>> > Hash: SHA512
>> >
>> > On 01/10/2014 06:11 PM, Brandt, Todd E wrote:
>> >> Yes yours is simpler, but it also opens a potential memory issue
>> >> by passing a static int as the return location for the error value.
>> >> I think it's just safer to tell the callback to attempt no return
>> >> value at all, and for that you need to expand it into two
>> >> arguments, one for selection, the other for the output address.
>> >
>> > What sort of memory issue?  Also isn't there a system NULL page
>> > somewhere that could be used?
>> >
>>
>> I think the static variable is ok.  We can be sure that all eh threads
>> are torn down before libata.ko is unloaded.
>
> 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
suspends are synchronous I do not think we have the reverse problem of
resume requests arriving while a suspend is in flight.  However, it
might be worth a WARN_ON_ONCE() to document that assumption.

In the libsas case suspends are asynchronous, but they are flushed by
libsas before any resumes are processed, so there should not be
conflicts.