linux-kernel - Re: [PATCH v1] iopoll: Tweak readx_poll

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 for Android: free password hash cracker in your pocket

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

Date:   Thu, 13 Jun 2019 18:36:57 +0200
From:   Marc Gonzalez <marc.w.gonzalez@...e.fr>
To:     Doug Anderson <dianders@...omium.org>
Cc:     Arnd Bergmann <arnd@...db.de>, Will Deacon <will.deacon@....com>,
        Linux ARM <linux-arm-kernel@...ts.infradead.org>,
        LKML <linux-kernel@...r.kernel.org>,
        Thierry Reding <thierry.reding@...il.com>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Bjorn Andersson <bjorn.andersson@...aro.org>,
        Thomas Gleixner <tglx@...utronix.de>,
        Bjorn Helgaas <helgaas@...nel.org>
Subject: Re: [PATCH v1] iopoll: Tweak readx_poll_timeout sleep range

On 13/06/2019 18:11, Doug Anderson wrote:

> On Thu, Jun 13, 2019 at 9:04 AM Marc Gonzalez wrote:
>
>> Hmmm, I expect the typical use-case to be:
>> "HW manual states operation X completes in 100 µs.
>> Let's call usleep_range(100, foo); before hitting the reg."
>>
>> And foo needs to be a "reasonable" value: big enough to be able
>> to merge several requests, low enough not to wait too long after
>> the HW is ready.
>>
>> In this case, I'd say usleep_range(100, 200); makes sense.
>>
>> Come to think of it, I'm not sure min=26 (or min=50) makes sense...
>> Why wait *less* than what the user specified?
> 
> IIRC usleep_range() nearly always tries to sleep for the max.  My
> recollection of the design is that you only end up with something less
> than the max if the system was going to wake up anyway.  In such a
> case it seems like it wouldn't be insane to go and check if the
> condition is already true if 25% of the time has passed.  Maybe you'll
> get lucky and you can return early.
> 
> Are you actually seeing problems with the / 4, or is this patch just a
> result of code inspection?

No actual issue. I just ran into a driver calling:

	readl_poll_timeout(status, val, val & mask, 1, 1000);

and it seemed... unwise(?) to call usleep_range(1, 1);

But if, as you say, usleep_range() aims for the max, then I guess it's
not a big deal to issue an early read or 3... Meh

Regards.