| Message ID | 20210924163825.634606-1-janusz.krzysztofik@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2] drm/i915: Flush buffer pools on driver remove | expand |
On Fri, Sep 24, 2021 at 06:38:25PM +0200, Janusz Krzysztofik wrote: > We currently do an explicit flush of the buffer pools within the call path > of drm_driver.release(); this removes all buffers, regardless of their age, > freeing the buffers' associated resources (objects, adress space areas). > However there is other code that runs within the drm_driver.release() call > chain that expects objects and their associated address space areas have > already been flushed. > > Since buffer pools auto-flush old buffers once per second in a worker > thread, there's a small window where if we remove the driver while there > are still objects in buffers with an age of less than one second, the > assumptions of the other release code may be violated. > > By moving the flush to driver remove (which executes earlier via the > pci_driver.remove() flow) we're ensuring that all buffers are flushed and > their associated objects freed before some other code in > pci_driver.remove() flushes those objects so they are released before > _any_ code in drm_driver.release() that check completness of those > flushes executes. > > v2: Reword commit descriptiom as suggested by Matt. > > Signed-off-by: Janusz Krzysztofik <janusz.krzysztofik@linux.intel.com> > Cc: Chris Wilson <chris@chris-wilson.co.uk> > Cc: Matt Roper <matthew.d.roper@intel.com> Reviewed-by: Matt Roper <matthew.d.roper@intel.com> and applied to drm-intel-gt-next (with a couple minor spelling fixes in the commit message). Thanks for the patch. Matt > --- > drivers/gpu/drm/i915/gt/intel_gt.c | 2 ++ > drivers/gpu/drm/i915/gt/intel_gt_buffer_pool.c | 2 -- > 2 files changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/i915/gt/intel_gt.c b/drivers/gpu/drm/i915/gt/intel_gt.c > index 4037c3778225..5b3acf2b064e 100644 > --- a/drivers/gpu/drm/i915/gt/intel_gt.c > +++ b/drivers/gpu/drm/i915/gt/intel_gt.c > @@ -741,6 +741,8 @@ void intel_gt_driver_remove(struct intel_gt *gt) > intel_uc_driver_remove(>->uc); > > intel_engines_release(gt); > + > + intel_gt_flush_buffer_pool(gt); > } > > void intel_gt_driver_unregister(struct intel_gt *gt) > diff --git a/drivers/gpu/drm/i915/gt/intel_gt_buffer_pool.c b/drivers/gpu/drm/i915/gt/intel_gt_buffer_pool.c > index aa0a59c5b614..acc49c56a9f3 100644 > --- a/drivers/gpu/drm/i915/gt/intel_gt_buffer_pool.c > +++ b/drivers/gpu/drm/i915/gt/intel_gt_buffer_pool.c > @@ -245,8 +245,6 @@ void intel_gt_fini_buffer_pool(struct intel_gt *gt) > struct intel_gt_buffer_pool *pool = >->buffer_pool; > int n; > > - intel_gt_flush_buffer_pool(gt); > - > for (n = 0; n < ARRAY_SIZE(pool->cache_list); n++) > GEM_BUG_ON(!list_empty(&pool->cache_list[n])); > } > -- > 2.25.1 >
diff --git a/drivers/gpu/drm/i915/gt/intel_gt.c b/drivers/gpu/drm/i915/gt/intel_gt.c index 4037c3778225..5b3acf2b064e 100644 --- a/drivers/gpu/drm/i915/gt/intel_gt.c +++ b/drivers/gpu/drm/i915/gt/intel_gt.c @@ -741,6 +741,8 @@ void intel_gt_driver_remove(struct intel_gt *gt) intel_uc_driver_remove(>->uc); intel_engines_release(gt); + + intel_gt_flush_buffer_pool(gt); } void intel_gt_driver_unregister(struct intel_gt *gt) diff --git a/drivers/gpu/drm/i915/gt/intel_gt_buffer_pool.c b/drivers/gpu/drm/i915/gt/intel_gt_buffer_pool.c index aa0a59c5b614..acc49c56a9f3 100644 --- a/drivers/gpu/drm/i915/gt/intel_gt_buffer_pool.c +++ b/drivers/gpu/drm/i915/gt/intel_gt_buffer_pool.c @@ -245,8 +245,6 @@ void intel_gt_fini_buffer_pool(struct intel_gt *gt) struct intel_gt_buffer_pool *pool = >->buffer_pool; int n; - intel_gt_flush_buffer_pool(gt); - for (n = 0; n < ARRAY_SIZE(pool->cache_list); n++) GEM_BUG_ON(!list_empty(&pool->cache_list[n])); }
We currently do an explicit flush of the buffer pools within the call path of drm_driver.release(); this removes all buffers, regardless of their age, freeing the buffers' associated resources (objects, adress space areas). However there is other code that runs within the drm_driver.release() call chain that expects objects and their associated address space areas have already been flushed. Since buffer pools auto-flush old buffers once per second in a worker thread, there's a small window where if we remove the driver while there are still objects in buffers with an age of less than one second, the assumptions of the other release code may be violated. By moving the flush to driver remove (which executes earlier via the pci_driver.remove() flow) we're ensuring that all buffers are flushed and their associated objects freed before some other code in pci_driver.remove() flushes those objects so they are released before _any_ code in drm_driver.release() that check completness of those flushes executes. v2: Reword commit descriptiom as suggested by Matt. Signed-off-by: Janusz Krzysztofik <janusz.krzysztofik@linux.intel.com> Cc: Chris Wilson <chris@chris-wilson.co.uk> Cc: Matt Roper <matthew.d.roper@intel.com> --- drivers/gpu/drm/i915/gt/intel_gt.c | 2 ++ drivers/gpu/drm/i915/gt/intel_gt_buffer_pool.c | 2 -- 2 files changed, 2 insertions(+), 2 deletions(-)