linux-kernel - Re: [RFC PATCH v2] driver core: hold dev's parent lock when needed

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, 29 May 2018 09:47:30 +0200
From:   Greg KH <gregkh@...uxfoundation.org>
To:     martin_liu <liumartin@...gle.com>
Cc:     stern@...land.harvard.edu, heikki.krogerus@...ux.intel.com,
        johan@...nel.org, linux-kernel@...r.kernel.org,
        linux-usb@...r.kernel.org, jenhaochen@...gle.com
Subject: Re: [RFC PATCH v2] driver core: hold dev's parent lock when needed

On Tue, May 29, 2018 at 03:07:20PM +0800, martin_liu wrote:
> SOC have internal I/O buses that can't be proved for devices. The
> devices on the buses can be accessed directly without additinal
> configuration required. This type of bus is represented as
> "simple-bus". In some platforms, we name "soc" with "simple-bus"
> attribute and many devices are hooked under it desribed in DT
> (device tree).
> 
> In commit 'bf74ad5bc417 introduce ("[PATCH] Hold the device's
> parent's lock during probe and remove")' to solve USB subsystem
> lock sequence since usb device's characteristic. Thus "soc"
> needs to be locked whenever a device and driver's probing
> happen under "soc" bus. During this period, an async driver
> tries to probe a device which is under the "soc" bus would be
> blocked until previous driver finish the probing and release "soc"
> lock. And the next probing under the "soc" bus need to wait for
> async finish. Because of that, driver's async probe for init
> time improvement will be shadowed.
> 
> Since many devices don't have USB devices' characteristic, they
> actually don't need parent's lock. Thus, we introduce a lock flag
> in device struct and driver core would lock the parent lock base
> on the flag. For usbsystem, we set this flag when its device and
> driver is matched and to keep original lock behavior in driver
> core.
> 
> Async probe could have more benefit after this patch.
> 
> Signed-off-by: martin_liu <liumartin@...gle.com>

Nit, your name here is probably not what you sign legal documents with.
Please use your "real name" in this line, and in your From: email line
(hint, no '_').  This prevents me from accepting the patch at all.

> Suggested-by: Alan Stern <stern@...land.harvard.edu>

Changes were suggested, I don't know if the whole idea was :)

> --- a/include/linux/device.h
> +++ b/include/linux/device.h
> @@ -992,6 +992,7 @@ struct device {
>  	bool			offline_disabled:1;
>  	bool			offline:1;
>  	bool			of_node_reused:1;
> +	bool			need_parent_lock:1;

Shouldn't this be a bus type flag, and not a device-specific flag?  You
are only ever caring about this based on the type of bus the device is
on, the rule would not be different for different devices on the same
bus from what I can tell.

thanks,

greg k-h