Hi Krzysztof, Sorry, I haven't replied to you in v1, but my concerns still remain and they are not adressed here.
> Closes: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/14808 > Signed-off-by: Krzysztof Niemiec <[email protected]> > --- > drivers/gpu/drm/i915/selftests/i915_active.c | 15 ++++++++++++--- > 1 file changed, 12 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/i915/selftests/i915_active.c > b/drivers/gpu/drm/i915/selftests/i915_active.c > index 0d89d70b9c36..a82a56c3eeb6 100644 > --- a/drivers/gpu/drm/i915/selftests/i915_active.c > +++ b/drivers/gpu/drm/i915/selftests/i915_active.c > @@ -74,15 +74,25 @@ static struct live_active *__live_alloc(struct > drm_i915_private *i915) > return active; > } > > +static struct i915_sw_fence *submit; > +static struct delayed_work __live_submit_work; > + > +static void __live_submit_work_handler(struct work_struct *work) > +{ > + i915_sw_fence_commit(submit); > + heap_fence_put(submit); > +} > + > static struct live_active * > __live_active_setup(struct drm_i915_private *i915) > { > struct intel_engine_cs *engine; > - struct i915_sw_fence *submit; > struct live_active *active; > unsigned int count = 0; > int err = 0; > > + INIT_DELAYED_WORK(&__live_submit_work, __live_submit_work_handler); > + > active = __live_alloc(i915); > if (!active) > return ERR_PTR(-ENOMEM); > @@ -132,8 +142,7 @@ __live_active_setup(struct drm_i915_private *i915) > } > > out: > - i915_sw_fence_commit(submit); > - heap_fence_put(submit); > + schedule_delayed_work(&__live_submit_work, msecs_to_jiffies(500)); Where does the 500 ms delay come from? How long will this work run? Could it overlap with the next test execution? If so, what happens to the global variables? Please ensure everything is cleaned up before exiting the test. Embed the global state in a per test structure that is torn down at the end of the test. I do not want to leave any stray work or state behind. Even if it looks fine, this is a house of cards and easy to break. Sorry, this patch is not ready to go. Andi > if (err) { > __live_put(active); > active = ERR_PTR(err); > -- > 2.45.2 >
