| Message ID | 20220526235040.678984-2-dmitry.osipenko@collabora.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Add generic memory shrinker to VirtIO-GPU and Panfrost DRM drivers | expand |
Hi, On 5/27/22 01:50, Dmitry Osipenko wrote: > Use ww_acquire_fini() in the error code paths. Otherwise lockdep > thinks that lock is held when lock's memory is freed after the > drm_gem_lock_reservations() error. The WW needs to be annotated > as "freed" s /WW/ww_acquire_context/ ? s /"freed"/"released"/ ? > , which fixes the noisy "WARNING: held lock freed!" splat > of VirtIO-GPU driver with CONFIG_DEBUG_MUTEXES=y and enabled lockdep. > > Cc: stable@vger.kernel.org Can you dig up the commit in error and add a Fixes: Tag? Using that and "dim fixes" will also make the Cc: stable tag a bit more verbose. With that fixed, Reviewed-by: Thomas Hellström <thomas.hellstrom@linux.intel.com> > Signed-off-by: Dmitry Osipenko <dmitry.osipenko@collabora.com> > --- > drivers/gpu/drm/drm_gem.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c > index eb0c2d041f13..86d670c71286 100644 > --- a/drivers/gpu/drm/drm_gem.c > +++ b/drivers/gpu/drm/drm_gem.c > @@ -1226,7 +1226,7 @@ drm_gem_lock_reservations(struct drm_gem_object **objs, int count, > ret = dma_resv_lock_slow_interruptible(obj->resv, > acquire_ctx); > if (ret) { > - ww_acquire_done(acquire_ctx); > + ww_acquire_fini(acquire_ctx); > return ret; > } > } > @@ -1251,7 +1251,7 @@ drm_gem_lock_reservations(struct drm_gem_object **objs, int count, > goto retry; > } > > - ww_acquire_done(acquire_ctx); > + ww_acquire_fini(acquire_ctx); > return ret; > } > }
On 6/28/22 23:12, Thomas Hellström (Intel) wrote: > Hi, > > On 5/27/22 01:50, Dmitry Osipenko wrote: >> Use ww_acquire_fini() in the error code paths. Otherwise lockdep >> thinks that lock is held when lock's memory is freed after the >> drm_gem_lock_reservations() error. The WW needs to be annotated >> as "freed" > > s /WW/ww_acquire_context/ ? > s /"freed"/"released"/ ? > > >> , which fixes the noisy "WARNING: held lock freed!" splat >> of VirtIO-GPU driver with CONFIG_DEBUG_MUTEXES=y and enabled lockdep. >> >> Cc: stable@vger.kernel.org > > Can you dig up the commit in error and add a Fixes: Tag? > > Using that and "dim fixes" will also make the Cc: stable tag a bit more > verbose. > > With that fixed, > > Reviewed-by: Thomas Hellström <thomas.hellstrom@linux.intel.com> I'll update this patch, thank you for taking a look.
diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c index eb0c2d041f13..86d670c71286 100644 --- a/drivers/gpu/drm/drm_gem.c +++ b/drivers/gpu/drm/drm_gem.c @@ -1226,7 +1226,7 @@ drm_gem_lock_reservations(struct drm_gem_object **objs, int count, ret = dma_resv_lock_slow_interruptible(obj->resv, acquire_ctx); if (ret) { - ww_acquire_done(acquire_ctx); + ww_acquire_fini(acquire_ctx); return ret; } } @@ -1251,7 +1251,7 @@ drm_gem_lock_reservations(struct drm_gem_object **objs, int count, goto retry; } - ww_acquire_done(acquire_ctx); + ww_acquire_fini(acquire_ctx); return ret; } }
Use ww_acquire_fini() in the error code paths. Otherwise lockdep thinks that lock is held when lock's memory is freed after the drm_gem_lock_reservations() error. The WW needs to be annotated as "freed", which fixes the noisy "WARNING: held lock freed!" splat of VirtIO-GPU driver with CONFIG_DEBUG_MUTEXES=y and enabled lockdep. Cc: stable@vger.kernel.org Signed-off-by: Dmitry Osipenko <dmitry.osipenko@collabora.com> --- drivers/gpu/drm/drm_gem.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)