| Message ID | 20210813203033.3179400-10-daniel.vetter@ffwll.ch (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [01/11] drm/i915: Release i915_gem_context from a worker | expand |
Op 13-08-2021 om 22:30 schreef Daniel Vetter: > We don't need the absolute speed of rcu for this. And > i915_address_space in general dont need rcu protection anywhere else, > after we've made gem contexts and engines a lot more immutable. > > Note that this semantically reverts > > commit aabbe344dc3ca5f7d8263a02608ba6179e8a4499 > Author: Chris Wilson <chris@chris-wilson.co.uk> > Date: Fri Aug 30 19:03:25 2019 +0100 > > drm/i915: Use RCU for unlocked vm_idr lookup > > except we have the conversion from idr to xarray in between. > > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> > Cc: Jon Bloomfield <jon.bloomfield@intel.com> > Cc: Chris Wilson <chris@chris-wilson.co.uk> > Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> > Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> > Cc: Daniel Vetter <daniel.vetter@ffwll.ch> > Cc: "Thomas Hellström" <thomas.hellstrom@linux.intel.com> > Cc: Matthew Auld <matthew.auld@intel.com> > Cc: Lionel Landwerlin <lionel.g.landwerlin@intel.com> > Cc: Dave Airlie <airlied@redhat.com> > Cc: Jason Ekstrand <jason@jlekstrand.net> > --- > drivers/gpu/drm/i915/i915_drv.h | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > index 005b1cec7007..e37fac8fac0c 100644 > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -1881,11 +1881,11 @@ i915_gem_vm_lookup(struct drm_i915_file_private *file_priv, u32 id) > { > struct i915_address_space *vm; > > - rcu_read_lock(); > + xa_lock(&file_priv->vm_xa); > vm = xa_load(&file_priv->vm_xa, id); > if (vm && !kref_get_unless_zero(&vm->ref)) > vm = NULL; I think this could be a plain i915_vm_get now, kref_get_unless_zero is not guarded by RCU any more. > - rcu_read_unlock(); > + xa_unlock(&file_priv->vm_xa); > > return vm; > } Apart from that, all looks good. With this fix, for patch 2-11: Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 005b1cec7007..e37fac8fac0c 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -1881,11 +1881,11 @@ i915_gem_vm_lookup(struct drm_i915_file_private *file_priv, u32 id) { struct i915_address_space *vm; - rcu_read_lock(); + xa_lock(&file_priv->vm_xa); vm = xa_load(&file_priv->vm_xa, id); if (vm && !kref_get_unless_zero(&vm->ref)) vm = NULL; - rcu_read_unlock(); + xa_unlock(&file_priv->vm_xa); return vm; }
We don't need the absolute speed of rcu for this. And i915_address_space in general dont need rcu protection anywhere else, after we've made gem contexts and engines a lot more immutable. Note that this semantically reverts commit aabbe344dc3ca5f7d8263a02608ba6179e8a4499 Author: Chris Wilson <chris@chris-wilson.co.uk> Date: Fri Aug 30 19:03:25 2019 +0100 drm/i915: Use RCU for unlocked vm_idr lookup except we have the conversion from idr to xarray in between. Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> Cc: Jon Bloomfield <jon.bloomfield@intel.com> Cc: Chris Wilson <chris@chris-wilson.co.uk> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> Cc: Daniel Vetter <daniel.vetter@ffwll.ch> Cc: "Thomas Hellström" <thomas.hellstrom@linux.intel.com> Cc: Matthew Auld <matthew.auld@intel.com> Cc: Lionel Landwerlin <lionel.g.landwerlin@intel.com> Cc: Dave Airlie <airlied@redhat.com> Cc: Jason Ekstrand <jason@jlekstrand.net> --- drivers/gpu/drm/i915/i915_drv.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)