-----Original Message----- From: Brost, Matthew <[email protected]> Sent: Tuesday, February 24, 2026 3:12 PM To: Cavitt, Jonathan <[email protected]> Cc: [email protected]; Gupta, Saurabhg <[email protected]>; Zuo, Alex <[email protected]>; [email protected]; [email protected] Subject: Re: [PATCH v2] drm/pagemap_util: Ensure proper cache lock management on free > > On Tue, Feb 24, 2026 at 03:48:33PM +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. > > > > v2: > > - Use requested code flow (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 | 13 ++++--------- > > 1 file changed, 4 insertions(+), 9 deletions(-) > > > > diff --git a/drivers/gpu/drm/drm_pagemap_util.c > > b/drivers/gpu/drm/drm_pagemap_util.c > > index 14ddb948a32e..66203a19f8f6 100644 > > --- a/drivers/gpu/drm/drm_pagemap_util.c > > +++ b/drivers/gpu/drm/drm_pagemap_util.c > > @@ -65,18 +65,13 @@ 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; > > - } > > + 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); > > The logic is different here as 'cache->dpagemap' never gets set to NULL > under cache->lock when drm_pagemap_shrinker_cancel returns true.
I forgot to add this back in as a part of the v2 request, although in this case, cache->dpagemap = NULL can just be assigned as such without checking the return of drm_pagemap_shrinker_cancel. I'll fix this for V3 and add the intel-xe list at that time. Thank you. -Jonathan Cavitt > > Also be sure to include intel-xe list for CI too. > > Matt > > > - } > > > > -out: > > mutex_destroy(&cache->lookup_mutex); > > kfree(cache); > > } > > -- > > 2.43.0 > > >
