linux-scsi.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Rafael J. Wysocki" <rjw@sisk.pl>
To: James Bottomley <James.Bottomley@hansenpartnership.com>
Cc: Dan Williams <dan.j.williams@intel.com>,
	mroos@linux.ee, linux-scsi@vger.kernel.org,
	Mark Brown <broonie@opensource.wolfsonmicro.com>,
	linux-kernel@vger.kernel.org,
	Arjan van de Ven <arjan@linux.intel.com>,
	Liam Girdwood <lrg@ti.com>
Subject: Re: [PATCH 1/4] async: introduce 'async_domain' type
Date: Fri, 25 May 2012 21:23:27 +0200	[thread overview]
Message-ID: <201205252123.27869.rjw@sisk.pl> (raw)
In-Reply-To: <1337935700.2932.18.camel@dabdike.int.hansenpartnership.com>

On Friday, May 25, 2012, James Bottomley wrote:
> On Fri, 2012-05-25 at 01:18 -0700, Dan Williams wrote:
> > On Fri, May 25, 2012 at 12:51 AM, James Bottomley
> > <James.Bottomley@hansenpartnership.com> wrote:
> > > On Fri, 2012-05-25 at 00:50 -0700, Dan Williams wrote:
> > >> This is in preparation for teaching async_synchronize_full() to sync all
> > >> pending async work, and not just on the async_running domain.  This
> > >> conversion is functionally equivalent, just embedding the existing list
> > >> in a new async_domain type.
> > >
> > > This looks good, but I want Arjan and others who invented the async code
> > > to speed up boot to comment on all of this.  What was the intention of
> > > async_synchronize_full() and if it wasn't to synchronise all domains,
> > > should we fix the documentation and add a new primitive to do that,
> > > since boot clearly assumes the all domains behaviour.
> > >
> > > In the mean time, this is probably all a bit much for a merge window, so
> > > I'll revert
> > >
> > > commit a7a20d103994fd760766e6c9d494daa569cbfe06
> > > Author: Dan Williams <dan.j.williams@intel.com>
> > > Date:   Thu Mar 22 17:05:11 2012 -0700
> > >
> > >    [SCSI] sd: limit the scope of the async probe domain
> > >
> > > And we'll put whatever is chosen in early for the next merge window.
> > >
> > 
> > Makes sense... but could also go ahead with the smaller fix I posted
> > for 3.5.  Meelis confirms it is working.
> 
> OK, that's what I hadn't seen.  I can't think of another way we could
> fail at the moment, except in suspend/resume because the
> scsi_complete_async_scans will be a nop. Can someone test the
> suspend/resume case?
> 
> 
> There is actually one good thing to come out of this:  Rafael's commit
> 
> commit c751085943362143f84346d274e0011419c84202
> Author: Rafael J. Wysocki <rjw@sisk.pl>
> Date:   Sun Apr 12 20:06:56 2009 +0200
> 
>     PM/Hibernate: Wait for SCSI devices scan to complete during resume
> 
> Actually broke the scsi_wait_scan module, because for modular SCSI
> (which is effectively all distributions) its scsi_complete_async_scans()
> is also a nop.  I assume this means that no distributions rely on it any
> more and we can remove it?

Well, there still are users who build their own kernels ...

The problem was that when we had started to do the async scan, resume from
hibernation stopped working for the poeple who _had_ built-in SCSI and the
commit above was meant to address that.

Thanks,
Rafael

  reply	other threads:[~2012-05-25 19:23 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-05-25  7:50 [RFT PATCH 0/4] fix / cleanup async scsi scanning Dan Williams
2012-05-25  7:50 ` [PATCH 1/4] async: introduce 'async_domain' type Dan Williams
2012-05-25  7:51   ` James Bottomley
2012-05-25  8:18     ` Dan Williams
2012-05-25  8:48       ` James Bottomley
2012-05-25 19:23         ` Rafael J. Wysocki [this message]
2012-05-25 13:31     ` Arjan van de Ven
2012-05-25 13:40     ` mroos
2012-05-25 15:05       ` Dan Williams
2012-05-27 22:34   ` Mark Brown
2012-05-25  7:50 ` [PATCH 2/4] async: make async_synchronize_full() flush all work regardless of domain Dan Williams
2012-05-25  7:50 ` [PATCH 3/4] scsi: queue async scan work to an async_schedule domain Dan Williams
2012-05-25  7:50 ` [PATCH 4/4] scsi: cleanup usages of scsi_complete_async_scans Dan Williams

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=201205252123.27869.rjw@sisk.pl \
    --to=rjw@sisk.pl \
    --cc=James.Bottomley@hansenpartnership.com \
    --cc=arjan@linux.intel.com \
    --cc=broonie@opensource.wolfsonmicro.com \
    --cc=dan.j.williams@intel.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-scsi@vger.kernel.org \
    --cc=lrg@ti.com \
    --cc=mroos@linux.ee \
    /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).