Den 2026-02-20 kl. 20:09, skrev Jonathan Cavitt:
> 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.
>
> 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 | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_pagemap_util.c
> b/drivers/gpu/drm/drm_pagemap_util.c
> index c6ae3357c7fb..79e07b3658ed 100644
> --- a/drivers/gpu/drm/drm_pagemap_util.c
> +++ b/drivers/gpu/drm/drm_pagemap_util.c
> @@ -65,18 +65,18 @@ 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);
> + if (!dpagemap)
> goto out;
> - }
>
> if (drm_pagemap_shrinker_cancel(dpagemap)) {
> cache->dpagemap = NULL;
> spin_unlock(&cache->lock);
> drm_pagemap_destroy(dpagemap, false);
> + } else {
> +out:
> + spin_unlock(&cache->lock);
> }
>
> -out:
> mutex_destroy(&cache->lookup_mutex);
> kfree(cache);
> }
I think this flow without goto is slightly more readable:
spin_lock(&cache->lock);
dpagemap = cache->dpagemap;
cache->dpagemap = NULL;
if (dpagemap && !drm_pagemap_shrinker_cancel(dpagemap))
dpagemap = NULL;
spin_unlock(&cache->lock);
if (dpagemap)
drm_pagemap_destroy(dpagemap, false);
mutex_destroy(&cache->lookup_mutex);
kfree(cache);
}
Although since we are freeing, it doesn't matter if cache->dpagemap stays
assigned,
so perhaps the cache->dpagemap = NULL assignment can be removed too.
Kind regards,
~Maarten Lankhorst
