linux-kernel - Re: [PATCH 1/3] driver core: Mark impossible return values of bus

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]

Message-ID: <2024090444-earmark-showpiece-b3dc@gregkh>
Date: Wed, 4 Sep 2024 15:53:30 +0200
From: Greg Kroah-Hartman <gregkh@...uxfoundation.org>
To: Zijun Hu <zijun_hu@...oud.com>
Cc: "Rafael J. Wysocki" <rafael@...nel.org>, linux-kernel@...r.kernel.org,
	Zijun Hu <quic_zijuhu@...cinc.com>
Subject: Re: [PATCH 1/3] driver core: Mark impossible return values of
 bus_type's match() with unlikely()

On Wed, Sep 04, 2024 at 08:56:42PM +0800, Zijun Hu wrote:
> From: Zijun Hu <quic_zijuhu@...cinc.com>
> 
> Bus_type's match() should return bool type compatible integer 0 or 1
> ideally since its main operations are lookup and comparison normally
> actually, this rule is followed by ALL bus_types but @amba_bustype within
> current v6.10 kernel tree, for @amba_bustype, ONLY extra -EPROBE_DEFER
> may be returned, so mark those impossible or rare return values with
> unlikely() to help readers understand device and driver binding logic.

unlikely() and likely() should ONLY be used when you can measure the
performance impact.  And any "scan all devices in the bus" function is
NOT performance critical at all.  So this is not the place for that,
sorry.

> Signed-off-by: Zijun Hu <quic_zijuhu@...cinc.com>
> ---
>  drivers/base/dd.c          | 16 ++++++++++++----
>  include/linux/device/bus.h |  9 ++++-----
>  2 files changed, 16 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/base/dd.c b/drivers/base/dd.c
> index 9b745ba54de1..288e19c9854b 100644
> --- a/drivers/base/dd.c
> +++ b/drivers/base/dd.c
> @@ -928,7 +928,11 @@ static int __device_attach_driver(struct device_driver *drv, void *_data)
>  	if (ret == 0) {
>  		/* no match */
>  		return 0;
> -	} else if (ret == -EPROBE_DEFER) {
> +	} else if (unlikely(ret == -EPROBE_DEFER)) {
> +		/*
> +		 * Only match() of @amba_bustype may return this error
> +		 * in current v6.10 tree, so also give unlikely() here.

version numbers in the kernel source mean nothing, and they age
horribly.  This is not going to work out at all.

let's fix up the one user that is doing this wrong and then we don't
have to worry about it, right?  We have the source for everything, let's
use it :)

thanks,

greg k-h