[PATCH] driver core: remove polling for driver_probe_done(v2)

[PATCH] driver core: remove polling for driver_probe_done(v2)

[tom.leiming]

From: Ming Lei <tom.leiming@gmail.com>

This patch renames driver_probe_done to driver_wait_probe_done,
and make it wait on condition of probing done to remove
polling for it in fs initialization. Also add
driver_wait_probe_done_and_dev_appear(name) to allow callers
to wait until all probing is done AND until the device @name
exists.

Removing polling in fs initialization may lead to a faster boot.

Signed-off-by: Ming Lei <tom.leiming@gmail.com>
---
 drivers/base/dd.c      |   26 +++++++++++++++++++-------
 include/linux/device.h |    3 ++-
 init/do_mounts.c       |    9 ++++-----
 init/do_mounts_md.c    |    5 +++--
 4 files changed, 28 insertions(+), 15 deletions(-)

diff --git a/drivers/base/dd.c b/drivers/base/dd.c
index 9b721d3..ab8c30f 100644
--- a/drivers/base/dd.c
+++ b/drivers/base/dd.c
@@ -21,6 +21,7 @@
 #include <linux/module.h>
 #include <linux/kthread.h>
 #include <linux/wait.h>
+#include <linux/mount.h>
 
 #include "base.h"
 #include "power/power.h"
@@ -153,21 +154,32 @@ done:
 }
 
 /**
- * driver_probe_done
- * Determine if the probe sequence is finished or not.
+ * driver_probe_wait_done
+ * wait until the probe sequence is finished.
  *
- * Should somehow figure out how to use a semaphore, not an atomic variable...
  */
-int driver_probe_done(void)
+void driver_wait_probe_done(void)
 {
 	pr_debug("%s: probe_count = %d\n", __func__,
 		 atomic_read(&probe_count));
-	if (atomic_read(&probe_count))
-		return -EBUSY;
-	return 0;
+
+	wait_event(probe_waitqueue, atomic_read(&probe_count) == 0);
 }
 
 /**
+ * driver_wait_probe_done_and_dev_appear
+ * wait until the probe sequence is finished _and_ the dev @name appears.
+ *
+ */
+void driver_wait_probe_done_and_dev_appear(char *name)
+{
+	pr_debug("%s: probe_count = %d\n", __func__,
+		 atomic_read(&probe_count));
+
+	wait_event(probe_waitqueue, (atomic_read(&probe_count) == 0) &&
+		  name_to_dev_t(name));
+}
+/**
  * driver_probe_device - attempt to bind device & driver together
  * @drv: driver to bind a device to
  * @dev: device to try to bind to the driver
diff --git a/include/linux/device.h b/include/linux/device.h
index 45e5b19..7e2e138 100644
--- a/include/linux/device.h
+++ b/include/linux/device.h
@@ -146,7 +146,8 @@ extern struct device_driver *get_driver(struct device_driver *drv);
 extern void put_driver(struct device_driver *drv);
 extern struct device_driver *driver_find(const char *name,
 					 struct bus_type *bus);
-extern int driver_probe_done(void);
+extern void driver_wait_probe_done(void);
+extern void driver_wait_probe_done_and_dev_appear(char *name);
 
 /* sysfs interface for exporting driver attributes */
 
diff --git a/init/do_mounts.c b/init/do_mounts.c
index 708105e..1bf34fc 100644
--- a/init/do_mounts.c
+++ b/init/do_mounts.c
@@ -371,8 +371,8 @@ void __init prepare_namespace(void)
 	}
 
 	/* wait for the known devices to complete their probing */
-	while (driver_probe_done() != 0)
-		msleep(100);
+	driver_wait_probe_done();
+
 	async_synchronize_full();
 
 	md_run_setup();
@@ -396,9 +396,8 @@ void __init prepare_namespace(void)
 	if ((ROOT_DEV == 0) && root_wait) {
 		printk(KERN_INFO "Waiting for root device %s...\n",
 			saved_root_name);
-		while (driver_probe_done() != 0 ||
-			(ROOT_DEV = name_to_dev_t(saved_root_name)) == 0)
-			msleep(100);
+		driver_wait_probe_done_and_dev_appear(saved_root_name);
+		ROOT_DEV = name_to_dev_t(saved_root_name);
 	}
 
 	is_floppy = MAJOR(ROOT_DEV) == FLOPPY_MAJOR;
diff --git a/init/do_mounts_md.c b/init/do_mounts_md.c
index ff95e31..bcea03b 100644
--- a/init/do_mounts_md.c
+++ b/init/do_mounts_md.c
@@ -281,8 +281,9 @@ static void __init autodetect_raid(void)
 	 */
 	printk(KERN_INFO "md: Waiting for all devices to be available before autodetect\n");
 	printk(KERN_INFO "md: If you don't use raid, use raid=noautodetect\n");
-	while (driver_probe_done() < 0)
-		msleep(100);
+
+	driver_wait_probe_done();
+
 	fd = sys_open("/dev/md0", 0, 0);
 	if (fd >= 0) {
 		sys_ioctl(fd, RAID_AUTORUN, raid_autopart);
-- 
1.6.0

Re: [PATCH] driver core: remove polling for driver_probe_done(v2)

[Arjan van de Ven]

On Fri, 30 Jan 2009 10:31:42 +0800
tom.leiming@gmail.com wrote:

> From: Ming Lei <tom.leiming@gmail.com>
> 
> This patch renames driver_probe_done to driver_wait_probe_done,
> and make it wait on condition of probing done to remove
> polling for it in fs initialization. Also add
> driver_wait_probe_done_and_dev_appear(name) to allow callers
> to wait until all probing is done AND until the device @name
> exists.

>  /**
> + * driver_wait_probe_done_and_dev_appear
> + * wait until the probe sequence is finished _and_ the dev @name
> appears.

Hi,

sorry to be the bad guy, but I think you turned the "either one" into a
"both".... so this isn't going to work ;(

unless I'm just too tired and miss something subtle, but I am pretty
sure DeMorgan does not work this way :)


-- 
Arjan van de Ven 	Intel Open Source Technology Centre
For development, discussion and tips for power savings, 
visit http://www.lesswatts.org

Re: [PATCH] driver core: remove polling for driver_probe_done(v2)

[Ming Lei]

2009/1/30 Arjan van de Ven <arjan@infradead.org>:
> On Fri, 30 Jan 2009 10:31:42 +0800
> tom.leiming@gmail.com wrote:
>
>> From: Ming Lei <tom.leiming@gmail.com>
>>
>> This patch renames driver_probe_done to driver_wait_probe_done,
>> and make it wait on condition of probing done to remove
>> polling for it in fs initialization. Also add
>> driver_wait_probe_done_and_dev_appear(name) to allow callers
>> to wait until all probing is done AND until the device @name
>> exists.
>
>>  /**
>> + * driver_wait_probe_done_and_dev_appear
>> + * wait until the probe sequence is finished _and_ the dev @name
>> appears.
>
> Hi,
>
> sorry to be the bad guy, but I think you turned the "either one" into a
> "both".... so this isn't going to work ;(

IMHO, it will msleep if either one is ture, so we turn this into
return( not sleep) if both are false.
( eg, A||B->C   <=>  ~C->~A&&~B )   :-)

           while (driver_probe_done() != 0 ||
                  (ROOT_DEV = name_to_dev_t(saved_root_name)) == 0)
                  msleep(100);
 here:
      driver_probe_done() == 0  &&  name_to_dev_t(saved_root_name)

===>

driver_wait_probe_done_and_dev_appear(name)
{
         if (driver_probe_done() == 0 &&
                  (ROOT_DEV = name_to_dev_t(saved_root_name)))
                  return;
         else
                  schedule();
}

Thanks!

>
> unless I'm just too tired and miss something subtle, but I am pretty
> sure DeMorgan does not work this way :)
>
>
> --
> Arjan van de Ven        Intel Open Source Technology Centre
> For development, discussion and tips for power savings,
> visit http://www.lesswatts.org
>



-- 
Lei Ming

Re: [PATCH] driver core: remove polling for driver_probe_done(v2)

[Cornelia Huck]

On Fri, 30 Jan 2009 10:31:42 +0800,
tom.leiming@gmail.com wrote:


>  /**
> + * driver_wait_probe_done_and_dev_appear
> + * wait until the probe sequence is finished _and_ the dev @name appears.
> + *
> + */
> +void driver_wait_probe_done_and_dev_appear(char *name)
> +{
> +	pr_debug("%s: probe_count = %d\n", __func__,
> +		 atomic_read(&probe_count));
> +
> +	wait_event(probe_waitqueue, (atomic_read(&probe_count) == 0) &&
> +		  name_to_dev_t(name));

probe_waitqueue is only woken at the end of really_probe(), not when
whatever needs to be done to make the device available is done...

> +}
> +/**
>   * driver_probe_device - attempt to bind device & driver together
>   * @drv: driver to bind a device to
>   * @dev: device to try to bind to the driver

Re: [PATCH] driver core: remove polling for driver_probe_done(v2)

[Ming Lei]

2009/1/30 Cornelia Huck <cornelia.huck@de.ibm.com>:
> On Fri, 30 Jan 2009 10:31:42 +0800,
> tom.leiming@gmail.com wrote:
>
>
>>  /**
>> + * driver_wait_probe_done_and_dev_appear
>> + * wait until the probe sequence is finished _and_ the dev @name appears.
>> + *
>> + */
>> +void driver_wait_probe_done_and_dev_appear(char *name)
>> +{
>> +     pr_debug("%s: probe_count = %d\n", __func__,
>> +              atomic_read(&probe_count));
>> +
>> +     wait_event(probe_waitqueue, (atomic_read(&probe_count) == 0) &&
>> +               name_to_dev_t(name));
>
> probe_waitqueue is only woken at the end of really_probe(), not when
> whatever needs to be done to make the device available is done...

Yes.

If probing done does not mean the device is available, this patch should be
ignored or fixed further.

But polling is really not good, is there a approach to avoid the polling for
appearence of the root device?

Thanks!

>
>> +}
>> +/**
>>   * driver_probe_device - attempt to bind device & driver together
>>   * @drv: driver to bind a device to
>>   * @dev: device to try to bind to the driver
>



-- 
Lei Ming

Re: [PATCH] driver core: remove polling for driver_probe_done(v2)

[Cornelia Huck]

On Fri, 30 Jan 2009 18:17:36 +0800,
Ming Lei <tom.leiming@gmail.com> wrote:

> If probing done does not mean the device is available, this patch should be
> ignored or fixed further.
> 
> But polling is really not good, is there a approach to avoid the polling for
> appearence of the root device?

I think the parts of your patch that replace polling for
driver_probe_done() with waiting on the waitqueue are fine. The
problematic part is the rootwait special case, where polling seems
unavoidable afaics.

Re: [PATCH] driver core: remove polling for driver_probe_done(v2)

[Ming Lei]

2009/1/30 Cornelia Huck <cornelia.huck@de.ibm.com>:
> On Fri, 30 Jan 2009 18:17:36 +0800,
> Ming Lei <tom.leiming@gmail.com> wrote:
>
>> If probing done does not mean the device is available, this patch should be
>> ignored or fixed further.
>>
>> But polling is really not good, is there a approach to avoid the polling for
>> appearence of the root device?
>
> I think the parts of your patch that replace polling for
> driver_probe_done() with waiting on the waitqueue are fine. The
> problematic part is the rootwait special case, where polling seems
> unavoidable afaics.

Yes.

Maybe we should keep the polling for root device. And I will resend a
patch ,which only replace the driver_probe_done().

Thanks.

>



-- 
Lei Ming