Commit a7a20d10 "[SCSI] sd: limit the scope of the async probe domain"
introduces a boot regression by moving sd probe work off of the global
async queue. Using a local async domain hides the probe work from being
synchronized by wait_for_device_probe()->async_synchronize_full().
Fix this by teaching async_synchronize_full() to flush all async work
regardless of domain, and take the opportunity to convert scsi scanning
to async_schedule(). This enables wait_for_device_probe() to flush scsi
scanning work.
Lightly boot tested, Meelis does this fix your regression?
Thanks for your help.
--
Dan
---
Dan Williams (4):
async: introduce 'async_domain' type
async: make async_synchronize_full() flush all work regardless of domain
scsi: queue async scan work to an async_schedule domain
scsi: cleanup usages of scsi_complete_async_scans
drivers/regulator/core.c | 2 +-
drivers/scsi/libsas/sas_ata.c | 2 +-
drivers/scsi/scsi.c | 3 ++-
drivers/scsi/scsi_priv.h | 3 ++-
drivers/scsi/scsi_scan.c | 24 ++++++------------------
drivers/scsi/scsi_wait_scan.c | 15 +++++----------
include/linux/async.h | 20 ++++++++++++++++----
include/scsi/scsi_scan.h | 11 -----------
kernel/async.c | 36 ++++++++++++++++++++----------------
kernel/power/hibernate.c | 8 --------
kernel/power/user.c | 2 --
11 files changed, 53 insertions(+), 73 deletions(-)
delete mode 100644 include/scsi/scsi_scan.h
In response to an async related regression James noted:
"My theory is that this is an init problem: The assumption in a lot of
our code is that async_synchronize_full() waits for everything ... even
the domain specific async schedules, which isn't true."
...so make this assumption true.
Each domain, including the default one, registers itself on a global domain
list when work is scheduled. Once all entries complete it exits that
list. Waiting for the list to be empty syncs all in-flight work across
all domains.
Cc: Arjan van de Ven <[email protected]>
Cc: Len Brown <[email protected]>
Cc: Rafael J. Wysocki <[email protected]>
Cc: James Bottomley <[email protected]>
Signed-off-by: Dan Williams <[email protected]>
---
kernel/async.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/kernel/async.c b/kernel/async.c
index aa23eec..f7d70b1 100644
--- a/kernel/async.c
+++ b/kernel/async.c
@@ -63,6 +63,7 @@ static async_cookie_t next_cookie = 1;
static LIST_HEAD(async_pending);
static ASYNC_DOMAIN(async_running);
+static LIST_HEAD(async_domains);
static DEFINE_SPINLOCK(async_lock);
struct async_entry {
@@ -145,6 +146,8 @@ static void async_run_entry_fn(struct work_struct *work)
/* 3) remove self from the running queue */
spin_lock_irqsave(&async_lock, flags);
list_del(&entry->list);
+ if (--running->count == 0)
+ list_del_init(&running->node);
/* 4) free the entry */
kfree(entry);
@@ -187,6 +190,8 @@ static async_cookie_t __async_schedule(async_func_ptr *ptr, void *data, struct a
spin_lock_irqsave(&async_lock, flags);
newcookie = entry->cookie = next_cookie++;
list_add_tail(&entry->list, &async_pending);
+ if (running->count++ == 0)
+ list_add_tail(&running->node, &async_domains);
atomic_inc(&entry_count);
spin_unlock_irqrestore(&async_lock, flags);
@@ -238,7 +243,7 @@ void async_synchronize_full(void)
{
do {
async_synchronize_cookie(next_cookie);
- } while (!list_empty(&async_running.domain) || !list_empty(&async_pending));
+ } while (!list_empty(&async_domains));
}
EXPORT_SYMBOL_GPL(async_synchronize_full);
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.
Cc: Liam Girdwood <[email protected]>
Cc: Mark Brown <[email protected]>
Cc: Arjan van de Ven <[email protected]>
Cc: James Bottomley <[email protected]>
Signed-off-by: Dan Williams <[email protected]>
---
drivers/regulator/core.c | 2 +-
drivers/scsi/libsas/sas_ata.c | 2 +-
drivers/scsi/scsi.c | 3 ++-
drivers/scsi/scsi_priv.h | 3 ++-
include/linux/async.h | 20 ++++++++++++++++----
kernel/async.c | 31 +++++++++++++++----------------
6 files changed, 37 insertions(+), 24 deletions(-)
diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index e70dd38..d1f4dae 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -2521,7 +2521,7 @@ static void regulator_bulk_enable_async(void *data, async_cookie_t cookie)
int regulator_bulk_enable(int num_consumers,
struct regulator_bulk_data *consumers)
{
- LIST_HEAD(async_domain);
+ ASYNC_DOMAIN(async_domain);
int i;
int ret = 0;
diff --git a/drivers/scsi/libsas/sas_ata.c b/drivers/scsi/libsas/sas_ata.c
index 607a35b..6dfbabc 100644
--- a/drivers/scsi/libsas/sas_ata.c
+++ b/drivers/scsi/libsas/sas_ata.c
@@ -828,7 +828,7 @@ static void async_sas_ata_eh(void *data, async_cookie_t cookie)
void sas_ata_strategy_handler(struct Scsi_Host *shost)
{
struct sas_ha_struct *sas_ha = SHOST_TO_SAS_HA(shost);
- LIST_HEAD(async);
+ ASYNC_DOMAIN(async);
int i;
/* it's ok to defer revalidation events during ata eh, these
diff --git a/drivers/scsi/scsi.c b/drivers/scsi/scsi.c
index 2ecdb01..2f92007 100644
--- a/drivers/scsi/scsi.c
+++ b/drivers/scsi/scsi.c
@@ -54,6 +54,7 @@
#include <linux/notifier.h>
#include <linux/cpu.h>
#include <linux/mutex.h>
+#include <linux/async.h>
#include <scsi/scsi.h>
#include <scsi/scsi_cmnd.h>
@@ -91,7 +92,7 @@ EXPORT_SYMBOL(scsi_logging_level);
#endif
/* sd and scsi_pm need to coordinate flushing async actions */
-LIST_HEAD(scsi_sd_probe_domain);
+ASYNC_DOMAIN(scsi_sd_probe_domain);
EXPORT_SYMBOL(scsi_sd_probe_domain);
/* NB: These are exposed through /proc/scsi/scsi and form part of the ABI.
diff --git a/drivers/scsi/scsi_priv.h b/drivers/scsi/scsi_priv.h
index 07ce3f5..eab472d 100644
--- a/drivers/scsi/scsi_priv.h
+++ b/drivers/scsi/scsi_priv.h
@@ -2,6 +2,7 @@
#define _SCSI_PRIV_H
#include <linux/device.h>
+#include <linux/async.h>
struct request_queue;
struct request;
@@ -163,7 +164,7 @@ static inline int scsi_autopm_get_host(struct Scsi_Host *h) { return 0; }
static inline void scsi_autopm_put_host(struct Scsi_Host *h) {}
#endif /* CONFIG_PM_RUNTIME */
-extern struct list_head scsi_sd_probe_domain;
+extern struct async_domain scsi_sd_probe_domain;
/*
* internal scsi timeout functions: for use by mid-layer and transport
diff --git a/include/linux/async.h b/include/linux/async.h
index 68a9530..dbb4d25 100644
--- a/include/linux/async.h
+++ b/include/linux/async.h
@@ -9,19 +9,31 @@
* as published by the Free Software Foundation; version 2
* of the License.
*/
+#ifndef __ASYNC_H__
+#define __ASYNC_H__
#include <linux/types.h>
#include <linux/list.h>
typedef u64 async_cookie_t;
typedef void (async_func_ptr) (void *data, async_cookie_t cookie);
+struct async_domain {
+ struct list_head node;
+ struct list_head domain;
+ int count;
+};
+
+#define ASYNC_DOMAIN(name) \
+ struct async_domain name = { .node = LIST_HEAD_INIT(name.node), \
+ .domain = LIST_HEAD_INIT(name.domain), \
+ .count = 0 }
extern async_cookie_t async_schedule(async_func_ptr *ptr, void *data);
extern async_cookie_t async_schedule_domain(async_func_ptr *ptr, void *data,
- struct list_head *list);
+ struct async_domain *domain);
extern void async_synchronize_full(void);
-extern void async_synchronize_full_domain(struct list_head *list);
+extern void async_synchronize_full_domain(struct async_domain *domain);
extern void async_synchronize_cookie(async_cookie_t cookie);
extern void async_synchronize_cookie_domain(async_cookie_t cookie,
- struct list_head *list);
-
+ struct async_domain *domain);
+#endif
diff --git a/kernel/async.c b/kernel/async.c
index bd0c168..aa23eec 100644
--- a/kernel/async.c
+++ b/kernel/async.c
@@ -62,7 +62,7 @@ static async_cookie_t next_cookie = 1;
#define MAX_WORK 32768
static LIST_HEAD(async_pending);
-static LIST_HEAD(async_running);
+static ASYNC_DOMAIN(async_running);
static DEFINE_SPINLOCK(async_lock);
struct async_entry {
@@ -71,7 +71,7 @@ struct async_entry {
async_cookie_t cookie;
async_func_ptr *func;
void *data;
- struct list_head *running;
+ struct async_domain *running;
};
static DECLARE_WAIT_QUEUE_HEAD(async_done);
@@ -82,13 +82,12 @@ static atomic_t entry_count;
/*
* MUST be called with the lock held!
*/
-static async_cookie_t __lowest_in_progress(struct list_head *running)
+static async_cookie_t __lowest_in_progress(struct async_domain *running)
{
struct async_entry *entry;
- if (!list_empty(running)) {
- entry = list_first_entry(running,
- struct async_entry, list);
+ if (!list_empty(&running->domain)) {
+ entry = list_first_entry(&running->domain, typeof(*entry), list);
return entry->cookie;
}
@@ -99,7 +98,7 @@ static async_cookie_t __lowest_in_progress(struct list_head *running)
return next_cookie; /* "infinity" value */
}
-static async_cookie_t lowest_in_progress(struct list_head *running)
+static async_cookie_t lowest_in_progress(struct async_domain *running)
{
unsigned long flags;
async_cookie_t ret;
@@ -119,10 +118,11 @@ static void async_run_entry_fn(struct work_struct *work)
container_of(work, struct async_entry, work);
unsigned long flags;
ktime_t uninitialized_var(calltime), delta, rettime;
+ struct async_domain *running = entry->running;
/* 1) move self to the running queue */
spin_lock_irqsave(&async_lock, flags);
- list_move_tail(&entry->list, entry->running);
+ list_move_tail(&entry->list, &running->domain);
spin_unlock_irqrestore(&async_lock, flags);
/* 2) run (and print duration) */
@@ -156,7 +156,7 @@ static void async_run_entry_fn(struct work_struct *work)
wake_up(&async_done);
}
-static async_cookie_t __async_schedule(async_func_ptr *ptr, void *data, struct list_head *running)
+static async_cookie_t __async_schedule(async_func_ptr *ptr, void *data, struct async_domain *running)
{
struct async_entry *entry;
unsigned long flags;
@@ -223,7 +223,7 @@ EXPORT_SYMBOL_GPL(async_schedule);
* Note: This function may be called from atomic or non-atomic contexts.
*/
async_cookie_t async_schedule_domain(async_func_ptr *ptr, void *data,
- struct list_head *running)
+ struct async_domain *running)
{
return __async_schedule(ptr, data, running);
}
@@ -238,20 +238,20 @@ void async_synchronize_full(void)
{
do {
async_synchronize_cookie(next_cookie);
- } while (!list_empty(&async_running) || !list_empty(&async_pending));
+ } while (!list_empty(&async_running.domain) || !list_empty(&async_pending));
}
EXPORT_SYMBOL_GPL(async_synchronize_full);
/**
* async_synchronize_full_domain - synchronize all asynchronous function within a certain domain
- * @list: running list to synchronize on
+ * @domain: running list to synchronize on
*
* This function waits until all asynchronous function calls for the
* synchronization domain specified by the running list @list have been done.
*/
-void async_synchronize_full_domain(struct list_head *list)
+void async_synchronize_full_domain(struct async_domain *domain)
{
- async_synchronize_cookie_domain(next_cookie, list);
+ async_synchronize_cookie_domain(next_cookie, domain);
}
EXPORT_SYMBOL_GPL(async_synchronize_full_domain);
@@ -264,8 +264,7 @@ EXPORT_SYMBOL_GPL(async_synchronize_full_domain);
* synchronization domain specified by the running list @list submitted
* prior to @cookie have been done.
*/
-void async_synchronize_cookie_domain(async_cookie_t cookie,
- struct list_head *running)
+void async_synchronize_cookie_domain(async_cookie_t cookie, struct async_domain *running)
{
ktime_t uninitialized_var(starttime), delta, endtime;
This is preparation to enable async_synchronize_full() to be used as a
replacement for scsi_complete_async_scans(), i.e. to stop leaking scsi
internal details where they are not needed.
Cc: Arjan van de Ven <[email protected]>
Cc: Len Brown <[email protected]>
Cc: Rafael J. Wysocki <[email protected]>
Cc: James Bottomley <[email protected]>
Signed-off-by: Dan Williams <[email protected]>
---
drivers/scsi/scsi_scan.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/scsi/scsi_scan.c b/drivers/scsi/scsi_scan.c
index 5e00e09..fb42aa0 100644
--- a/drivers/scsi/scsi_scan.c
+++ b/drivers/scsi/scsi_scan.c
@@ -1847,14 +1847,13 @@ static void do_scsi_scan_host(struct Scsi_Host *shost)
}
}
-static int do_scan_async(void *_data)
+static void do_scan_async(void *_data, async_cookie_t c)
{
struct async_scan_data *data = _data;
struct Scsi_Host *shost = data->shost;
do_scsi_scan_host(shost);
scsi_finish_async_scan(data);
- return 0;
}
/**
@@ -1863,7 +1862,6 @@ static int do_scan_async(void *_data)
**/
void scsi_scan_host(struct Scsi_Host *shost)
{
- struct task_struct *p;
struct async_scan_data *data;
if (strncmp(scsi_scan_type, "none", 4) == 0)
@@ -1878,9 +1876,11 @@ void scsi_scan_host(struct Scsi_Host *shost)
return;
}
- p = kthread_run(do_scan_async, data, "scsi_scan_%d", shost->host_no);
- if (IS_ERR(p))
- do_scan_async(data);
+ /* register with the async subsystem so wait_for_device_probe()
+ * will flush this work
+ */
+ async_schedule(do_scan_async, data);
+
/* scsi_autopm_put_host(shost) is called in scsi_finish_async_scan() */
}
EXPORT_SYMBOL(scsi_scan_host);
Now that scsi registers its async scan work with the async subsystem,
wait_for_device_probe() is sufficient for ensuring all scanning is
complete.
Cc: Arjan van de Ven <[email protected]>
Cc: Len Brown <[email protected]>
Cc: Rafael J. Wysocki <[email protected]>
Cc: James Bottomley <[email protected]>
Signed-off-by: Dan Williams <[email protected]>
---
drivers/scsi/scsi_scan.c | 12 ------------
drivers/scsi/scsi_wait_scan.c | 15 +++++----------
include/scsi/scsi_scan.h | 11 -----------
kernel/power/hibernate.c | 8 --------
kernel/power/user.c | 2 --
5 files changed, 5 insertions(+), 43 deletions(-)
delete mode 100644 include/scsi/scsi_scan.h
diff --git a/drivers/scsi/scsi_scan.c b/drivers/scsi/scsi_scan.c
index fb42aa0..20c7108 100644
--- a/drivers/scsi/scsi_scan.c
+++ b/drivers/scsi/scsi_scan.c
@@ -184,18 +184,6 @@ int scsi_complete_async_scans(void)
return 0;
}
-/* Only exported for the benefit of scsi_wait_scan */
-EXPORT_SYMBOL_GPL(scsi_complete_async_scans);
-
-#ifndef MODULE
-/*
- * For async scanning we need to wait for all the scans to complete before
- * trying to mount the root fs. Otherwise non-modular drivers may not be ready
- * yet.
- */
-late_initcall(scsi_complete_async_scans);
-#endif
-
/**
* scsi_unlock_floptical - unlock device via a special MODE SENSE command
* @sdev: scsi device to send command to
diff --git a/drivers/scsi/scsi_wait_scan.c b/drivers/scsi/scsi_wait_scan.c
index 74708fc..57de24a 100644
--- a/drivers/scsi/scsi_wait_scan.c
+++ b/drivers/scsi/scsi_wait_scan.c
@@ -12,21 +12,16 @@
#include <linux/module.h>
#include <linux/device.h>
-#include <scsi/scsi_scan.h>
static int __init wait_scan_init(void)
{
/*
- * First we need to wait for device probing to finish;
- * the drivers we just loaded might just still be probing
- * and might not yet have reached the scsi async scanning
+ * This will not return until all async work (system wide) is
+ * quiesced. Probing queues host-scanning work to the async
+ * queue which is why we don't need a separate call to
+ * scsi_complete_async_scans()
*/
wait_for_device_probe();
- /*
- * and then we wait for the actual asynchronous scsi scan
- * to finish.
- */
- scsi_complete_async_scans();
return 0;
}
@@ -38,5 +33,5 @@ MODULE_DESCRIPTION("SCSI wait for scans");
MODULE_AUTHOR("James Bottomley");
MODULE_LICENSE("GPL");
-late_initcall(wait_scan_init);
+module_init(wait_scan_init);
module_exit(wait_scan_exit);
diff --git a/include/scsi/scsi_scan.h b/include/scsi/scsi_scan.h
deleted file mode 100644
index 7889888..0000000
--- a/include/scsi/scsi_scan.h
+++ /dev/null
@@ -1,11 +0,0 @@
-#ifndef _SCSI_SCSI_SCAN_H
-#define _SCSI_SCSI_SCAN_H
-
-#ifdef CONFIG_SCSI
-/* drivers/scsi/scsi_scan.c */
-extern int scsi_complete_async_scans(void);
-#else
-static inline int scsi_complete_async_scans(void) { return 0; }
-#endif
-
-#endif /* _SCSI_SCSI_SCAN_H */
diff --git a/kernel/power/hibernate.c b/kernel/power/hibernate.c
index e09dfbf..821114a 100644
--- a/kernel/power/hibernate.c
+++ b/kernel/power/hibernate.c
@@ -25,7 +25,6 @@
#include <linux/freezer.h>
#include <linux/gfp.h>
#include <linux/syscore_ops.h>
-#include <scsi/scsi_scan.h>
#include "power.h"
@@ -735,13 +734,6 @@ static int software_resume(void)
async_synchronize_full();
}
- /*
- * We can't depend on SCSI devices being available after loading
- * one of their modules until scsi_complete_async_scans() is
- * called and the resume device usually is a SCSI one.
- */
- scsi_complete_async_scans();
-
swsusp_resume_device = name_to_dev_t(resume_file);
if (!swsusp_resume_device) {
error = -ENODEV;
diff --git a/kernel/power/user.c b/kernel/power/user.c
index 91b0fd0..4ed81e7 100644
--- a/kernel/power/user.c
+++ b/kernel/power/user.c
@@ -24,7 +24,6 @@
#include <linux/console.h>
#include <linux/cpu.h>
#include <linux/freezer.h>
-#include <scsi/scsi_scan.h>
#include <asm/uaccess.h>
@@ -84,7 +83,6 @@ static int snapshot_open(struct inode *inode, struct file *filp)
* appear.
*/
wait_for_device_probe();
- scsi_complete_async_scans();
data->swap = -1;
data->mode = O_WRONLY;
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 <[email protected]>
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.
James
On Fri, May 25, 2012 at 12:51 AM, James Bottomley
<[email protected]> 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 <[email protected]>
> 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.
Otherwise this leaves the pending libsas suspend/resume support in
limbo, since it will certainly deadlock in the case where any device
fails, or is slow to come back from resume.
--
Dan
On Fri, 2012-05-25 at 01:18 -0700, Dan Williams wrote:
> On Fri, May 25, 2012 at 12:51 AM, James Bottomley
> <[email protected]> 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 <[email protected]>
> > 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 <[email protected]>
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?
> Otherwise this leaves the pending libsas suspend/resume support in
> limbo, since it will certainly deadlock in the case where any device
> fails, or is slow to come back from resume.
I appreciate this is a bug, but it's not quite as serious as breaking
suspend and hibernate ... can we demonstrate they're still working?
James
On 5/25/2012 12:51 AM, James Bottomley 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.
it was not what was intended originally (the domains were supposed to be
completely independent beasts), however I can see that this is confusing
and even undesired, so I am ok with the change.
Ideally we get a way to have an async domain opt out of the global sync,
but until I get an actual user of that into mainline, don't worry about it.
I applied these 4 patches on top of todays git (one trivial conflict
with include removal) and it does not work - hangs here:
[ 65.322282] PCI: Enabling device: (0000:00:0d.0), cmd 5
[ 65.394252] scsi0 : pata_ali
[ 65.432976] scsi1 : pata_ali
[ 65.471610] ata1: PATA max UDMA/66 cmd 0x1fe02010200 ctl 0x1fe02010218 bmdma 0x1fe02010220 irq 12
[ 65.588340] ata2: PATA max UDMA/66 cmd 0x1fe02010210 ctl 0x1fe02010208 bmdma 0x1fe02010228 irq 12
[ 65.705941] Linux Tulip driver version 1.1.15-NAPI (Feb 27, 2007)
[ 65.788850] tulip0: Old style EEPROM with no media selection information
[ 65.877209] tulip0: MII transceiver #1 config 1000 status 782d advertising 01e1
[ 65.978529] net eth0: Davicom DM9102/DM9102A rev 49 at MMIO 0x1ff00000000, EEPROM not present, 00:03:ba:0c:06:cd, IRQ 9
[ 66.123140] tulip1: Old style EEPROM with no media selection information
[ 66.211483] tulip1: MII transceiver #1 config 1000 status 7809 advertising 01e1
[ 66.312539] net eth1: Davicom DM9102/DM9102A rev 49 at MMIO 0x1ff00002000, EEPROM not present, 00:03:ba:0c:06:ce, IRQ 10
[ 66.456108] ata1.00: ATA-7: SAMSUNG SP1213N, TL100-30, max UDMA/100
[ 66.538542] ata1.00: 234493056 sectors, multi 16: LBA48
[ 66.608850] mousedev: PS/2 mouse device common for all mice
[ 66.683438] rtc_cmos rtc_cmos: rtc core: registered rtc_cmos as rtc0
[ 66.766961] rtc0: no alarms, 114 bytes nvram
[ 66.823471] ata1.00: configured for UDMA/66
[ 66.879054] scsi 0:0:0:0: Direct-Access ATA SAMSUNG SP1213N TL10 PQ: 0 ANSI: 5
[ 66.988422] TCP: cubic registered
[ 67.032002] NET: Registered protocol family 17
[ 67.090463] Key type dns_resolver registered
[ 67.146589] initlevel:7=late, 15 registered initcalls
[ 67.214787] sd 0:0:0:0: [sda] 234493056 512-byte logical blocks: (120 GB/111 GiB)
[ 67.314623] sd 0:0:0:0: [sda] Write Protect is off
[ 67.377601] sd 0:0:0:0: [sda] Mode Sense: 00 3a 00 00
[ 67.444782] registered taskstats version 1
[ 67.510339] console [netcon0] enabled
[ 67.558375] netconsole: network logging started
[ 67.617966] rtc_cmos rtc_cmos: setting system clock to 2012-05-25 13:11:47 UTC (1337951507)
[ 89.787327] BUG: soft lockup - CPU#0 stuck for 23s! [swapper:1]
[ 89.865086] Modules linked in:
[ 89.905086] TSTATE: 0000004480001600 TPC: 0000000000471f10 TNPC: 0000000000471f14 Y: 00000000 Not tainted
[ 90.034288] TPC: <async_synchronize_cookie_domain+0x170/0x240>
[ 90.110926] g0: fffff8006e341001 g1: 0000000000000000 g2: 0000000000000000 g3: 0000000000000000
[ 90.225242] g4: fffff8006e054000 g5: 0000000000000000 g6: fffff8006e058000 g7: 0000000000846da0
[ 90.339642] o0: 0000000000000001 o1: fffff8006e058400 o2: 0000000000471ff0 o3: 0000000000000000
[ 90.454045] o4: 0000000000000000 o5: 000000000085b800 sp: fffff8006e05b371 ret_pc: 00000000004209d4
[ 90.573025] RPC: <tl0_irq14+0x14/0x20>
[ 90.622249] l0: 0000000000001000 l1: 0000000080001607 l2: 0000000000471fec l3: 0000000000000400
[ 90.736565] l4: 0000000000000000 l5: 0000000000000000 l6: 0000000000000000 l7: 0000000000000008
[ 90.850969] i0: 0000000000000004 i1: 0000000000846d70 i2: 0000000000846d80 i3: 00000000004d3a4c
[ 90.965369] i4: 00000000000005c3 i5: 000000000089a000 i6: fffff8006e05b451 i7: 0000000000472014
[ 91.079775] I7: <async_synchronize_full+0x14/0x40>
[ 91.142711] Call Trace:
[ 91.174721] [0000000000472014] async_synchronize_full+0x14/0x40
[ 91.253593] [0000000000601134] wait_for_device_probe+0x74/0xa0
[ 91.331420] [0000000000876e30] prepare_namespace+0x44/0x198
[ 91.405812] [00000000008769a0] kernel_init+0x100/0x114
[ 91.474394] [000000000042b0d0] kernel_thread+0x30/0x60
[ 91.543074] [00000000006f71c8] rest_init+0x10/0x68
--
Meelis Roos ([email protected])
On Fri, May 25, 2012 at 6:40 AM, <[email protected]> wrote:
> I applied these 4 patches on top of todays git (one trivial conflict
> with include removal) and it does not work - hangs here:
Ok, I'll take a look.
Thanks for the help!
--
Dan
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
> > <[email protected]> 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 <[email protected]>
> > > 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 <[email protected]>
> 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
On Fri, May 25, 2012 at 12:50:27AM -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.
Acked-by: Mark Brown <[email protected]>