Re: [PATCH i-g-t] tests/intel/xe_exec_reset: Add a vm_unbind after the stress test completes

["Summers, Stuart" <stuart.summers@intel.com>]

On Wed, 2025-01-08 at 23:38 +0000, Cavitt, Jonathan wrote:
> -----Original Message-----
> From: igt-dev <igt-dev-bounces@lists.freedesktop.org> On Behalf Of
> Stuart Summers
> Sent: Wednesday, January 8, 2025 2:54 PM
> Cc: igt-dev@lists.freedesktop.org; Summers, Stuart
> <stuart.summers@intel.com>
> Subject: [PATCH i-g-t] tests/intel/xe_exec_reset: Add a vm_unbind
> after the stress test completes
> > 
> > The test is submitting workloads in a tight loop and then
> > destroying the exec queue for each of these. There is a
> > potential that the GuC IDs could get used up during these
> > submissions and before a GT reset goes through. If that
> > happens, the subsequent submissions will essentially be
> > "lost" in that they aren't submitted to GuC and just wait
> > for a submission timeout to happen, at which point we do
> > a reset_async in the driver. What seems to be happening
> > however is these workloads are lingering beyond the
> > completion of the xe_exec_reset test. Then when the
> > reset_async eventually goes through, the submissions
> > from a next test can also hang (or possibly because
> > they too ran into the guc_id exhaustion case).
> > 
> > Add an explicit vm_unbind before destroying the VM
> > to give time for those submissions to complete
> > before the test ends.
> > 
> > https://gitlab.freedesktop.org/drm/xe/kernel/issues/4015
> 
> I think this needs to be:
> Closes: https://gitlab.freedesktop.org/drm/xe/kernel/issues/4015

Ok I'll update. 

> But otherwise:
> Reviewed-by: Jonathan Cavitt <jonathan.cavitt@intel.com>

Thanks!

> Well, assuming this works, at least.  I might be reading a bit too
> far into the
> commit message and kernel issue comment, but it seems you aren't too
> certain this will work?

Yeah before merging I need to make sure this fully fixes the issue :)
This particular test was already passing so just need to review the
logs and make sure the cleanup is consistently happening. I'll report
back here with the details...

Thanks,
Stuart

> -Jonathan Cavitt
> 
> > 
> > Signed-off-by: Stuart Summers <stuart.summers@intel.com>
> > ---
> >  tests/intel/xe_exec_reset.c | 2 ++
> >  1 file changed, 2 insertions(+)
> > 
> > diff --git a/tests/intel/xe_exec_reset.c
> > b/tests/intel/xe_exec_reset.c
> > index a3eaf8bbf..ca5566d11 100644
> > --- a/tests/intel/xe_exec_reset.c
> > +++ b/tests/intel/xe_exec_reset.c
> > @@ -645,6 +645,8 @@ static void submit_jobs(struct gt_thread_data
> > *t)
> >                 xe_exec_queue_destroy(fd, exec.exec_queue_id);
> >         }
> >  
> > +       xe_vm_unbind_sync(fd, vm, 0, addr, bo_size);
> > +
> >         munmap(data, bo_size);
> >         gem_close(fd, bo);
> >         xe_vm_destroy(fd, vm);
> > -- 
> > 2.34.1
> > 
> >