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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 25 Apr 2017 12:36:29 +0100
From:   Greg Kroah-Hartman <gregkh@...uxfoundation.org>
To:     weili@...eaurora.org
Cc:     linux-kernel@...r.kernel.org, vatsa@...eaurora.org
Subject: Re: [PATCH] driver-core: remove lock for platform devices during
 probe

On Tue, Apr 25, 2017 at 04:43:33PM +0800, weili@...eaurora.org wrote:
> Hi Greg K-H,
> 
> On 2017-04-24 16:46, Greg Kroah-Hartman wrote:
> 
> > And does it really reduce boot time? What are the numbers?
>   Yes, it really reduce boot time. After making most time-consuming platform
> driver using async probe
>   and also applying this patch, we see the driver run in parallel with
> others and saving 140ms.

And why wasn't that information in the initial commit message?

And how much of a % is 140ms?  Why is a single driver taking that long
to initialize itself?

> > Why do you have so many platform devices and not "real bus" devices?
>   We are working on an ARM soc. There are many host controllers implemented
> as platform devices.

Don't you think that is also a problem?

> > What does the boot graph look like when you run with and without this
> > patch?
>   Without the patch, the boot graph is like this:
>     CPU0: platform driver1 probe -> lock parent -> do probe staff -> unlock
> parent -> probe finish
>     CPU1: platform driver2 probe ->                wait for lock on parent
> -> lock parent -> do probe -> unlock parent -> probe finish
> 
>   With the patch, the boot graph is like this:
>     CPU0: platform driver1 probe -> do probe staff -> probe finish
>     CPU1: platform drvier2 probe -> do probe staff -> probe finish

No, I mean the boot graph in pretty .svg format that the kernel can
output, with times and processes and everything.  Look in the tools
directory for more information, it will give you the exact timing for
your change before and after and show you exactly where you are taking
long periods of time.

You did use that, or something else to measure this somehow, right?

> > Why is the platform bus so "special" to warrant this?  Should we perhaps
> > make this
> > an option for any bus to enable/disable?
>   The lock on parent was first introduced by USB guys in following commit
> https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable.git/commit/drivers/base/dd.c?id=bf74ad5bc41727d5f2f1c6bedb2c1fac394de731
>   This may be useful for real bus devices such as USB and they think
> overhead of acquiring a lock is not large.
>   But since platfrom bus is virtual, the lock is not necessary. Removing it
> for platform devices will make
>   driver running in parallel and benefit boot time.

I know all about USB here :)

You did not answer my questions :(

greg k-h