On Fri, 2026-03-06 at 15:43 +0000, Jonathan Cavitt wrote: > Static analysis issue: > > Though probably unnecessary given the cache is being freed at this > step, > for the sake of consistency, ensure that the cache lock is always > unlocked after drm_pagemap_cache_fini. >
Typically you don't start the commit message with the tool that found it, like "fixing an error found by checkpatch.pl" or something similar, but rather mention it at the bottom after explaining the fix and its implications. Also I'd skip the "Though probably unnecessary..." sentence. Spinlocks typically disable preemption and if the code-path missing the unlock is hit, preemption will remain disabled even if the lock is subsequently freed. Fix looks good, so with the commit message fixed, Reviewed-by: Thomas Hellström <[email protected]> > v2: > - Use requested code flow (Maarten) > > v3: > - Clear cache->dpagemap (Matt Brost, Maarten) > > Fixes: 77f14f2f2d73f ("drm/pagemap: Add a drm_pagemap cache and > shrinker") > Signed-off-by: Jonathan Cavitt <[email protected]> > Cc: Thomas Hellstrom <[email protected]> > Cc: Matthew Brost <[email protected]> > Cc: Maarten Lankhorst <[email protected]> > --- > drivers/gpu/drm/drm_pagemap_util.c | 14 +++++--------- > 1 file changed, 5 insertions(+), 9 deletions(-) > > diff --git a/drivers/gpu/drm/drm_pagemap_util.c > b/drivers/gpu/drm/drm_pagemap_util.c > index 14ddb948a32e..6111d90a38e2 100644 > --- a/drivers/gpu/drm/drm_pagemap_util.c > +++ b/drivers/gpu/drm/drm_pagemap_util.c > @@ -65,18 +65,14 @@ static void drm_pagemap_cache_fini(void *arg) > drm_dbg(cache->shrinker->drm, "Destroying dpagemap > cache.\n"); > spin_lock(&cache->lock); > dpagemap = cache->dpagemap; > - if (!dpagemap) { > - spin_unlock(&cache->lock); > - goto out; > - } > + cache->dpagemap = NULL; > + if (dpagemap && !drm_pagemap_shrinker_cancel(dpagemap)) > + dpagemap = NULL; > + spin_unlock(&cache->lock); > > - if (drm_pagemap_shrinker_cancel(dpagemap)) { > - cache->dpagemap = NULL; > - spin_unlock(&cache->lock); > + if (dpagemap) > drm_pagemap_destroy(dpagemap, false); > - } > > -out: > mutex_destroy(&cache->lookup_mutex); > kfree(cache); > }
