linux-kernel - Re: [PATCH] iio: sx9310: Prefer async 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]

Message-ID: <20200829161230.398e0051@archlinux>
Date:   Sat, 29 Aug 2020 16:12:30 +0100
From:   Jonathan Cameron <jic23@...nel.org>
To:     Douglas Anderson <dianders@...omium.org>
Cc:     swboyd@...omium.org,
        Alexandru Ardelean <alexandru.ardelean@...log.com>,
        Daniel Campello <campello@...omium.org>,
        Enrico Granata <egranata@...omium.org>,
        Hartmut Knaack <knaack.h@....de>,
        Lars-Peter Clausen <lars@...afoo.de>,
        Peter Meerwald-Stadler <pmeerw@...erw.net>,
        linux-iio@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] iio: sx9310: Prefer async probe

On Fri, 28 Aug 2020 17:01:18 -0700
Douglas Anderson <dianders@...omium.org> wrote:

> On one board I found that:
>   probe of 5-0028 returned 1 after 259547 usecs
> 
> There's no reason to block probe of all other devices on our probe.
> Turn on async probe.
> 
> Signed-off-by: Douglas Anderson <dianders@...omium.org>
> ---
> NOTE: I haven't done any analysis of the driver to see _why_ it's so
> slow, only that I have measured it to be slow.  Someone could
> certainly take the time to profile / optimize it, but in any case it
> still won't hurt to be async.

Hmm. It is vanishingly rare to use that flag so I'm not particularly
keen on starting to deploy it when we don't know why a particular
driver is taking so long.  I agree it should be safe but I don't
like oddities I don't understand!

There are some sleeps in there but they are all of the order of a few
msecs.

Could it be there is a regulator that is coming up very slowly?

Any other ideas?

Jonathan

> 
> This is a very safe flag to turn on since:
> 
> 1. It's not like our probe order was defined by anything anyway.  When
> we probe is at the whim of when our i2c controller probes and that can
> be any time.
> 
> 2. If some other driver needs us then they have to handle the fact
> that we might not have probed yet anyway.
> 
> 3. There may be other drivers probing at the same time as us anyway
> because _they_ used async probe.
> 
> While I won't say that it's impossible to tickle a bug by turning on
> async probe, I would assert that in almost all cases the bug was
> already there and needed to be fixed anyway.
> 
> ALSO NOTE: measurement / testing was done on the downstream Chrome OS
> 5.4 tree.  I confirmed compiling on mainline.
> 
>  drivers/iio/proximity/sx9310.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/iio/proximity/sx9310.c b/drivers/iio/proximity/sx9310.c
> index dc2e11b43431..444cafc53408 100644
> --- a/drivers/iio/proximity/sx9310.c
> +++ b/drivers/iio/proximity/sx9310.c
> @@ -1054,6 +1054,7 @@ static struct i2c_driver sx9310_driver = {
>  		.acpi_match_table = ACPI_PTR(sx9310_acpi_match),
>  		.of_match_table = of_match_ptr(sx9310_of_match),
>  		.pm = &sx9310_pm_ops,
> +		.probe_type = PROBE_PREFER_ASYNCHRONOUS,
>  	},
>  	.probe		= sx9310_probe,
>  	.id_table	= sx9310_id,