linux-scsi.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Todd E Brandt <todd.e.brandt@linux.intel.com>
To: Dan Williams <dan.j.williams@intel.com>
Cc: tj@kernel.org, James.Bottomley@hansenpartnership.com,
	linux-ide@vger.kernel.org, linux-scsi@vger.kernel.org,
	psusi@ubuntu.com
Subject: Re: [PATCH/RESEND v2 1/2] Hard disk S3 resume time optimization
Date: Mon, 13 Jan 2014 12:06:48 -0800	[thread overview]
Message-ID: <20140113200648.GB13900@linux.intel.com> (raw)
In-Reply-To: <CAA9_cmdkNNfE8-b60x4ULw6OA+zxrVZoJhEX3a5XZOQKnsqfHg@mail.gmail.com>

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.

  reply	other threads:[~2014-01-13 20:06 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-01-08  0:56 [PATCH/RESEND v2 1/2] Hard disk S3 resume time optimization Todd E Brandt
2014-01-09 17:03 ` Phillip Susi
2014-01-10 23:11   ` Brandt, Todd E
2014-01-11  2:13     ` Phillip Susi
2014-01-11  3:13       ` Dan Williams
2014-01-13 20:06         ` Todd E Brandt [this message]
2014-01-13 20:37           ` Dan Williams
2014-01-13 23:51             ` Todd E Brandt
2014-01-14  0:05               ` Dan Williams
2014-01-11 19:13 ` Tejun Heo
2014-01-13 19:55   ` Todd E Brandt
2014-01-13 20:30     ` Tejun Heo
2014-01-13 23:30       ` Todd E Brandt
2014-01-14 14:31         ` Tejun Heo
2014-01-15  0:31       ` [PATCH v3 0/2] " Todd E Brandt
2014-01-15 12:55         ` Tejun Heo
2014-01-15  0:31       ` [PATCH v3 1/2] " Todd E Brandt
2014-01-15 13:01         ` Tejun Heo
2014-01-15 20:04           ` Todd E Brandt
2014-01-15  0:32       ` [PATCH v3 2/2] " Todd E Brandt

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20140113200648.GB13900@linux.intel.com \
    --to=todd.e.brandt@linux.intel.com \
    --cc=James.Bottomley@hansenpartnership.com \
    --cc=dan.j.williams@intel.com \
    --cc=linux-ide@vger.kernel.org \
    --cc=linux-scsi@vger.kernel.org \
    --cc=psusi@ubuntu.com \
    --cc=tj@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).