Re: [PATCH i-g-t] lib/igt_store: Fix unexpected reuse of allocated offset

Hi Janusz,

On 2026-07-20 at 07:59:10 +0200, Janusz Krzysztofik wrote:
> CI was extremely sporadically (once a month or two) reporting an issue
> with unexpected premature completion of a spin supposed to be still
> running.  It occurred possible to reproduce the issue in a few hours by
> running the test in a loop on one of affected platforms.
> 
> Having a closer look at the logs, it became clear that the test was
> blocking somewhere before the check for the spin still busy,
"check for the spin still busy" -> I feel like you are missing a
verb in here, like "was still busy".

Apart from that, LGTM:
Reviewed-by: Krzysztof Karas <[email protected]>

> and the spin
> was meanwhile aborted by a request watchdog timer after a default period
> of 20s of its activity.
> 
> Deeper debugging revealed that one of the test subprocesses was trying to
> evict a VMA and was waiting infinitely for its idleness.  Since the scope
> of affected platforms was limited to Gen12, where no kernel side
> relocations are used, Chris suggested that must have been a userspace bug,
> i.e., the test was apparently providing conflicting object offsets.
> Further analysis also shown that the main process of the test was looping
> infinitely around a page fault that occurred by reading another, mmapped
> GEM object shared with subprocesses, apparently locked by the subprocess
> waiting on eviction.
> 
> Based on all those observations, Zbigniew noticed incorrect use of
> intel_allocator library, leading to unexpected reuse of a just closed
> object handle, and its already allocated offset, by another child process
> before the allocator drops a link of the offset to the handle.  That
> theory was then confirmed by:
> a) forcing a single fixed offset to be used by each child process, which
>    resulted in immediate reproduction of the issue on first attempt, and
> b) extending a potential race window by introducing a short delay between
>    GEM closing the object of the VMA and the allocator forgetting the link
>    between its handle and offset, which also resulted in prompt
>    reproduction of the issue.
> 
> Close the race window by reordering those two cleanup operations inside an
> IGT store function: call put_offset() first, then gem_close(), so other
> test processes have no chance to pick up the same offset when reusing the
> just closed GEM object handle.
> 
> Correctness of the proposed solution was confirmed by running the fixed
> test in a loop for 12 hours with no issues.
> 
> Closes: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/15263
> Suggested-by: Chris Wilson <[email protected]>
> Suggested-by: Zbigniew Kempczyński <[email protected]>
> Signed-off-by: Janusz Krzysztofik <[email protected]>
> ---
>  lib/igt_store.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/lib/igt_store.c b/lib/igt_store.c
> index 42ffdc5cdb..ce29355c3d 100644
> --- a/lib/igt_store.c
d> +++ b/lib/igt_store.c
> @@ -95,6 +95,6 @@ void igt_store_word(int fd, uint64_t ahnd, const intel_ctx_t *ctx,
>  	batch[++i] = MI_BATCH_BUFFER_END;
>  	gem_write(fd, obj[BATCH].handle, 0, batch, sizeof(batch));
>  	gem_execbuf(fd, &execbuf);
> -	gem_close(fd, obj[BATCH].handle);
>  	put_offset(ahnd, obj[BATCH].handle);
> +	gem_close(fd, obj[BATCH].handle);
>  }
> -- 
> 2.54.0
> 

-- 
Best Regards,
Krzysztof