Re: [PATCH i-g-t 2/2] tests/intel/gem_close_race: Skip offset probing without full-ppgtt

[Janusz Krzysztofik <janusz.krzysztofik@linux.intel.com>]

Hi Zbyszek,

Thanks for looking at this.

On Fri, 2026-09-25 at 10:40 +0200, Zbigniew Kempczyński wrote:
> On Thu, Sep 24, 2026 at 05:24:18PM +0200, Janusz Krzysztofik wrote:
> > gem_close_race computed exec_addr/data_addr via
> > gem_detect_safe_start_offset() and gem_detect_safe_alignment()
> > whenever the kernel disallowed relocations, without checking ppgtt
> > mode. Those helpers exist solely to bootstrap intel_allocator and are
> > meaningless without full-ppgtt, since the kernel can't see the ranges
> > intel_allocator considers reserved and may hand them out to other
> > users, as already acknowledged by __intel_bb_create() which enforces
> > relocations and skips the allocator entirely in that case.
> > 
> > Require gem_uses_full_ppgtt() as well before enabling the softpin
> > path, mirroring __intel_bb_create()'s logic, and skip the probing
> > calls otherwise so the existing relocation fallback is used.
> > 
> > Signed-off-by: Janusz Krzysztofik <janusz.krzysztofik@linux.intel.com>
> > Cc: Kamil Konieczny <kamil.konieczny@linux.intel.com>
> > Assisted-by: GitHub Copilot (Claude Sonnet 5)
> > ---
> >  tests/intel/gem_close_race.c | 19 ++++++++++++++-----
> >  1 file changed, 14 insertions(+), 5 deletions(-)
> > 
> > diff --git a/tests/intel/gem_close_race.c b/tests/intel/gem_close_race.c
> > index b2688774d7..ff2877f67e 100644
> > --- a/tests/intel/gem_close_race.c
> > +++ b/tests/intel/gem_close_race.c
> > @@ -348,11 +348,20 @@ int igt_main()
> >  
> >  		devid = intel_get_drm_devid(fd);
> >  		has_64bit_relocations = intel_gen(devid) >= 8;
> > -		has_softpin = !gem_has_relocations(fd);
> > -		exec_addr = gem_detect_safe_start_offset(fd);
> > -		data_addr = gem_detect_safe_alignment(fd);
> > -		exec_addr = max_t(exec_addr, exec_addr, data_addr);
> > -		data_addr += exec_addr;
> > +		/*
> > +		 * Without full-ppgtt the kernel can't see the ranges the
> > +		 * detected offsets are meant to reserve and may hand them
> > +		 * out to other users, so pinning at a detected offset isn't
> > +		 * meaningful; fall back to relocations in that case, same
> > +		 * as __intel_bb_create() does.
> > +		 */
> > +		has_softpin = !gem_has_relocations(fd) && gem_uses_full_ppgtt(fd);
> 
> Which platforms don't have relocations and are not using full ppgtt?

Correct, today all configurations without relocations also expose full
PPGTT, so the added check does not currently change runtime behavior.
However, the purpose of the check is not to distinguish existing platforms.
The probing helpers are conceptually valid only with full PPGTT, as they
rely on allocator-managed address space reservations being respected by the
kernel. Therefore I would prefer to keep the explicit gem_uses_full_ppgtt()
requirement to document that dependency and to keep the code aligned with
the logic already described in __intel_bb_create().  The condition is
intentionally expressing the semantic requirement of the helpers, not
distinguishing currently known hardware configurations.

Thanks,
Janusz

> 
> --
> Zbigniew
> 
> > +		if (has_softpin) {
> > +			exec_addr = gem_detect_safe_start_offset(fd);
> > +			data_addr = gem_detect_safe_alignment(fd);
> > +			exec_addr = max_t(exec_addr, exec_addr, data_addr);
> > +			data_addr += exec_addr;
> > +		}
> >  
> >  		gpu_count = igt_device_filter_count();
> >  
> > -- 
> > 2.55.0
> >