Re: [Intel-gfx] [PATCH v4] drm/i915: avoid flush_scheduled_work() usage
On 18/05/2023 15:44, Tetsuo Handa wrote:
Like commit c4f135d643823a86 ("workqueue: Wrap flush_workqueue() using a
macro") says, flush_scheduled_work() is dangerous and will be forbidden.
i915 became the last flush_scheduled_work() user, but developers cannot
find time for auditing which work items does this flush_scheduled_work()
need to wait.
Therefore, for now let's start with blind/mechanical conversion within
the whole drivers/gpu/drm/i915/ directory, based on an assumption that
i915 does not need to wait for work items outside of this directory.
Link: https://lkml.kernel.org/r/87sfeita1p@intel.com
Signed-off-by: Tetsuo Handa
Cc: Tvrtko Ursulin
Cc: Jani Nikula
Cc: Ville Syrjälä
---
Changes in v4:
Refreshed using drm-tip.git.
Changes in v3:
Refreshed using drm-tip.git, for commit 40053823baad ("drm/i915/display:
move modeset probe/remove functions to intel_display_driver.c") moved
flush_scheduled_work() from intel_display.c to intel_display_driver.c .
Please check the comment from Daniel Vetter at
https://lkml.kernel.org/r/ZDuntOkUeh0Eve8a@phenom.ffwll.local .
I can't help with that display code assesment but in general the
approach with a driver global wq works for me.
Acked-by: Tvrtko Ursulin
I haven't read the patch in detail, just one thing caught my eye which
is that we already have a workqueue named "i915" (see
i915_workqueues_init). So I'd suggest naming the one from this patch
"i915-global" or something. Just so it is clearer if/when debugging aids
start firing what is what.
Regards,
Tvrtko
Changes in v2:
Add missing alloc_workqueue() failure check.
drivers/gpu/drm/i915/display/intel_display.c | 2 +-
.../drm/i915/display/intel_display_driver.c| 2 +-
drivers/gpu/drm/i915/display/intel_dmc.c | 2 +-
drivers/gpu/drm/i915/display/intel_dp.c| 2 +-
.../drm/i915/display/intel_dp_link_training.c | 2 +-
drivers/gpu/drm/i915/display/intel_drrs.c | 2 +-
drivers/gpu/drm/i915/display/intel_fbc.c | 2 +-
drivers/gpu/drm/i915/display/intel_fbdev.c | 2 +-
drivers/gpu/drm/i915/display/intel_hdcp.c | 18 +-
drivers/gpu/drm/i915/display/intel_hotplug.c | 12 ++--
drivers/gpu/drm/i915/display/intel_opregion.c | 2 +-
drivers/gpu/drm/i915/display/intel_pps.c | 2 +-
drivers/gpu/drm/i915/display/intel_psr.c | 6 +++---
.../drm/i915/gt/intel_execlists_submission.c | 4 ++--
drivers/gpu/drm/i915/gt/intel_gt_buffer_pool.c | 8
drivers/gpu/drm/i915/gt/intel_gt_irq.c | 2 +-
drivers/gpu/drm/i915/gt/intel_gt_requests.c| 10 +-
drivers/gpu/drm/i915/gt/intel_reset.c | 2 +-
drivers/gpu/drm/i915/gt/intel_rps.c| 14 +++---
drivers/gpu/drm/i915/gt/selftest_engine_cs.c | 2 +-
drivers/gpu/drm/i915/i915_drv.h| 1 +
drivers/gpu/drm/i915/i915_module.c | 7 +++
drivers/gpu/drm/i915/i915_request.c| 2 +-
drivers/gpu/drm/i915/intel_wakeref.c | 4 +++-
drivers/gpu/drm/i915/selftests/i915_sw_fence.c | 4 +++-
25 files changed, 64 insertions(+), 52 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_display.c
b/drivers/gpu/drm/i915/display/intel_display.c
index 09320e14d75c..5f1ba9c908cb 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -7145,7 +7145,7 @@ intel_atomic_commit_ready(struct i915_sw_fence *fence,
_i915(state->base.dev)->display.atomic_helper;
if (llist_add(>freed, >free_list))
- schedule_work(>free_work);
+ queue_work(i915_wq, >free_work);
break;
}
}
diff --git a/drivers/gpu/drm/i915/display/intel_display_driver.c
b/drivers/gpu/drm/i915/display/intel_display_driver.c
index 60ce10fc7205..a20a9cfaab0e 100644
--- a/drivers/gpu/drm/i915/display/intel_display_driver.c
+++ b/drivers/gpu/drm/i915/display/intel_display_driver.c
@@ -435,7 +435,7 @@ void intel_display_driver_remove_noirq(struct
drm_i915_private *i915)
intel_unregister_dsm_handler();
/* flush any delayed tasks or pending work */
- flush_scheduled_work();
+ flush_workqueue(i915_wq);
intel_hdcp_component_fini(i915);
diff --git a/drivers/gpu/drm/i915/display/intel_dmc.c b/drivers/gpu/drm/i915/display/intel_dmc.c
index 8a88de67ff0a..57d015006784 100644
--- a/drivers/gpu/drm/i915/display/intel_dmc.c
+++ b/drivers/gpu/drm/i915/display/intel_dmc.c
@@ -1057,7 +1057,7 @@ void intel_dmc_init(struct drm_i915_private *i915)
i915->display.dmc.dmc = dmc;
drm_dbg_kms(>drm, "Loading %s\n", dmc->fw_path);
- schedule_work(>work);
+ queue_work(i915_wq, >work);
return;
diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
index 4bec8cd7979f..4782bdfc7c61
[Intel-gfx] [PATCH v4] drm/i915: avoid flush_scheduled_work() usage
Like commit c4f135d643823a86 ("workqueue: Wrap flush_workqueue() using a
macro") says, flush_scheduled_work() is dangerous and will be forbidden.
i915 became the last flush_scheduled_work() user, but developers cannot
find time for auditing which work items does this flush_scheduled_work()
need to wait.
Therefore, for now let's start with blind/mechanical conversion within
the whole drivers/gpu/drm/i915/ directory, based on an assumption that
i915 does not need to wait for work items outside of this directory.
Link: https://lkml.kernel.org/r/87sfeita1p@intel.com
Signed-off-by: Tetsuo Handa
Cc: Tvrtko Ursulin
Cc: Jani Nikula
Cc: Ville Syrjälä
---
Changes in v4:
Refreshed using drm-tip.git.
Changes in v3:
Refreshed using drm-tip.git, for commit 40053823baad ("drm/i915/display:
move modeset probe/remove functions to intel_display_driver.c") moved
flush_scheduled_work() from intel_display.c to intel_display_driver.c .
Please check the comment from Daniel Vetter at
https://lkml.kernel.org/r/ZDuntOkUeh0Eve8a@phenom.ffwll.local .
Changes in v2:
Add missing alloc_workqueue() failure check.
drivers/gpu/drm/i915/display/intel_display.c | 2 +-
.../drm/i915/display/intel_display_driver.c| 2 +-
drivers/gpu/drm/i915/display/intel_dmc.c | 2 +-
drivers/gpu/drm/i915/display/intel_dp.c| 2 +-
.../drm/i915/display/intel_dp_link_training.c | 2 +-
drivers/gpu/drm/i915/display/intel_drrs.c | 2 +-
drivers/gpu/drm/i915/display/intel_fbc.c | 2 +-
drivers/gpu/drm/i915/display/intel_fbdev.c | 2 +-
drivers/gpu/drm/i915/display/intel_hdcp.c | 18 +-
drivers/gpu/drm/i915/display/intel_hotplug.c | 12 ++--
drivers/gpu/drm/i915/display/intel_opregion.c | 2 +-
drivers/gpu/drm/i915/display/intel_pps.c | 2 +-
drivers/gpu/drm/i915/display/intel_psr.c | 6 +++---
.../drm/i915/gt/intel_execlists_submission.c | 4 ++--
drivers/gpu/drm/i915/gt/intel_gt_buffer_pool.c | 8
drivers/gpu/drm/i915/gt/intel_gt_irq.c | 2 +-
drivers/gpu/drm/i915/gt/intel_gt_requests.c| 10 +-
drivers/gpu/drm/i915/gt/intel_reset.c | 2 +-
drivers/gpu/drm/i915/gt/intel_rps.c| 14 +++---
drivers/gpu/drm/i915/gt/selftest_engine_cs.c | 2 +-
drivers/gpu/drm/i915/i915_drv.h| 1 +
drivers/gpu/drm/i915/i915_module.c | 7 +++
drivers/gpu/drm/i915/i915_request.c| 2 +-
drivers/gpu/drm/i915/intel_wakeref.c | 4 +++-
drivers/gpu/drm/i915/selftests/i915_sw_fence.c | 4 +++-
25 files changed, 64 insertions(+), 52 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_display.c
b/drivers/gpu/drm/i915/display/intel_display.c
index 09320e14d75c..5f1ba9c908cb 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -7145,7 +7145,7 @@ intel_atomic_commit_ready(struct i915_sw_fence *fence,
_i915(state->base.dev)->display.atomic_helper;
if (llist_add(>freed, >free_list))
- schedule_work(>free_work);
+ queue_work(i915_wq, >free_work);
break;
}
}
diff --git a/drivers/gpu/drm/i915/display/intel_display_driver.c
b/drivers/gpu/drm/i915/display/intel_display_driver.c
index 60ce10fc7205..a20a9cfaab0e 100644
--- a/drivers/gpu/drm/i915/display/intel_display_driver.c
+++ b/drivers/gpu/drm/i915/display/intel_display_driver.c
@@ -435,7 +435,7 @@ void intel_display_driver_remove_noirq(struct
drm_i915_private *i915)
intel_unregister_dsm_handler();
/* flush any delayed tasks or pending work */
- flush_scheduled_work();
+ flush_workqueue(i915_wq);
intel_hdcp_component_fini(i915);
diff --git a/drivers/gpu/drm/i915/display/intel_dmc.c
b/drivers/gpu/drm/i915/display/intel_dmc.c
index 8a88de67ff0a..57d015006784 100644
--- a/drivers/gpu/drm/i915/display/intel_dmc.c
+++ b/drivers/gpu/drm/i915/display/intel_dmc.c
@@ -1057,7 +1057,7 @@ void intel_dmc_init(struct drm_i915_private *i915)
i915->display.dmc.dmc = dmc;
drm_dbg_kms(>drm, "Loading %s\n", dmc->fw_path);
- schedule_work(>work);
+ queue_work(i915_wq, >work);
return;
diff --git a/drivers/gpu/drm/i915/display/intel_dp.c
b/drivers/gpu/drm/i915/display/intel_dp.c
index 4bec8cd7979f..4782bdfc7c61 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -5251,7 +5251,7 @@ static void intel_dp_oob_hotplug_event(struct
drm_connector *connector)
spin_lock_irq(>irq_lock);
i915->display.hotplug.event_bits |= BIT(encoder->hpd_pin);
spin_unlock_irq(>irq_lock);
- queue_delayed_work(system_wq, >display.hotplug.hotplug_work, 0);
+ queue_delayed_work(i915_wq, >display.hotplug.hotplug_work, 0);
}
static const struct
