[v3,3/5] drm/i915/selftests: Replace mock_file hackery with drm's true fake
diff mbox series

Message ID 20191106142432.14022-4-chris@chris-wilson.co.uk
State New
Headers show
Series
  • [v3,1/5] drm: Move EXPORT_SYMBOL_FOR_TESTS_ONLY under a separate Kconfig
Related show

Commit Message

Chris Wilson Nov. 6, 2019, 2:24 p.m. UTC
As drm now exports a method to create an anonymous struct file around a
drm_device for internal use, make use of it to avoid our horrible hacks.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/Kconfig.debug            |  2 +
 .../gpu/drm/i915/gem/selftests/huge_pages.c   |  2 +-
 .../drm/i915/gem/selftests/i915_gem_context.c | 12 ++---
 .../i915/gem/selftests/i915_gem_object_blt.c  |  4 +-
 drivers/gpu/drm/i915/gt/selftest_context.c    |  4 +-
 drivers/gpu/drm/i915/gt/selftest_hangcheck.c  |  8 +--
 .../gpu/drm/i915/gt/selftest_workarounds.c    |  2 +-
 drivers/gpu/drm/i915/selftests/i915_gem.c     |  4 +-
 .../gpu/drm/i915/selftests/i915_gem_evict.c   |  2 +-
 drivers/gpu/drm/i915/selftests/i915_gem_gtt.c |  4 +-
 drivers/gpu/drm/i915/selftests/i915_request.c |  2 +-
 .../drm/i915/selftests/intel_memory_region.c  |  2 +-
 drivers/gpu/drm/i915/selftests/mock_drm.c     | 49 +++----------------
 drivers/gpu/drm/i915/selftests/mock_drm.h     |  8 ++-
 14 files changed, 39 insertions(+), 66 deletions(-)

Comments

Daniel Vetter Nov. 7, 2019, 8:39 a.m. UTC | #1
On Wed, Nov 06, 2019 at 02:24:30PM +0000, Chris Wilson wrote:
> As drm now exports a method to create an anonymous struct file around a
> drm_device for internal use, make use of it to avoid our horrible hacks.
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
>  drivers/gpu/drm/i915/Kconfig.debug            |  2 +
>  .../gpu/drm/i915/gem/selftests/huge_pages.c   |  2 +-
>  .../drm/i915/gem/selftests/i915_gem_context.c | 12 ++---
>  .../i915/gem/selftests/i915_gem_object_blt.c  |  4 +-
>  drivers/gpu/drm/i915/gt/selftest_context.c    |  4 +-
>  drivers/gpu/drm/i915/gt/selftest_hangcheck.c  |  8 +--
>  .../gpu/drm/i915/gt/selftest_workarounds.c    |  2 +-
>  drivers/gpu/drm/i915/selftests/i915_gem.c     |  4 +-
>  .../gpu/drm/i915/selftests/i915_gem_evict.c   |  2 +-
>  drivers/gpu/drm/i915/selftests/i915_gem_gtt.c |  4 +-
>  drivers/gpu/drm/i915/selftests/i915_request.c |  2 +-
>  .../drm/i915/selftests/intel_memory_region.c  |  2 +-
>  drivers/gpu/drm/i915/selftests/mock_drm.c     | 49 +++----------------
>  drivers/gpu/drm/i915/selftests/mock_drm.h     |  8 ++-
>  14 files changed, 39 insertions(+), 66 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/Kconfig.debug b/drivers/gpu/drm/i915/Kconfig.debug
> index ef123eb29168..1140525da75a 100644
> --- a/drivers/gpu/drm/i915/Kconfig.debug
> +++ b/drivers/gpu/drm/i915/Kconfig.debug
> @@ -37,6 +37,7 @@ config DRM_I915_DEBUG
>  	select X86_MSR # used by igt/pm_rpm
>  	select DRM_VGEM # used by igt/prime_vgem (dmabuf interop checks)
>  	select DRM_DEBUG_MM if DRM=y
> +	select DRM_EXPORT_FOR_TESTS if m
>  	select DRM_DEBUG_SELFTEST
>  	select DMABUF_SELFTESTS
>  	select SW_SYNC # signaling validation framework (igt/syncobj*)
> @@ -160,6 +161,7 @@ config DRM_I915_SELFTEST
>  	bool "Enable selftests upon driver load"
>  	depends on DRM_I915
>  	default n
> +	select DRM_EXPORT_FOR_TESTS if m
>  	select FAULT_INJECTION
>  	select PRIME_NUMBERS
>  	help
> diff --git a/drivers/gpu/drm/i915/gem/selftests/huge_pages.c b/drivers/gpu/drm/i915/gem/selftests/huge_pages.c
> index 688c49a24f32..06dad7b0db82 100644
> --- a/drivers/gpu/drm/i915/gem/selftests/huge_pages.c
> +++ b/drivers/gpu/drm/i915/gem/selftests/huge_pages.c
> @@ -1944,6 +1944,6 @@ int i915_gem_huge_page_live_selftests(struct drm_i915_private *i915)
>  	err = i915_subtests(tests, ctx);
>  
>  out_file:
> -	mock_file_free(i915, file);
> +	mock_file_put(file);
>  	return err;
>  }
> diff --git a/drivers/gpu/drm/i915/gem/selftests/i915_gem_context.c b/drivers/gpu/drm/i915/gem/selftests/i915_gem_context.c
> index 62fabc023a83..47890c92534c 100644
> --- a/drivers/gpu/drm/i915/gem/selftests/i915_gem_context.c
> +++ b/drivers/gpu/drm/i915/gem/selftests/i915_gem_context.c
> @@ -149,7 +149,7 @@ static int live_nop_switch(void *arg)
>  	}
>  
>  out_file:
> -	mock_file_free(i915, file);
> +	mock_file_put(file);
>  	return err;
>  }
>  
> @@ -377,7 +377,7 @@ static int live_parallel_switch(void *arg)
>  	}
>  	kfree(data);
>  out_file:
> -	mock_file_free(i915, file);
> +	mock_file_put(file);
>  	return err;
>  }
>  
> @@ -716,7 +716,7 @@ static int igt_ctx_exec(void *arg)
>  		if (igt_live_test_end(&t))
>  			err = -EIO;
>  
> -		mock_file_free(i915, file);
> +		mock_file_put(file);
>  		if (err)
>  			return err;
>  
> @@ -854,7 +854,7 @@ static int igt_shared_ctx_exec(void *arg)
>  	if (igt_live_test_end(&t))
>  		err = -EIO;
>  out_file:
> -	mock_file_free(i915, file);
> +	mock_file_put(file);
>  	return err;
>  }
>  
> @@ -1426,7 +1426,7 @@ static int igt_ctx_readonly(void *arg)
>  	if (igt_live_test_end(&t))
>  		err = -EIO;
>  
> -	mock_file_free(i915, file);
> +	mock_file_put(file);
>  	return err;
>  }
>  
> @@ -1750,7 +1750,7 @@ static int igt_vm_isolation(void *arg)
>  out_file:
>  	if (igt_live_test_end(&t))
>  		err = -EIO;
> -	mock_file_free(i915, file);
> +	mock_file_put(file);
>  	return err;
>  }
>  
> diff --git a/drivers/gpu/drm/i915/gem/selftests/i915_gem_object_blt.c b/drivers/gpu/drm/i915/gem/selftests/i915_gem_object_blt.c
> index e8132aca0bb6..d9fdfddb7091 100644
> --- a/drivers/gpu/drm/i915/gem/selftests/i915_gem_object_blt.c
> +++ b/drivers/gpu/drm/i915/gem/selftests/i915_gem_object_blt.c
> @@ -301,7 +301,7 @@ static int igt_fill_blt_thread(void *arg)
>  
>  	intel_context_put(ce);
>  out_file:
> -	mock_file_free(i915, file);
> +	mock_file_put(file);
>  	return err;
>  }
>  
> @@ -432,7 +432,7 @@ static int igt_copy_blt_thread(void *arg)
>  
>  	intel_context_put(ce);
>  out_file:
> -	mock_file_free(i915, file);
> +	mock_file_put(file);
>  	return err;
>  }
>  
> diff --git a/drivers/gpu/drm/i915/gt/selftest_context.c b/drivers/gpu/drm/i915/gt/selftest_context.c
> index bc720defc6b8..a5688f7d9073 100644
> --- a/drivers/gpu/drm/i915/gt/selftest_context.c
> +++ b/drivers/gpu/drm/i915/gt/selftest_context.c
> @@ -313,7 +313,7 @@ static int live_active_context(void *arg)
>  	}
>  
>  out_file:
> -	mock_file_free(gt->i915, file);
> +	mock_file_put(file);
>  	return err;
>  }
>  
> @@ -423,7 +423,7 @@ static int live_remote_context(void *arg)
>  	}
>  
>  out_file:
> -	mock_file_free(gt->i915, file);
> +	mock_file_put(file);
>  	return err;
>  }
>  
> diff --git a/drivers/gpu/drm/i915/gt/selftest_hangcheck.c b/drivers/gpu/drm/i915/gt/selftest_hangcheck.c
> index 85e9ccf5c304..cdaaee4432b2 100644
> --- a/drivers/gpu/drm/i915/gt/selftest_hangcheck.c
> +++ b/drivers/gpu/drm/i915/gt/selftest_hangcheck.c
> @@ -439,7 +439,7 @@ static int igt_reset_nop(void *arg)
>  
>  	err = igt_flush_test(gt->i915);
>  out:
> -	mock_file_free(gt->i915, file);
> +	mock_file_put(file);
>  	if (intel_gt_is_wedged(gt))
>  		err = -EIO;
>  	return err;
> @@ -535,7 +535,7 @@ static int igt_reset_nop_engine(void *arg)
>  
>  	err = igt_flush_test(gt->i915);
>  out:
> -	mock_file_free(gt->i915, file);
> +	mock_file_put(file);
>  	if (intel_gt_is_wedged(gt))
>  		err = -EIO;
>  	return err;
> @@ -752,7 +752,7 @@ static int active_engine(void *data)
>  	}
>  
>  err_file:
> -	mock_file_free(engine->i915, file);
> +	mock_file_put(file);
>  	return err;
>  }
>  
> @@ -1325,7 +1325,7 @@ static int igt_reset_evict_ppgtt(void *arg)
>  	i915_vm_put(vm);
>  
>  out:
> -	mock_file_free(gt->i915, file);
> +	mock_file_put(file);
>  	return err;
>  }
>  
> diff --git a/drivers/gpu/drm/i915/gt/selftest_workarounds.c b/drivers/gpu/drm/i915/gt/selftest_workarounds.c
> index abce6e4ec9c0..5c69ec5c5ef9 100644
> --- a/drivers/gpu/drm/i915/gt/selftest_workarounds.c
> +++ b/drivers/gpu/drm/i915/gt/selftest_workarounds.c
> @@ -739,7 +739,7 @@ static int live_dirty_whitelist(void *arg)
>  	}
>  
>  out_file:
> -	mock_file_free(gt->i915, file);
> +	mock_file_put(file);
>  	return err;
>  }
>  
> diff --git a/drivers/gpu/drm/i915/selftests/i915_gem.c b/drivers/gpu/drm/i915/selftests/i915_gem.c
> index d83f6bf6d9d4..aa6282adfd09 100644
> --- a/drivers/gpu/drm/i915/selftests/i915_gem.c
> +++ b/drivers/gpu/drm/i915/selftests/i915_gem.c
> @@ -163,7 +163,7 @@ static int igt_gem_suspend(void *arg)
>  
>  	err = switch_to_context(ctx);
>  out:
> -	mock_file_free(i915, file);
> +	mock_file_put(file);
>  	return err;
>  }
>  
> @@ -198,7 +198,7 @@ static int igt_gem_hibernate(void *arg)
>  
>  	err = switch_to_context(ctx);
>  out:
> -	mock_file_free(i915, file);
> +	mock_file_put(file);
>  	return err;
>  }
>  
> diff --git a/drivers/gpu/drm/i915/selftests/i915_gem_evict.c b/drivers/gpu/drm/i915/selftests/i915_gem_evict.c
> index 42e948144f1b..41092dcea5b1 100644
> --- a/drivers/gpu/drm/i915/selftests/i915_gem_evict.c
> +++ b/drivers/gpu/drm/i915/selftests/i915_gem_evict.c
> @@ -515,7 +515,7 @@ static int igt_evict_contexts(void *arg)
>  		pr_info("Submitted %lu contexts/requests on %s\n",
>  			count, engine->name);
>  
> -		mock_file_free(i915, file);
> +		mock_file_put(file);
>  		if (err)
>  			break;
>  	}
> diff --git a/drivers/gpu/drm/i915/selftests/i915_gem_gtt.c b/drivers/gpu/drm/i915/selftests/i915_gem_gtt.c
> index 3f7e80fb3bbd..c3e0d63c4d0c 100644
> --- a/drivers/gpu/drm/i915/selftests/i915_gem_gtt.c
> +++ b/drivers/gpu/drm/i915/selftests/i915_gem_gtt.c
> @@ -1026,7 +1026,7 @@ static int exercise_ppgtt(struct drm_i915_private *dev_priv,
>  	i915_vm_put(&ppgtt->vm);
>  
>  out_free:
> -	mock_file_free(dev_priv, file);
> +	mock_file_put(file);
>  	return err;
>  }
>  
> @@ -2022,7 +2022,7 @@ static int igt_cs_tlb(void *arg)
>  out_vm:
>  	i915_vm_put(vm);
>  out_unlock:
> -	mock_file_free(i915, file);
> +	mock_file_put(file);
>  	return err;
>  }
>  
> diff --git a/drivers/gpu/drm/i915/selftests/i915_request.c b/drivers/gpu/drm/i915/selftests/i915_request.c
> index 9e6d3159cd80..7c56ee38cc5b 100644
> --- a/drivers/gpu/drm/i915/selftests/i915_request.c
> +++ b/drivers/gpu/drm/i915/selftests/i915_request.c
> @@ -1430,7 +1430,7 @@ static int live_breadcrumbs_smoketest(void *arg)
>  out_smoke:
>  	kfree(smoke);
>  out_file:
> -	mock_file_free(i915, file);
> +	mock_file_put(file);
>  out_rpm:
>  	intel_runtime_pm_put(&i915->runtime_pm, wakeref);
>  
> diff --git a/drivers/gpu/drm/i915/selftests/intel_memory_region.c b/drivers/gpu/drm/i915/selftests/intel_memory_region.c
> index 19e1cca8f143..fa283ce4532a 100644
> --- a/drivers/gpu/drm/i915/selftests/intel_memory_region.c
> +++ b/drivers/gpu/drm/i915/selftests/intel_memory_region.c
> @@ -439,7 +439,7 @@ static int igt_lmem_write_gpu(void *arg)
>  out_put:
>  	i915_gem_object_put(obj);
>  out_file:
> -	mock_file_free(i915, file);
> +	mock_file_put(file);
>  	return err;
>  }
>  
> diff --git a/drivers/gpu/drm/i915/selftests/mock_drm.c b/drivers/gpu/drm/i915/selftests/mock_drm.c
> index 09c704153456..c100c3efe239 100644
> --- a/drivers/gpu/drm/i915/selftests/mock_drm.c
> +++ b/drivers/gpu/drm/i915/selftests/mock_drm.c
> @@ -22,52 +22,17 @@
>   *
>   */
>  
> +#include <drm/drm_file.h>
> +
>  #include "mock_drm.h"
>  
>  struct drm_file *mock_file(struct drm_i915_private *i915)
>  {
> -	struct file *filp;
> -	struct inode *inode;
> -	struct drm_file *file;
> -	int err;
> -
> -	inode = kzalloc(sizeof(*inode), GFP_KERNEL);
> -	if (!inode) {
> -		err = -ENOMEM;
> -		goto err;
> -	}
> -
> -	inode->i_rdev = i915->drm.primary->index;
> -
> -	filp = kzalloc(sizeof(*filp), GFP_KERNEL);
> -	if (!filp) {
> -		err = -ENOMEM;
> -		goto err_inode;
> -	}
> -
> -	err = drm_open(inode, filp);
> -	if (err)
> -		goto err_filp;
> +	struct file *file;
>  
> -	file = filp->private_data;
> -	memset(&file->filp, POISON_INUSE, sizeof(file->filp));
> -	file->authenticated = true;
> -
> -	kfree(filp);
> -	kfree(inode);
> -	return file;
> -
> -err_filp:
> -	kfree(filp);
> -err_inode:
> -	kfree(inode);
> -err:
> -	return ERR_PTR(err);
> -}
> -
> -void mock_file_free(struct drm_i915_private *i915, struct drm_file *file)
> -{
> -	struct file filp = { .private_data = file };
> +	file = mock_drm_getfile(i915->drm.primary, O_RDWR);
> +	if (IS_ERR(file))
> +		return ERR_CAST(file);
>  
> -	drm_release(NULL, &filp);
> +	return file->private_data;

Hm looking at this patch seems like the function interface for the core
mock helper is wrong, and we want a drm_file there. Then you could also
simplify this wrapper a notch more.

>  }
> diff --git a/drivers/gpu/drm/i915/selftests/mock_drm.h b/drivers/gpu/drm/i915/selftests/mock_drm.h
> index b39beee9f8f6..dc4190bd3f5a 100644
> --- a/drivers/gpu/drm/i915/selftests/mock_drm.h
> +++ b/drivers/gpu/drm/i915/selftests/mock_drm.h
> @@ -25,7 +25,13 @@
>  #ifndef __MOCK_DRM_H
>  #define __MOCK_DRM_H
>  
> +struct drm_file;
> +struct drm_i915_private;
> +
>  struct drm_file *mock_file(struct drm_i915_private *i915);
> -void mock_file_free(struct drm_i915_private *i915, struct drm_file *file);
> +static inline void mock_file_put(struct drm_file *file)

I still think this would be nice next to the core mock_drm_open helper, as
maybe mock_drm_close. Suitable bikeshedding on the names included :-)
-Daniel

> +{
> +	fput(file->filp);
> +}
>  
>  #endif /* !__MOCK_DRM_H */
> -- 
> 2.24.0
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Chris Wilson Nov. 7, 2019, 5:17 p.m. UTC | #2
Quoting Daniel Vetter (2019-11-07 08:39:24)
> On Wed, Nov 06, 2019 at 02:24:30PM +0000, Chris Wilson wrote:
> > As drm now exports a method to create an anonymous struct file around a
> > drm_device for internal use, make use of it to avoid our horrible hacks.
> > 
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > ---
> >  drivers/gpu/drm/i915/Kconfig.debug            |  2 +
> >  .../gpu/drm/i915/gem/selftests/huge_pages.c   |  2 +-
> >  .../drm/i915/gem/selftests/i915_gem_context.c | 12 ++---
> >  .../i915/gem/selftests/i915_gem_object_blt.c  |  4 +-
> >  drivers/gpu/drm/i915/gt/selftest_context.c    |  4 +-
> >  drivers/gpu/drm/i915/gt/selftest_hangcheck.c  |  8 +--
> >  .../gpu/drm/i915/gt/selftest_workarounds.c    |  2 +-
> >  drivers/gpu/drm/i915/selftests/i915_gem.c     |  4 +-
> >  .../gpu/drm/i915/selftests/i915_gem_evict.c   |  2 +-
> >  drivers/gpu/drm/i915/selftests/i915_gem_gtt.c |  4 +-
> >  drivers/gpu/drm/i915/selftests/i915_request.c |  2 +-
> >  .../drm/i915/selftests/intel_memory_region.c  |  2 +-
> >  drivers/gpu/drm/i915/selftests/mock_drm.c     | 49 +++----------------
> >  drivers/gpu/drm/i915/selftests/mock_drm.h     |  8 ++-
> >  14 files changed, 39 insertions(+), 66 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/Kconfig.debug b/drivers/gpu/drm/i915/Kconfig.debug
> > index ef123eb29168..1140525da75a 100644
> > --- a/drivers/gpu/drm/i915/Kconfig.debug
> > +++ b/drivers/gpu/drm/i915/Kconfig.debug
> > @@ -37,6 +37,7 @@ config DRM_I915_DEBUG
> >       select X86_MSR # used by igt/pm_rpm
> >       select DRM_VGEM # used by igt/prime_vgem (dmabuf interop checks)
> >       select DRM_DEBUG_MM if DRM=y
> > +     select DRM_EXPORT_FOR_TESTS if m
> >       select DRM_DEBUG_SELFTEST
> >       select DMABUF_SELFTESTS
> >       select SW_SYNC # signaling validation framework (igt/syncobj*)
> > @@ -160,6 +161,7 @@ config DRM_I915_SELFTEST
> >       bool "Enable selftests upon driver load"
> >       depends on DRM_I915
> >       default n
> > +     select DRM_EXPORT_FOR_TESTS if m
> >       select FAULT_INJECTION
> >       select PRIME_NUMBERS
> >       help
> > diff --git a/drivers/gpu/drm/i915/gem/selftests/huge_pages.c b/drivers/gpu/drm/i915/gem/selftests/huge_pages.c
> > index 688c49a24f32..06dad7b0db82 100644
> > --- a/drivers/gpu/drm/i915/gem/selftests/huge_pages.c
> > +++ b/drivers/gpu/drm/i915/gem/selftests/huge_pages.c
> > @@ -1944,6 +1944,6 @@ int i915_gem_huge_page_live_selftests(struct drm_i915_private *i915)
> >       err = i915_subtests(tests, ctx);
> >  
> >  out_file:
> > -     mock_file_free(i915, file);
> > +     mock_file_put(file);
> >       return err;
> >  }
> > diff --git a/drivers/gpu/drm/i915/gem/selftests/i915_gem_context.c b/drivers/gpu/drm/i915/gem/selftests/i915_gem_context.c
> > index 62fabc023a83..47890c92534c 100644
> > --- a/drivers/gpu/drm/i915/gem/selftests/i915_gem_context.c
> > +++ b/drivers/gpu/drm/i915/gem/selftests/i915_gem_context.c
> > @@ -149,7 +149,7 @@ static int live_nop_switch(void *arg)
> >       }
> >  
> >  out_file:
> > -     mock_file_free(i915, file);
> > +     mock_file_put(file);
> >       return err;
> >  }
> >  
> > @@ -377,7 +377,7 @@ static int live_parallel_switch(void *arg)
> >       }
> >       kfree(data);
> >  out_file:
> > -     mock_file_free(i915, file);
> > +     mock_file_put(file);
> >       return err;
> >  }
> >  
> > @@ -716,7 +716,7 @@ static int igt_ctx_exec(void *arg)
> >               if (igt_live_test_end(&t))
> >                       err = -EIO;
> >  
> > -             mock_file_free(i915, file);
> > +             mock_file_put(file);
> >               if (err)
> >                       return err;
> >  
> > @@ -854,7 +854,7 @@ static int igt_shared_ctx_exec(void *arg)
> >       if (igt_live_test_end(&t))
> >               err = -EIO;
> >  out_file:
> > -     mock_file_free(i915, file);
> > +     mock_file_put(file);
> >       return err;
> >  }
> >  
> > @@ -1426,7 +1426,7 @@ static int igt_ctx_readonly(void *arg)
> >       if (igt_live_test_end(&t))
> >               err = -EIO;
> >  
> > -     mock_file_free(i915, file);
> > +     mock_file_put(file);
> >       return err;
> >  }
> >  
> > @@ -1750,7 +1750,7 @@ static int igt_vm_isolation(void *arg)
> >  out_file:
> >       if (igt_live_test_end(&t))
> >               err = -EIO;
> > -     mock_file_free(i915, file);
> > +     mock_file_put(file);
> >       return err;
> >  }
> >  
> > diff --git a/drivers/gpu/drm/i915/gem/selftests/i915_gem_object_blt.c b/drivers/gpu/drm/i915/gem/selftests/i915_gem_object_blt.c
> > index e8132aca0bb6..d9fdfddb7091 100644
> > --- a/drivers/gpu/drm/i915/gem/selftests/i915_gem_object_blt.c
> > +++ b/drivers/gpu/drm/i915/gem/selftests/i915_gem_object_blt.c
> > @@ -301,7 +301,7 @@ static int igt_fill_blt_thread(void *arg)
> >  
> >       intel_context_put(ce);
> >  out_file:
> > -     mock_file_free(i915, file);
> > +     mock_file_put(file);
> >       return err;
> >  }
> >  
> > @@ -432,7 +432,7 @@ static int igt_copy_blt_thread(void *arg)
> >  
> >       intel_context_put(ce);
> >  out_file:
> > -     mock_file_free(i915, file);
> > +     mock_file_put(file);
> >       return err;
> >  }
> >  
> > diff --git a/drivers/gpu/drm/i915/gt/selftest_context.c b/drivers/gpu/drm/i915/gt/selftest_context.c
> > index bc720defc6b8..a5688f7d9073 100644
> > --- a/drivers/gpu/drm/i915/gt/selftest_context.c
> > +++ b/drivers/gpu/drm/i915/gt/selftest_context.c
> > @@ -313,7 +313,7 @@ static int live_active_context(void *arg)
> >       }
> >  
> >  out_file:
> > -     mock_file_free(gt->i915, file);
> > +     mock_file_put(file);
> >       return err;
> >  }
> >  
> > @@ -423,7 +423,7 @@ static int live_remote_context(void *arg)
> >       }
> >  
> >  out_file:
> > -     mock_file_free(gt->i915, file);
> > +     mock_file_put(file);
> >       return err;
> >  }
> >  
> > diff --git a/drivers/gpu/drm/i915/gt/selftest_hangcheck.c b/drivers/gpu/drm/i915/gt/selftest_hangcheck.c
> > index 85e9ccf5c304..cdaaee4432b2 100644
> > --- a/drivers/gpu/drm/i915/gt/selftest_hangcheck.c
> > +++ b/drivers/gpu/drm/i915/gt/selftest_hangcheck.c
> > @@ -439,7 +439,7 @@ static int igt_reset_nop(void *arg)
> >  
> >       err = igt_flush_test(gt->i915);
> >  out:
> > -     mock_file_free(gt->i915, file);
> > +     mock_file_put(file);
> >       if (intel_gt_is_wedged(gt))
> >               err = -EIO;
> >       return err;
> > @@ -535,7 +535,7 @@ static int igt_reset_nop_engine(void *arg)
> >  
> >       err = igt_flush_test(gt->i915);
> >  out:
> > -     mock_file_free(gt->i915, file);
> > +     mock_file_put(file);
> >       if (intel_gt_is_wedged(gt))
> >               err = -EIO;
> >       return err;
> > @@ -752,7 +752,7 @@ static int active_engine(void *data)
> >       }
> >  
> >  err_file:
> > -     mock_file_free(engine->i915, file);
> > +     mock_file_put(file);
> >       return err;
> >  }
> >  
> > @@ -1325,7 +1325,7 @@ static int igt_reset_evict_ppgtt(void *arg)
> >       i915_vm_put(vm);
> >  
> >  out:
> > -     mock_file_free(gt->i915, file);
> > +     mock_file_put(file);
> >       return err;
> >  }
> >  
> > diff --git a/drivers/gpu/drm/i915/gt/selftest_workarounds.c b/drivers/gpu/drm/i915/gt/selftest_workarounds.c
> > index abce6e4ec9c0..5c69ec5c5ef9 100644
> > --- a/drivers/gpu/drm/i915/gt/selftest_workarounds.c
> > +++ b/drivers/gpu/drm/i915/gt/selftest_workarounds.c
> > @@ -739,7 +739,7 @@ static int live_dirty_whitelist(void *arg)
> >       }
> >  
> >  out_file:
> > -     mock_file_free(gt->i915, file);
> > +     mock_file_put(file);
> >       return err;
> >  }
> >  
> > diff --git a/drivers/gpu/drm/i915/selftests/i915_gem.c b/drivers/gpu/drm/i915/selftests/i915_gem.c
> > index d83f6bf6d9d4..aa6282adfd09 100644
> > --- a/drivers/gpu/drm/i915/selftests/i915_gem.c
> > +++ b/drivers/gpu/drm/i915/selftests/i915_gem.c
> > @@ -163,7 +163,7 @@ static int igt_gem_suspend(void *arg)
> >  
> >       err = switch_to_context(ctx);
> >  out:
> > -     mock_file_free(i915, file);
> > +     mock_file_put(file);
> >       return err;
> >  }
> >  
> > @@ -198,7 +198,7 @@ static int igt_gem_hibernate(void *arg)
> >  
> >       err = switch_to_context(ctx);
> >  out:
> > -     mock_file_free(i915, file);
> > +     mock_file_put(file);
> >       return err;
> >  }
> >  
> > diff --git a/drivers/gpu/drm/i915/selftests/i915_gem_evict.c b/drivers/gpu/drm/i915/selftests/i915_gem_evict.c
> > index 42e948144f1b..41092dcea5b1 100644
> > --- a/drivers/gpu/drm/i915/selftests/i915_gem_evict.c
> > +++ b/drivers/gpu/drm/i915/selftests/i915_gem_evict.c
> > @@ -515,7 +515,7 @@ static int igt_evict_contexts(void *arg)
> >               pr_info("Submitted %lu contexts/requests on %s\n",
> >                       count, engine->name);
> >  
> > -             mock_file_free(i915, file);
> > +             mock_file_put(file);
> >               if (err)
> >                       break;
> >       }
> > diff --git a/drivers/gpu/drm/i915/selftests/i915_gem_gtt.c b/drivers/gpu/drm/i915/selftests/i915_gem_gtt.c
> > index 3f7e80fb3bbd..c3e0d63c4d0c 100644
> > --- a/drivers/gpu/drm/i915/selftests/i915_gem_gtt.c
> > +++ b/drivers/gpu/drm/i915/selftests/i915_gem_gtt.c
> > @@ -1026,7 +1026,7 @@ static int exercise_ppgtt(struct drm_i915_private *dev_priv,
> >       i915_vm_put(&ppgtt->vm);
> >  
> >  out_free:
> > -     mock_file_free(dev_priv, file);
> > +     mock_file_put(file);
> >       return err;
> >  }
> >  
> > @@ -2022,7 +2022,7 @@ static int igt_cs_tlb(void *arg)
> >  out_vm:
> >       i915_vm_put(vm);
> >  out_unlock:
> > -     mock_file_free(i915, file);
> > +     mock_file_put(file);
> >       return err;
> >  }
> >  
> > diff --git a/drivers/gpu/drm/i915/selftests/i915_request.c b/drivers/gpu/drm/i915/selftests/i915_request.c
> > index 9e6d3159cd80..7c56ee38cc5b 100644
> > --- a/drivers/gpu/drm/i915/selftests/i915_request.c
> > +++ b/drivers/gpu/drm/i915/selftests/i915_request.c
> > @@ -1430,7 +1430,7 @@ static int live_breadcrumbs_smoketest(void *arg)
> >  out_smoke:
> >       kfree(smoke);
> >  out_file:
> > -     mock_file_free(i915, file);
> > +     mock_file_put(file);
> >  out_rpm:
> >       intel_runtime_pm_put(&i915->runtime_pm, wakeref);
> >  
> > diff --git a/drivers/gpu/drm/i915/selftests/intel_memory_region.c b/drivers/gpu/drm/i915/selftests/intel_memory_region.c
> > index 19e1cca8f143..fa283ce4532a 100644
> > --- a/drivers/gpu/drm/i915/selftests/intel_memory_region.c
> > +++ b/drivers/gpu/drm/i915/selftests/intel_memory_region.c
> > @@ -439,7 +439,7 @@ static int igt_lmem_write_gpu(void *arg)
> >  out_put:
> >       i915_gem_object_put(obj);
> >  out_file:
> > -     mock_file_free(i915, file);
> > +     mock_file_put(file);
> >       return err;
> >  }
> >  
> > diff --git a/drivers/gpu/drm/i915/selftests/mock_drm.c b/drivers/gpu/drm/i915/selftests/mock_drm.c
> > index 09c704153456..c100c3efe239 100644
> > --- a/drivers/gpu/drm/i915/selftests/mock_drm.c
> > +++ b/drivers/gpu/drm/i915/selftests/mock_drm.c
> > @@ -22,52 +22,17 @@
> >   *
> >   */
> >  
> > +#include <drm/drm_file.h>
> > +
> >  #include "mock_drm.h"
> >  
> >  struct drm_file *mock_file(struct drm_i915_private *i915)
> >  {
> > -     struct file *filp;
> > -     struct inode *inode;
> > -     struct drm_file *file;
> > -     int err;
> > -
> > -     inode = kzalloc(sizeof(*inode), GFP_KERNEL);
> > -     if (!inode) {
> > -             err = -ENOMEM;
> > -             goto err;
> > -     }
> > -
> > -     inode->i_rdev = i915->drm.primary->index;
> > -
> > -     filp = kzalloc(sizeof(*filp), GFP_KERNEL);
> > -     if (!filp) {
> > -             err = -ENOMEM;
> > -             goto err_inode;
> > -     }
> > -
> > -     err = drm_open(inode, filp);
> > -     if (err)
> > -             goto err_filp;
> > +     struct file *file;
> >  
> > -     file = filp->private_data;
> > -     memset(&file->filp, POISON_INUSE, sizeof(file->filp));
> > -     file->authenticated = true;
> > -
> > -     kfree(filp);
> > -     kfree(inode);
> > -     return file;
> > -
> > -err_filp:
> > -     kfree(filp);
> > -err_inode:
> > -     kfree(inode);
> > -err:
> > -     return ERR_PTR(err);
> > -}
> > -
> > -void mock_file_free(struct drm_i915_private *i915, struct drm_file *file)
> > -{
> > -     struct file filp = { .private_data = file };
> > +     file = mock_drm_getfile(i915->drm.primary, O_RDWR);
> > +     if (IS_ERR(file))
> > +             return ERR_CAST(file);
> >  
> > -     drm_release(NULL, &filp);
> > +     return file->private_data;
> 
> Hm looking at this patch seems like the function interface for the core
> mock helper is wrong, and we want a drm_file there. Then you could also
> simplify this wrapper a notch more.
> 
> >  }
> > diff --git a/drivers/gpu/drm/i915/selftests/mock_drm.h b/drivers/gpu/drm/i915/selftests/mock_drm.h
> > index b39beee9f8f6..dc4190bd3f5a 100644
> > --- a/drivers/gpu/drm/i915/selftests/mock_drm.h
> > +++ b/drivers/gpu/drm/i915/selftests/mock_drm.h
> > @@ -25,7 +25,13 @@
> >  #ifndef __MOCK_DRM_H
> >  #define __MOCK_DRM_H
> >  
> > +struct drm_file;
> > +struct drm_i915_private;
> > +
> >  struct drm_file *mock_file(struct drm_i915_private *i915);
> > -void mock_file_free(struct drm_i915_private *i915, struct drm_file *file);
> > +static inline void mock_file_put(struct drm_file *file)
> 
> I still think this would be nice next to the core mock_drm_open helper, as
> maybe mock_drm_close. Suitable bikeshedding on the names included :-)

My intention is to transition the tests to tracking struct file on
allocation and so use fput() directly, with a local struct drm_file as
appropriate. Nobody else is in the habit of creating anonymous struct
drm_file, just yet so moving this helper to core seems wasteful.
-Chris
Matthew Auld Nov. 7, 2019, 5:28 p.m. UTC | #3
On Wed, 6 Nov 2019 at 14:24, Chris Wilson <chris@chris-wilson.co.uk> wrote:
>
> As drm now exports a method to create an anonymous struct file around a
> drm_device for internal use, make use of it to avoid our horrible hacks.
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>

As per your eventual plan, fwiw,
Reviewed-by: Matthew Auld <matthew.auld@intel.com>
Daniel Vetter Nov. 7, 2019, 5:33 p.m. UTC | #4
On Thu, Nov 07, 2019 at 05:17:00PM +0000, Chris Wilson wrote:
> Quoting Daniel Vetter (2019-11-07 08:39:24)
> > On Wed, Nov 06, 2019 at 02:24:30PM +0000, Chris Wilson wrote:
> > > As drm now exports a method to create an anonymous struct file around a
> > > drm_device for internal use, make use of it to avoid our horrible hacks.
> > > 
> > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > > ---
> > >  drivers/gpu/drm/i915/Kconfig.debug            |  2 +
> > >  .../gpu/drm/i915/gem/selftests/huge_pages.c   |  2 +-
> > >  .../drm/i915/gem/selftests/i915_gem_context.c | 12 ++---
> > >  .../i915/gem/selftests/i915_gem_object_blt.c  |  4 +-
> > >  drivers/gpu/drm/i915/gt/selftest_context.c    |  4 +-
> > >  drivers/gpu/drm/i915/gt/selftest_hangcheck.c  |  8 +--
> > >  .../gpu/drm/i915/gt/selftest_workarounds.c    |  2 +-
> > >  drivers/gpu/drm/i915/selftests/i915_gem.c     |  4 +-
> > >  .../gpu/drm/i915/selftests/i915_gem_evict.c   |  2 +-
> > >  drivers/gpu/drm/i915/selftests/i915_gem_gtt.c |  4 +-
> > >  drivers/gpu/drm/i915/selftests/i915_request.c |  2 +-
> > >  .../drm/i915/selftests/intel_memory_region.c  |  2 +-
> > >  drivers/gpu/drm/i915/selftests/mock_drm.c     | 49 +++----------------
> > >  drivers/gpu/drm/i915/selftests/mock_drm.h     |  8 ++-
> > >  14 files changed, 39 insertions(+), 66 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/Kconfig.debug b/drivers/gpu/drm/i915/Kconfig.debug
> > > index ef123eb29168..1140525da75a 100644
> > > --- a/drivers/gpu/drm/i915/Kconfig.debug
> > > +++ b/drivers/gpu/drm/i915/Kconfig.debug
> > > @@ -37,6 +37,7 @@ config DRM_I915_DEBUG
> > >       select X86_MSR # used by igt/pm_rpm
> > >       select DRM_VGEM # used by igt/prime_vgem (dmabuf interop checks)
> > >       select DRM_DEBUG_MM if DRM=y
> > > +     select DRM_EXPORT_FOR_TESTS if m
> > >       select DRM_DEBUG_SELFTEST
> > >       select DMABUF_SELFTESTS
> > >       select SW_SYNC # signaling validation framework (igt/syncobj*)
> > > @@ -160,6 +161,7 @@ config DRM_I915_SELFTEST
> > >       bool "Enable selftests upon driver load"
> > >       depends on DRM_I915
> > >       default n
> > > +     select DRM_EXPORT_FOR_TESTS if m
> > >       select FAULT_INJECTION
> > >       select PRIME_NUMBERS
> > >       help
> > > diff --git a/drivers/gpu/drm/i915/gem/selftests/huge_pages.c b/drivers/gpu/drm/i915/gem/selftests/huge_pages.c
> > > index 688c49a24f32..06dad7b0db82 100644
> > > --- a/drivers/gpu/drm/i915/gem/selftests/huge_pages.c
> > > +++ b/drivers/gpu/drm/i915/gem/selftests/huge_pages.c
> > > @@ -1944,6 +1944,6 @@ int i915_gem_huge_page_live_selftests(struct drm_i915_private *i915)
> > >       err = i915_subtests(tests, ctx);
> > >  
> > >  out_file:
> > > -     mock_file_free(i915, file);
> > > +     mock_file_put(file);
> > >       return err;
> > >  }
> > > diff --git a/drivers/gpu/drm/i915/gem/selftests/i915_gem_context.c b/drivers/gpu/drm/i915/gem/selftests/i915_gem_context.c
> > > index 62fabc023a83..47890c92534c 100644
> > > --- a/drivers/gpu/drm/i915/gem/selftests/i915_gem_context.c
> > > +++ b/drivers/gpu/drm/i915/gem/selftests/i915_gem_context.c
> > > @@ -149,7 +149,7 @@ static int live_nop_switch(void *arg)
> > >       }
> > >  
> > >  out_file:
> > > -     mock_file_free(i915, file);
> > > +     mock_file_put(file);
> > >       return err;
> > >  }
> > >  
> > > @@ -377,7 +377,7 @@ static int live_parallel_switch(void *arg)
> > >       }
> > >       kfree(data);
> > >  out_file:
> > > -     mock_file_free(i915, file);
> > > +     mock_file_put(file);
> > >       return err;
> > >  }
> > >  
> > > @@ -716,7 +716,7 @@ static int igt_ctx_exec(void *arg)
> > >               if (igt_live_test_end(&t))
> > >                       err = -EIO;
> > >  
> > > -             mock_file_free(i915, file);
> > > +             mock_file_put(file);
> > >               if (err)
> > >                       return err;
> > >  
> > > @@ -854,7 +854,7 @@ static int igt_shared_ctx_exec(void *arg)
> > >       if (igt_live_test_end(&t))
> > >               err = -EIO;
> > >  out_file:
> > > -     mock_file_free(i915, file);
> > > +     mock_file_put(file);
> > >       return err;
> > >  }
> > >  
> > > @@ -1426,7 +1426,7 @@ static int igt_ctx_readonly(void *arg)
> > >       if (igt_live_test_end(&t))
> > >               err = -EIO;
> > >  
> > > -     mock_file_free(i915, file);
> > > +     mock_file_put(file);
> > >       return err;
> > >  }
> > >  
> > > @@ -1750,7 +1750,7 @@ static int igt_vm_isolation(void *arg)
> > >  out_file:
> > >       if (igt_live_test_end(&t))
> > >               err = -EIO;
> > > -     mock_file_free(i915, file);
> > > +     mock_file_put(file);
> > >       return err;
> > >  }
> > >  
> > > diff --git a/drivers/gpu/drm/i915/gem/selftests/i915_gem_object_blt.c b/drivers/gpu/drm/i915/gem/selftests/i915_gem_object_blt.c
> > > index e8132aca0bb6..d9fdfddb7091 100644
> > > --- a/drivers/gpu/drm/i915/gem/selftests/i915_gem_object_blt.c
> > > +++ b/drivers/gpu/drm/i915/gem/selftests/i915_gem_object_blt.c
> > > @@ -301,7 +301,7 @@ static int igt_fill_blt_thread(void *arg)
> > >  
> > >       intel_context_put(ce);
> > >  out_file:
> > > -     mock_file_free(i915, file);
> > > +     mock_file_put(file);
> > >       return err;
> > >  }
> > >  
> > > @@ -432,7 +432,7 @@ static int igt_copy_blt_thread(void *arg)
> > >  
> > >       intel_context_put(ce);
> > >  out_file:
> > > -     mock_file_free(i915, file);
> > > +     mock_file_put(file);
> > >       return err;
> > >  }
> > >  
> > > diff --git a/drivers/gpu/drm/i915/gt/selftest_context.c b/drivers/gpu/drm/i915/gt/selftest_context.c
> > > index bc720defc6b8..a5688f7d9073 100644
> > > --- a/drivers/gpu/drm/i915/gt/selftest_context.c
> > > +++ b/drivers/gpu/drm/i915/gt/selftest_context.c
> > > @@ -313,7 +313,7 @@ static int live_active_context(void *arg)
> > >       }
> > >  
> > >  out_file:
> > > -     mock_file_free(gt->i915, file);
> > > +     mock_file_put(file);
> > >       return err;
> > >  }
> > >  
> > > @@ -423,7 +423,7 @@ static int live_remote_context(void *arg)
> > >       }
> > >  
> > >  out_file:
> > > -     mock_file_free(gt->i915, file);
> > > +     mock_file_put(file);
> > >       return err;
> > >  }
> > >  
> > > diff --git a/drivers/gpu/drm/i915/gt/selftest_hangcheck.c b/drivers/gpu/drm/i915/gt/selftest_hangcheck.c
> > > index 85e9ccf5c304..cdaaee4432b2 100644
> > > --- a/drivers/gpu/drm/i915/gt/selftest_hangcheck.c
> > > +++ b/drivers/gpu/drm/i915/gt/selftest_hangcheck.c
> > > @@ -439,7 +439,7 @@ static int igt_reset_nop(void *arg)
> > >  
> > >       err = igt_flush_test(gt->i915);
> > >  out:
> > > -     mock_file_free(gt->i915, file);
> > > +     mock_file_put(file);
> > >       if (intel_gt_is_wedged(gt))
> > >               err = -EIO;
> > >       return err;
> > > @@ -535,7 +535,7 @@ static int igt_reset_nop_engine(void *arg)
> > >  
> > >       err = igt_flush_test(gt->i915);
> > >  out:
> > > -     mock_file_free(gt->i915, file);
> > > +     mock_file_put(file);
> > >       if (intel_gt_is_wedged(gt))
> > >               err = -EIO;
> > >       return err;
> > > @@ -752,7 +752,7 @@ static int active_engine(void *data)
> > >       }
> > >  
> > >  err_file:
> > > -     mock_file_free(engine->i915, file);
> > > +     mock_file_put(file);
> > >       return err;
> > >  }
> > >  
> > > @@ -1325,7 +1325,7 @@ static int igt_reset_evict_ppgtt(void *arg)
> > >       i915_vm_put(vm);
> > >  
> > >  out:
> > > -     mock_file_free(gt->i915, file);
> > > +     mock_file_put(file);
> > >       return err;
> > >  }
> > >  
> > > diff --git a/drivers/gpu/drm/i915/gt/selftest_workarounds.c b/drivers/gpu/drm/i915/gt/selftest_workarounds.c
> > > index abce6e4ec9c0..5c69ec5c5ef9 100644
> > > --- a/drivers/gpu/drm/i915/gt/selftest_workarounds.c
> > > +++ b/drivers/gpu/drm/i915/gt/selftest_workarounds.c
> > > @@ -739,7 +739,7 @@ static int live_dirty_whitelist(void *arg)
> > >       }
> > >  
> > >  out_file:
> > > -     mock_file_free(gt->i915, file);
> > > +     mock_file_put(file);
> > >       return err;
> > >  }
> > >  
> > > diff --git a/drivers/gpu/drm/i915/selftests/i915_gem.c b/drivers/gpu/drm/i915/selftests/i915_gem.c
> > > index d83f6bf6d9d4..aa6282adfd09 100644
> > > --- a/drivers/gpu/drm/i915/selftests/i915_gem.c
> > > +++ b/drivers/gpu/drm/i915/selftests/i915_gem.c
> > > @@ -163,7 +163,7 @@ static int igt_gem_suspend(void *arg)
> > >  
> > >       err = switch_to_context(ctx);
> > >  out:
> > > -     mock_file_free(i915, file);
> > > +     mock_file_put(file);
> > >       return err;
> > >  }
> > >  
> > > @@ -198,7 +198,7 @@ static int igt_gem_hibernate(void *arg)
> > >  
> > >       err = switch_to_context(ctx);
> > >  out:
> > > -     mock_file_free(i915, file);
> > > +     mock_file_put(file);
> > >       return err;
> > >  }
> > >  
> > > diff --git a/drivers/gpu/drm/i915/selftests/i915_gem_evict.c b/drivers/gpu/drm/i915/selftests/i915_gem_evict.c
> > > index 42e948144f1b..41092dcea5b1 100644
> > > --- a/drivers/gpu/drm/i915/selftests/i915_gem_evict.c
> > > +++ b/drivers/gpu/drm/i915/selftests/i915_gem_evict.c
> > > @@ -515,7 +515,7 @@ static int igt_evict_contexts(void *arg)
> > >               pr_info("Submitted %lu contexts/requests on %s\n",
> > >                       count, engine->name);
> > >  
> > > -             mock_file_free(i915, file);
> > > +             mock_file_put(file);
> > >               if (err)
> > >                       break;
> > >       }
> > > diff --git a/drivers/gpu/drm/i915/selftests/i915_gem_gtt.c b/drivers/gpu/drm/i915/selftests/i915_gem_gtt.c
> > > index 3f7e80fb3bbd..c3e0d63c4d0c 100644
> > > --- a/drivers/gpu/drm/i915/selftests/i915_gem_gtt.c
> > > +++ b/drivers/gpu/drm/i915/selftests/i915_gem_gtt.c
> > > @@ -1026,7 +1026,7 @@ static int exercise_ppgtt(struct drm_i915_private *dev_priv,
> > >       i915_vm_put(&ppgtt->vm);
> > >  
> > >  out_free:
> > > -     mock_file_free(dev_priv, file);
> > > +     mock_file_put(file);
> > >       return err;
> > >  }
> > >  
> > > @@ -2022,7 +2022,7 @@ static int igt_cs_tlb(void *arg)
> > >  out_vm:
> > >       i915_vm_put(vm);
> > >  out_unlock:
> > > -     mock_file_free(i915, file);
> > > +     mock_file_put(file);
> > >       return err;
> > >  }
> > >  
> > > diff --git a/drivers/gpu/drm/i915/selftests/i915_request.c b/drivers/gpu/drm/i915/selftests/i915_request.c
> > > index 9e6d3159cd80..7c56ee38cc5b 100644
> > > --- a/drivers/gpu/drm/i915/selftests/i915_request.c
> > > +++ b/drivers/gpu/drm/i915/selftests/i915_request.c
> > > @@ -1430,7 +1430,7 @@ static int live_breadcrumbs_smoketest(void *arg)
> > >  out_smoke:
> > >       kfree(smoke);
> > >  out_file:
> > > -     mock_file_free(i915, file);
> > > +     mock_file_put(file);
> > >  out_rpm:
> > >       intel_runtime_pm_put(&i915->runtime_pm, wakeref);
> > >  
> > > diff --git a/drivers/gpu/drm/i915/selftests/intel_memory_region.c b/drivers/gpu/drm/i915/selftests/intel_memory_region.c
> > > index 19e1cca8f143..fa283ce4532a 100644
> > > --- a/drivers/gpu/drm/i915/selftests/intel_memory_region.c
> > > +++ b/drivers/gpu/drm/i915/selftests/intel_memory_region.c
> > > @@ -439,7 +439,7 @@ static int igt_lmem_write_gpu(void *arg)
> > >  out_put:
> > >       i915_gem_object_put(obj);
> > >  out_file:
> > > -     mock_file_free(i915, file);
> > > +     mock_file_put(file);
> > >       return err;
> > >  }
> > >  
> > > diff --git a/drivers/gpu/drm/i915/selftests/mock_drm.c b/drivers/gpu/drm/i915/selftests/mock_drm.c
> > > index 09c704153456..c100c3efe239 100644
> > > --- a/drivers/gpu/drm/i915/selftests/mock_drm.c
> > > +++ b/drivers/gpu/drm/i915/selftests/mock_drm.c
> > > @@ -22,52 +22,17 @@
> > >   *
> > >   */
> > >  
> > > +#include <drm/drm_file.h>
> > > +
> > >  #include "mock_drm.h"
> > >  
> > >  struct drm_file *mock_file(struct drm_i915_private *i915)
> > >  {
> > > -     struct file *filp;
> > > -     struct inode *inode;
> > > -     struct drm_file *file;
> > > -     int err;
> > > -
> > > -     inode = kzalloc(sizeof(*inode), GFP_KERNEL);
> > > -     if (!inode) {
> > > -             err = -ENOMEM;
> > > -             goto err;
> > > -     }
> > > -
> > > -     inode->i_rdev = i915->drm.primary->index;
> > > -
> > > -     filp = kzalloc(sizeof(*filp), GFP_KERNEL);
> > > -     if (!filp) {
> > > -             err = -ENOMEM;
> > > -             goto err_inode;
> > > -     }
> > > -
> > > -     err = drm_open(inode, filp);
> > > -     if (err)
> > > -             goto err_filp;
> > > +     struct file *file;
> > >  
> > > -     file = filp->private_data;
> > > -     memset(&file->filp, POISON_INUSE, sizeof(file->filp));
> > > -     file->authenticated = true;
> > > -
> > > -     kfree(filp);
> > > -     kfree(inode);
> > > -     return file;
> > > -
> > > -err_filp:
> > > -     kfree(filp);
> > > -err_inode:
> > > -     kfree(inode);
> > > -err:
> > > -     return ERR_PTR(err);
> > > -}
> > > -
> > > -void mock_file_free(struct drm_i915_private *i915, struct drm_file *file)
> > > -{
> > > -     struct file filp = { .private_data = file };
> > > +     file = mock_drm_getfile(i915->drm.primary, O_RDWR);
> > > +     if (IS_ERR(file))
> > > +             return ERR_CAST(file);
> > >  
> > > -     drm_release(NULL, &filp);
> > > +     return file->private_data;
> > 
> > Hm looking at this patch seems like the function interface for the core
> > mock helper is wrong, and we want a drm_file there. Then you could also
> > simplify this wrapper a notch more.
> > 
> > >  }
> > > diff --git a/drivers/gpu/drm/i915/selftests/mock_drm.h b/drivers/gpu/drm/i915/selftests/mock_drm.h
> > > index b39beee9f8f6..dc4190bd3f5a 100644
> > > --- a/drivers/gpu/drm/i915/selftests/mock_drm.h
> > > +++ b/drivers/gpu/drm/i915/selftests/mock_drm.h
> > > @@ -25,7 +25,13 @@
> > >  #ifndef __MOCK_DRM_H
> > >  #define __MOCK_DRM_H
> > >  
> > > +struct drm_file;
> > > +struct drm_i915_private;
> > > +
> > >  struct drm_file *mock_file(struct drm_i915_private *i915);
> > > -void mock_file_free(struct drm_i915_private *i915, struct drm_file *file);
> > > +static inline void mock_file_put(struct drm_file *file)
> > 
> > I still think this would be nice next to the core mock_drm_open helper, as
> > maybe mock_drm_close. Suitable bikeshedding on the names included :-)
> 
> My intention is to transition the tests to tracking struct file on
> allocation and so use fput() directly, with a local struct drm_file as
> appropriate. Nobody else is in the habit of creating anonymous struct
> drm_file, just yet so moving this helper to core seems wasteful.

Ah if that's the plan, then please quote the above Q&A in the commit
message and the patch is

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

Cheers, Daniel

Patch
diff mbox series

diff --git a/drivers/gpu/drm/i915/Kconfig.debug b/drivers/gpu/drm/i915/Kconfig.debug
index ef123eb29168..1140525da75a 100644
--- a/drivers/gpu/drm/i915/Kconfig.debug
+++ b/drivers/gpu/drm/i915/Kconfig.debug
@@ -37,6 +37,7 @@  config DRM_I915_DEBUG
 	select X86_MSR # used by igt/pm_rpm
 	select DRM_VGEM # used by igt/prime_vgem (dmabuf interop checks)
 	select DRM_DEBUG_MM if DRM=y
+	select DRM_EXPORT_FOR_TESTS if m
 	select DRM_DEBUG_SELFTEST
 	select DMABUF_SELFTESTS
 	select SW_SYNC # signaling validation framework (igt/syncobj*)
@@ -160,6 +161,7 @@  config DRM_I915_SELFTEST
 	bool "Enable selftests upon driver load"
 	depends on DRM_I915
 	default n
+	select DRM_EXPORT_FOR_TESTS if m
 	select FAULT_INJECTION
 	select PRIME_NUMBERS
 	help
diff --git a/drivers/gpu/drm/i915/gem/selftests/huge_pages.c b/drivers/gpu/drm/i915/gem/selftests/huge_pages.c
index 688c49a24f32..06dad7b0db82 100644
--- a/drivers/gpu/drm/i915/gem/selftests/huge_pages.c
+++ b/drivers/gpu/drm/i915/gem/selftests/huge_pages.c
@@ -1944,6 +1944,6 @@  int i915_gem_huge_page_live_selftests(struct drm_i915_private *i915)
 	err = i915_subtests(tests, ctx);
 
 out_file:
-	mock_file_free(i915, file);
+	mock_file_put(file);
 	return err;
 }
diff --git a/drivers/gpu/drm/i915/gem/selftests/i915_gem_context.c b/drivers/gpu/drm/i915/gem/selftests/i915_gem_context.c
index 62fabc023a83..47890c92534c 100644
--- a/drivers/gpu/drm/i915/gem/selftests/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/gem/selftests/i915_gem_context.c
@@ -149,7 +149,7 @@  static int live_nop_switch(void *arg)
 	}
 
 out_file:
-	mock_file_free(i915, file);
+	mock_file_put(file);
 	return err;
 }
 
@@ -377,7 +377,7 @@  static int live_parallel_switch(void *arg)
 	}
 	kfree(data);
 out_file:
-	mock_file_free(i915, file);
+	mock_file_put(file);
 	return err;
 }
 
@@ -716,7 +716,7 @@  static int igt_ctx_exec(void *arg)
 		if (igt_live_test_end(&t))
 			err = -EIO;
 
-		mock_file_free(i915, file);
+		mock_file_put(file);
 		if (err)
 			return err;
 
@@ -854,7 +854,7 @@  static int igt_shared_ctx_exec(void *arg)
 	if (igt_live_test_end(&t))
 		err = -EIO;
 out_file:
-	mock_file_free(i915, file);
+	mock_file_put(file);
 	return err;
 }
 
@@ -1426,7 +1426,7 @@  static int igt_ctx_readonly(void *arg)
 	if (igt_live_test_end(&t))
 		err = -EIO;
 
-	mock_file_free(i915, file);
+	mock_file_put(file);
 	return err;
 }
 
@@ -1750,7 +1750,7 @@  static int igt_vm_isolation(void *arg)
 out_file:
 	if (igt_live_test_end(&t))
 		err = -EIO;
-	mock_file_free(i915, file);
+	mock_file_put(file);
 	return err;
 }
 
diff --git a/drivers/gpu/drm/i915/gem/selftests/i915_gem_object_blt.c b/drivers/gpu/drm/i915/gem/selftests/i915_gem_object_blt.c
index e8132aca0bb6..d9fdfddb7091 100644
--- a/drivers/gpu/drm/i915/gem/selftests/i915_gem_object_blt.c
+++ b/drivers/gpu/drm/i915/gem/selftests/i915_gem_object_blt.c
@@ -301,7 +301,7 @@  static int igt_fill_blt_thread(void *arg)
 
 	intel_context_put(ce);
 out_file:
-	mock_file_free(i915, file);
+	mock_file_put(file);
 	return err;
 }
 
@@ -432,7 +432,7 @@  static int igt_copy_blt_thread(void *arg)
 
 	intel_context_put(ce);
 out_file:
-	mock_file_free(i915, file);
+	mock_file_put(file);
 	return err;
 }
 
diff --git a/drivers/gpu/drm/i915/gt/selftest_context.c b/drivers/gpu/drm/i915/gt/selftest_context.c
index bc720defc6b8..a5688f7d9073 100644
--- a/drivers/gpu/drm/i915/gt/selftest_context.c
+++ b/drivers/gpu/drm/i915/gt/selftest_context.c
@@ -313,7 +313,7 @@  static int live_active_context(void *arg)
 	}
 
 out_file:
-	mock_file_free(gt->i915, file);
+	mock_file_put(file);
 	return err;
 }
 
@@ -423,7 +423,7 @@  static int live_remote_context(void *arg)
 	}
 
 out_file:
-	mock_file_free(gt->i915, file);
+	mock_file_put(file);
 	return err;
 }
 
diff --git a/drivers/gpu/drm/i915/gt/selftest_hangcheck.c b/drivers/gpu/drm/i915/gt/selftest_hangcheck.c
index 85e9ccf5c304..cdaaee4432b2 100644
--- a/drivers/gpu/drm/i915/gt/selftest_hangcheck.c
+++ b/drivers/gpu/drm/i915/gt/selftest_hangcheck.c
@@ -439,7 +439,7 @@  static int igt_reset_nop(void *arg)
 
 	err = igt_flush_test(gt->i915);
 out:
-	mock_file_free(gt->i915, file);
+	mock_file_put(file);
 	if (intel_gt_is_wedged(gt))
 		err = -EIO;
 	return err;
@@ -535,7 +535,7 @@  static int igt_reset_nop_engine(void *arg)
 
 	err = igt_flush_test(gt->i915);
 out:
-	mock_file_free(gt->i915, file);
+	mock_file_put(file);
 	if (intel_gt_is_wedged(gt))
 		err = -EIO;
 	return err;
@@ -752,7 +752,7 @@  static int active_engine(void *data)
 	}
 
 err_file:
-	mock_file_free(engine->i915, file);
+	mock_file_put(file);
 	return err;
 }
 
@@ -1325,7 +1325,7 @@  static int igt_reset_evict_ppgtt(void *arg)
 	i915_vm_put(vm);
 
 out:
-	mock_file_free(gt->i915, file);
+	mock_file_put(file);
 	return err;
 }
 
diff --git a/drivers/gpu/drm/i915/gt/selftest_workarounds.c b/drivers/gpu/drm/i915/gt/selftest_workarounds.c
index abce6e4ec9c0..5c69ec5c5ef9 100644
--- a/drivers/gpu/drm/i915/gt/selftest_workarounds.c
+++ b/drivers/gpu/drm/i915/gt/selftest_workarounds.c
@@ -739,7 +739,7 @@  static int live_dirty_whitelist(void *arg)
 	}
 
 out_file:
-	mock_file_free(gt->i915, file);
+	mock_file_put(file);
 	return err;
 }
 
diff --git a/drivers/gpu/drm/i915/selftests/i915_gem.c b/drivers/gpu/drm/i915/selftests/i915_gem.c
index d83f6bf6d9d4..aa6282adfd09 100644
--- a/drivers/gpu/drm/i915/selftests/i915_gem.c
+++ b/drivers/gpu/drm/i915/selftests/i915_gem.c
@@ -163,7 +163,7 @@  static int igt_gem_suspend(void *arg)
 
 	err = switch_to_context(ctx);
 out:
-	mock_file_free(i915, file);
+	mock_file_put(file);
 	return err;
 }
 
@@ -198,7 +198,7 @@  static int igt_gem_hibernate(void *arg)
 
 	err = switch_to_context(ctx);
 out:
-	mock_file_free(i915, file);
+	mock_file_put(file);
 	return err;
 }
 
diff --git a/drivers/gpu/drm/i915/selftests/i915_gem_evict.c b/drivers/gpu/drm/i915/selftests/i915_gem_evict.c
index 42e948144f1b..41092dcea5b1 100644
--- a/drivers/gpu/drm/i915/selftests/i915_gem_evict.c
+++ b/drivers/gpu/drm/i915/selftests/i915_gem_evict.c
@@ -515,7 +515,7 @@  static int igt_evict_contexts(void *arg)
 		pr_info("Submitted %lu contexts/requests on %s\n",
 			count, engine->name);
 
-		mock_file_free(i915, file);
+		mock_file_put(file);
 		if (err)
 			break;
 	}
diff --git a/drivers/gpu/drm/i915/selftests/i915_gem_gtt.c b/drivers/gpu/drm/i915/selftests/i915_gem_gtt.c
index 3f7e80fb3bbd..c3e0d63c4d0c 100644
--- a/drivers/gpu/drm/i915/selftests/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/selftests/i915_gem_gtt.c
@@ -1026,7 +1026,7 @@  static int exercise_ppgtt(struct drm_i915_private *dev_priv,
 	i915_vm_put(&ppgtt->vm);
 
 out_free:
-	mock_file_free(dev_priv, file);
+	mock_file_put(file);
 	return err;
 }
 
@@ -2022,7 +2022,7 @@  static int igt_cs_tlb(void *arg)
 out_vm:
 	i915_vm_put(vm);
 out_unlock:
-	mock_file_free(i915, file);
+	mock_file_put(file);
 	return err;
 }
 
diff --git a/drivers/gpu/drm/i915/selftests/i915_request.c b/drivers/gpu/drm/i915/selftests/i915_request.c
index 9e6d3159cd80..7c56ee38cc5b 100644
--- a/drivers/gpu/drm/i915/selftests/i915_request.c
+++ b/drivers/gpu/drm/i915/selftests/i915_request.c
@@ -1430,7 +1430,7 @@  static int live_breadcrumbs_smoketest(void *arg)
 out_smoke:
 	kfree(smoke);
 out_file:
-	mock_file_free(i915, file);
+	mock_file_put(file);
 out_rpm:
 	intel_runtime_pm_put(&i915->runtime_pm, wakeref);
 
diff --git a/drivers/gpu/drm/i915/selftests/intel_memory_region.c b/drivers/gpu/drm/i915/selftests/intel_memory_region.c
index 19e1cca8f143..fa283ce4532a 100644
--- a/drivers/gpu/drm/i915/selftests/intel_memory_region.c
+++ b/drivers/gpu/drm/i915/selftests/intel_memory_region.c
@@ -439,7 +439,7 @@  static int igt_lmem_write_gpu(void *arg)
 out_put:
 	i915_gem_object_put(obj);
 out_file:
-	mock_file_free(i915, file);
+	mock_file_put(file);
 	return err;
 }
 
diff --git a/drivers/gpu/drm/i915/selftests/mock_drm.c b/drivers/gpu/drm/i915/selftests/mock_drm.c
index 09c704153456..c100c3efe239 100644
--- a/drivers/gpu/drm/i915/selftests/mock_drm.c
+++ b/drivers/gpu/drm/i915/selftests/mock_drm.c
@@ -22,52 +22,17 @@ 
  *
  */
 
+#include <drm/drm_file.h>
+
 #include "mock_drm.h"
 
 struct drm_file *mock_file(struct drm_i915_private *i915)
 {
-	struct file *filp;
-	struct inode *inode;
-	struct drm_file *file;
-	int err;
-
-	inode = kzalloc(sizeof(*inode), GFP_KERNEL);
-	if (!inode) {
-		err = -ENOMEM;
-		goto err;
-	}
-
-	inode->i_rdev = i915->drm.primary->index;
-
-	filp = kzalloc(sizeof(*filp), GFP_KERNEL);
-	if (!filp) {
-		err = -ENOMEM;
-		goto err_inode;
-	}
-
-	err = drm_open(inode, filp);
-	if (err)
-		goto err_filp;
+	struct file *file;
 
-	file = filp->private_data;
-	memset(&file->filp, POISON_INUSE, sizeof(file->filp));
-	file->authenticated = true;
-
-	kfree(filp);
-	kfree(inode);
-	return file;
-
-err_filp:
-	kfree(filp);
-err_inode:
-	kfree(inode);
-err:
-	return ERR_PTR(err);
-}
-
-void mock_file_free(struct drm_i915_private *i915, struct drm_file *file)
-{
-	struct file filp = { .private_data = file };
+	file = mock_drm_getfile(i915->drm.primary, O_RDWR);
+	if (IS_ERR(file))
+		return ERR_CAST(file);
 
-	drm_release(NULL, &filp);
+	return file->private_data;
 }
diff --git a/drivers/gpu/drm/i915/selftests/mock_drm.h b/drivers/gpu/drm/i915/selftests/mock_drm.h
index b39beee9f8f6..dc4190bd3f5a 100644
--- a/drivers/gpu/drm/i915/selftests/mock_drm.h
+++ b/drivers/gpu/drm/i915/selftests/mock_drm.h
@@ -25,7 +25,13 @@ 
 #ifndef __MOCK_DRM_H
 #define __MOCK_DRM_H
 
+struct drm_file;
+struct drm_i915_private;
+
 struct drm_file *mock_file(struct drm_i915_private *i915);
-void mock_file_free(struct drm_i915_private *i915, struct drm_file *file);
+static inline void mock_file_put(struct drm_file *file)
+{
+	fput(file->filp);
+}
 
 #endif /* !__MOCK_DRM_H */