Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1629215 > unrolled thread

[PATCH] driver-core: remove lock for platform devices during probe

Back to article view | Back to linux.kernel

Contents

  [PATCH] driver-core: remove lock for platform devices during probe Wei Li <weili@codeaurora.org> - 2017-04-24 07:50 +0200
    Re: [PATCH] driver-core: remove lock for platform devices during  probe Greg Kroah-Hartman <gregkh@linuxfoundation.org> - 2017-04-24 09:40 +0200
      Re: [PATCH] driver-core: remove lock for platform devices during  probe weili@codeaurora.org - 2017-04-24 10:30 +0200
        Re: [PATCH] driver-core: remove lock for platform devices during  probe Greg Kroah-Hartman <gregkh@linuxfoundation.org> - 2017-04-24 10:50 +0200

#1629215 — [PATCH] driver-core: remove lock for platform devices during probe

During driver probe procedure, lock on the parent of
platform devices could be removed to make probe in
parallel.

Signed-off-by: Wei Li <weili@codeaurora.org>
---
 drivers/base/dd.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/base/dd.c b/drivers/base/dd.c
index a1fbf55..e238fbc 100644
--- a/drivers/base/dd.c
+++ b/drivers/base/dd.c
@@ -25,6 +25,7 @@
 #include <linux/async.h>
 #include <linux/pm_runtime.h>
 #include <linux/pinctrl/devinfo.h>
+#include <linux/platform_device.h>
 
 #include "base.h"
 #include "power/power.h"
@@ -749,13 +750,14 @@ static int __driver_attach(struct device *dev, void *data)
 		return ret;
 	} /* ret > 0 means positive match */
 
-	if (dev->parent)	/* Needed for USB */
+	if (dev->parent &&
+		(dev->bus != &platform_bus_type))	/* Needed for USB */
 		device_lock(dev->parent);
 	device_lock(dev);
 	if (!dev->driver)
 		driver_probe_device(drv, dev);
 	device_unlock(dev);
-	if (dev->parent)
+	if (dev->parent && (dev->bus != &platform_bus_type))
 		device_unlock(dev->parent);
 
 	return 0;
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,\na Linux Foundation Collaborative Project

[toc] | [next] | [standalone]

#1629269 — Re: [PATCH] driver-core: remove lock for platform devices during probe

On Mon, Apr 24, 2017 at 01:42:16PM +0800, Wei Li wrote:
> During driver probe procedure, lock on the parent of
> platform devices could be removed to make probe in
> parallel.
> 
> Signed-off-by: Wei Li <weili@codeaurora.org>

Why?  Why does this matter?

> ---
>  drivers/base/dd.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/base/dd.c b/drivers/base/dd.c
> index a1fbf55..e238fbc 100644
> --- a/drivers/base/dd.c
> +++ b/drivers/base/dd.c
> @@ -25,6 +25,7 @@
>  #include <linux/async.h>
>  #include <linux/pm_runtime.h>
>  #include <linux/pinctrl/devinfo.h>
> +#include <linux/platform_device.h>
>  
>  #include "base.h"
>  #include "power/power.h"
> @@ -749,13 +750,14 @@ static int __driver_attach(struct device *dev, void *data)
>  		return ret;
>  	} /* ret > 0 means positive match */
>  
> -	if (dev->parent)	/* Needed for USB */
> +	if (dev->parent &&
> +		(dev->bus != &platform_bus_type))	/* Needed for USB */

The platform_bus_type check is not needed by USB, right?

thanks,

greg k-h

[toc] | [prev] | [next] | [standalone]

#1629293 — Re: [PATCH] driver-core: remove lock for platform devices during probe

Hi Greg,

    We are optimizing boot time for Linux kernel and try to make some 
platform drivers use asynchronous probe(by changing probe type of driver 
to PROBE_PREFER_ASYNCHRONOUS) to reduce boot time. However we found the 
platform drivers did not probe in parallel because they will lock the 
same parent device(platform bus for platform drivers) during probe.  So 
we add this patch to remove lock of parent for platform device. This 
will help to make platform driver probe in parallel and reduce boot 
time.


Best Regards
Wei


On 2017-04-24 15:32, Greg Kroah-Hartman wrote:
> On Mon, Apr 24, 2017 at 01:42:16PM +0800, Wei Li wrote:
>> During driver probe procedure, lock on the parent of
>> platform devices could be removed to make probe in
>> parallel.
>> 
>> Signed-off-by: Wei Li <weili@codeaurora.org>
> 
> Why?  Why does this matter?
> 
>> ---
>>  drivers/base/dd.c | 6 ++++--
>>  1 file changed, 4 insertions(+), 2 deletions(-)
>> 
>> diff --git a/drivers/base/dd.c b/drivers/base/dd.c
>> index a1fbf55..e238fbc 100644
>> --- a/drivers/base/dd.c
>> +++ b/drivers/base/dd.c
>> @@ -25,6 +25,7 @@
>>  #include <linux/async.h>
>>  #include <linux/pm_runtime.h>
>>  #include <linux/pinctrl/devinfo.h>
>> +#include <linux/platform_device.h>
>> 
>>  #include "base.h"
>>  #include "power/power.h"
>> @@ -749,13 +750,14 @@ static int __driver_attach(struct device *dev, 
>> void *data)
>>  		return ret;
>>  	} /* ret > 0 means positive match */
>> 
>> -	if (dev->parent)	/* Needed for USB */
>> +	if (dev->parent &&
>> +		(dev->bus != &platform_bus_type))	/* Needed for USB */
> 
> The platform_bus_type check is not needed by USB, right?
> 
> thanks,
> 
> greg k-h

[toc] | [prev] | [next] | [standalone]

#1629309 — Re: [PATCH] driver-core: remove lock for platform devices during probe

A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?
A: Top-posting.
Q: What is the most annoying thing in e-mail?

A: No.
Q: Should I include quotations after my reply?


http://daringfireball.net/2007/07/on_top

On Mon, Apr 24, 2017 at 04:27:44PM +0800, weili@codeaurora.org wrote:
> Hi Greg,
> 
>    We are optimizing boot time for Linux kernel and try to make some
> platform drivers use asynchronous probe(by changing probe type of driver to
> PROBE_PREFER_ASYNCHRONOUS) to reduce boot time. However we found the
> platform drivers did not probe in parallel because they will lock the same
> parent device(platform bus for platform drivers) during probe.  So we add
> this patch to remove lock of parent for platform device. This will help to
> make platform driver probe in parallel and reduce boot time.

And does it really reduce boot time?  What are the numbers?  Why do you
have so many platform devices and not "real bus" devices?  What does the
boot graph look like when you run with and without this patch?  Why is
the platform bus so "special" to warrant this?  Should we perhaps make
this an option for any bus to enable/disable?

In other words, you need to provide a whole lot more information here,
justify why you are doing this type of change, and fix the issue I
pointed out, in order to get such a chance accepted.

Come on now, you know better than this...

greg k-h

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web