Hi Krzysztof, On Fri, Jan 30, 2026 at 07:45:08PM +0100, Krzysztof Niemiec wrote: > The i915_active selftests live_active_wait and live_active_retire > operate on an i915_active attached to a mock, empty request, created as > part of test setup. A fence is attached to this request to control when > the request is processed. The tests then wait for the completion of the > active with __i915_active_wait(), and the test is considered successful > if this results in setting a variable in the active callback. > > However, the behavior of __i915_active_wait() is such that if the > refcount for the active is 0, the function is almost completely skipped; > waiting on a already completed active yields no effect. This includes a > subsequent call to the retire() function of the active (which is the > callback that the test is interested about, and which dictates whether > its successful or not). So, if the active is completed before the > aforementioned call to __i915_active_wait(), the test will fail. > > Most of the test runs in a single thread, including creating the > request, creating the fence for it, signalling that fence, and calling > __i915_active_wait(). However, the request itself is handled > asynchronously. This creates a race condition where if the request is > completed after signalling the fence, but before waiting on its active, > the active callback will not be invoked, failing the test. > > Defer signalling the request's fence, to ensure the main test thread > gets to call __i915_active_wait() before request completion. > > v4: > - Lower the delay timeout to 50ms (Jonathan) > - Put the check on work_finished inside a helper function (Jonathan) > > v3: > - Embed the variables inside the live_active struct (Andi) > - Move the schedule_delayed_work call closer to the wait (Andi) > - Implement error handling in case an error state - the wait has > finished, but the deferred work didn't run - is somehow achieved (Andi) > > v2: > - Clarify the need for a fix a little more (Krzysztof K., Janusz) > > Closes: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/14808 > Signed-off-by: Krzysztof Niemiec <[email protected]>
BTW, I don't want to block this patch, I'm just not feeling comfortable at merging it and I don't have better suggestions. BTW, you already have consensus here: Reviewed-by: Sebastian Brzezinka <[email protected]> Reviewed-by: Krzysztof Karas <[email protected]> Reviewed-by: Jonathan Cavitt <[email protected]> And, BTW, can you please add comments through the lines so that people understand what you are doing. Moreover, as Janusz suggested I would also like to have a real use case description of the issue and how it appeared in our environment. Thanks, Andi
