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@xxxxxxxxxxxxxxxxxx>
> > > ---
> > > 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@xxxxxxxx>
Cheers, Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
