Re: [PATCH i-g-t v2 2/3] tests/intel/gem_lmem_swapping: Be more clear about subprocesses role

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

Hi Krzysztof,

Thanks for looking into this.

On Thu, 2026-03-19 at 07:20 +0000, Krzysztof Karas wrote:
> Hi Janusz,
> 
> > In the smem-oom subtest, helper processes are now spawn with igt_fork(),
> > not with igt_fork_helper() as one might expect.  That unfortunate use
> > of igt_fork() may introduce uncertainty about the role of those
> > subprocesses, whether their failures should count or not.
> > 
> > Use igt_fork_helper() for clarity.
> > 
> > Signed-off-by: Janusz Krzysztofik <janusz.krzysztofik@linux.intel.com>
> > ---
> >  tests/intel/gem_lmem_swapping.c | 12 ++++++++----
> >  1 file changed, 8 insertions(+), 4 deletions(-)
> > 
> > diff --git a/tests/intel/gem_lmem_swapping.c b/tests/intel/gem_lmem_swapping.c
> > index 3a35318a74..f790dc66e9 100644
> > --- a/tests/intel/gem_lmem_swapping.c
> > +++ b/tests/intel/gem_lmem_swapping.c
> > @@ -737,7 +737,9 @@ static void test_smem_oom(int i915,
> >  	/* smem memory hog processes, respawn till the lmem process completes */
> >  	igt_fork_helper(&smem_loop[0]) {
> >  		while (!READ_ONCE(*lmem_done)) {
> > -			igt_fork(child, 1) {
> > +			struct igt_helper_process smem_proc = {};
> > +
> > +			igt_fork_helper(&smem_proc) {
> >  				for (int pass = 0; pass < num_alloc; pass++) {
> >  					if (READ_ONCE(*lmem_done))
> >  						break;
> > @@ -749,12 +751,14 @@ static void test_smem_oom(int i915,
> >  			 * killed by the oom killer, don't call
> >  			 * igt_waitchildren because of the noise
> >  			 */
> This comment is no longer applicable, you should remove it.
> igt_waitchildren() is supposed to be for igt_fork-ed processes,
> so as you change the background fork to igt_helper_process type,
> this may be confusing if left in the code.

You are right, and my intention was like that but I missed it.
Unfortunately, the patch has been already applied to upstream.  But
that's not a problem, I'll prepare a follow-up patch with that fixed.

Thanks,
Janusz


> 
> After that change, feel free to add:
> Revieved-by: Krzysztof Karas <krzysztof.karas@intel.com>
> 
> > -			wait(NULL);
> > +			igt_wait_helper(&smem_proc);
> >  		}
> >  	}
> >  	igt_fork_helper(&smem_loop[1]) {
> >  		while (!READ_ONCE(*lmem_done)) {
> > -			igt_fork(child, 1) {
> > +			struct igt_helper_process smem_proc = {};
> > +
> > +			igt_fork_helper(&smem_proc) {
> >  				int fd = drm_reopen_driver(i915);
> >  
> >  				for (int pass = 0; pass < num_alloc; pass++) {
> > @@ -764,7 +768,7 @@ static void test_smem_oom(int i915,
> >  				}
> >  				drm_close_driver(fd);
> >  			}
> > -			wait(NULL);
> > +			igt_wait_helper(&smem_proc);
> >  		}
> >  	}
> >  
> > -- 
> > 2.53.0
> >