linux-kernel - Re: [PATCH 4/4] DO-NOT-MERGE: drm/i915: Use poll_timeout

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: <aGZ8s2mQCmvYK7w1@intel.com>
Date: Thu, 3 Jul 2025 15:50:59 +0300
From: Ville Syrjälä <ville.syrjala@...ux.intel.com>
To: Jani Nikula <jani.nikula@...el.com>
Cc: linux-kernel@...r.kernel.org,
	Lucas De Marchi <lucas.demarchi@...el.com>,
	Dibin Moolakadan Subrahmanian <dibin.moolakadan.subrahmanian@...el.com>,
	Imre Deak <imre.deak@...el.com>,
	David Laight <david.laight.linux@...il.com>,
	Geert Uytterhoeven <geert+renesas@...der.be>,
	Matt Wagantall <mattw@...eaurora.org>,
	Dejin Zheng <zhengdejin5@...il.com>,
	intel-gfx@...ts.freedesktop.org, intel-xe@...ts.freedesktop.org
Subject: Re: [PATCH 4/4] DO-NOT-MERGE: drm/i915: Use poll_timeout_us()

On Thu, Jul 03, 2025 at 03:12:39PM +0300, Jani Nikula wrote:
> On Thu, 03 Jul 2025, Ville Syrjala <ville.syrjala@...ux.intel.com> wrote:
> > From: Ville Syrjälä <ville.syrjala@...ux.intel.com>
> >
> > Make sure poll_timeout_us() works by using it in i915
> > instead of the custom __wait_for().
> >
> > Remaining difference between two:
> >                | poll_timeout_us() | __wait_for()
> > ---------------------------------------------------
> > backoff        | fixed interval    | exponential
> > usleep_range() | N/4+1 to N        | N to N*2
> > clock          | MONOTONIC         | MONOTONIC_RAW
> >
> > Just a test hack for now, proper conversion probably
> > needs actual thought.
> 
> Agreed.
> 
> I feel pretty strongly about converting everything to use
> poll_timeout_us() and poll_timeout_us_atomic() directly. I think the
> plethora of wait_for variants in i915_utils.h is more confusing than
> helpful (even if some of them are supposed to be "simpler"
> alternatives). I also think the separate atomic variant is better than
> magically deciding that based on delay length.
> 
> I'm also not all that convinced about the exponential wait. Not all of
> the wait_for versions use it, and then it needs to have a max wait
> anyway (we have an issue with xe not having that [1]). I believe callers
> can decide on a sleep length that is appropriate for the timeout, case
> by case, and gut feeling says it's probably fine. ;)

Yeah, we've not really done any work to justify the polling interval/backoff
strategy. At some point it would be nice to collect some statistics to see
what the typical wait durations are, and then perhaps tune the polling
interval on a case by case basis to be at least somewhat optimal (short
enough to not cause significant delays, but long enough to avoid excessive
polling).

-- 
Ville Syrjälä
Intel