Re: [Intel-gfx] [PATCH v2] drm/i915: avoid flush_scheduled_work() usage

[Daniel Vetter <daniel@ffwll.ch>]

On Fri, Apr 14, 2023 at 07:52:12PM +0900, Tetsuo Handa wrote:
> On 2023/04/14 19:13, Jani Nikula wrote:
> > On Fri, 14 Apr 2023, Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp> wrote:
> >> On 2023/03/15 19:47, Luca Coelho wrote:
> >>> On Tue, 2023-03-14 at 20:21 +0900, Tetsuo Handa wrote:
> >>>> Like commit c4f135d643823a86 ("workqueue: Wrap flush_workqueue() using a
> >>>> macro") says, flush_scheduled_work() is dangerous and will be forbidden.
> >>>>
> >>>> Now that i915 is the last flush_scheduled_work() user, for now let's
> >>>> start with blind conversion inside the whole drivers/gpu/drm/i915/
> >>>> directory. Jani Nikula wants to use two workqueues in order to avoid
> >>>> adding new module globals, but I'm not familiar enough to audit and
> >>>> split into two workqueues.
> >>>>
> >>>> Link: https://lkml.kernel.org/r/87sfeita1p.fsf@intel.com
> >>>> Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
> >>>> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> >>>> Cc: Jani Nikula <jani.nikula@intel.com>
> >>>> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >>>> ---
> >>>> Changes in v2:
> >>>>   Add missing alloc_workqueue() failure check.
> >>>
> >>> Hi,
> >>>
> >>> Thanks for your patch! But it seems that you only fixed that failure
> >>> check, without making the other change Jani proposed, namely, move the
> >>> work to the i915 struct instead of making it a global.
> >>>
> >>> I'm working on that now.
> >>
> >> What is estimated time of arrival on this?
> >> Can we expect your work in Linux 6.4 ?
> > 
> > I'm afraid that ship has sailed. Sorry. :(
> 
> Well, then, can we temporarily apply "[PATCH v2] drm/i915: avoid flush_scheduled_work() usage" ?
> This patch is a mechanical conversion which unlikely causes regressions. This patch eliminates
> interference from work items outside of i915, which is small but an improvement for i915 users.

I think if someone from i915 team triple-checks that i915 really doesn't
use any of the drm workers (hotplug handling, atomic commit, ...) then I
think we should be fine. The one that's unavoidable is the rmfb work
(which really only exists to avoid signal interruptions when doing this in
userspace process context, it's entirely synchronous otherwise), but I
think that's safe.

With that tripled checked I think the mechanical conversion is ok to land
late for 6.4 and has my

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

[Dropped this on irc already, here just for the record]

Cheers, Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch