Re: [PATCH 3/8] execmem: rework execmem_cache_free()

From: Peter Zijlstra
Date: Mon Jul 07 2025 - 07:11:32 EST


On Fri, Jul 04, 2025 at 04:49:38PM +0300, Mike Rapoport wrote:
> static bool execmem_cache_free(void *ptr)
> {
> struct maple_tree *busy_areas = &execmem_cache.busy_areas;
> unsigned long addr = (unsigned long)ptr;
> MA_STATE(mas, busy_areas, addr, addr);
> void *area;
> + int err;
> +
> + guard(mutex)(&execmem_cache.mutex);
>
> area = mas_walk(&mas);
> + if (!area)
> return false;
>
> + err = __execmem_cache_free(&mas, ptr, GFP_KERNEL | __GFP_NORETRY);
> + if (err)
> + goto err_slowpath;
>
> schedule_work(&execmem_cache_clean_work);
>
> return true;
> +
> +err_slowpath:
> + mas_store_gfp(&mas, pending_free_set(ptr), GFP_KERNEL);
> + execmem_cache.pending_free_cnt++;
> + schedule_delayed_work(&execmem_cache_free_work, FREE_DELAY);
> + return true;
> }

This is a bit if an anti-pattern, using guard() and error goto. Since
there is only the one site, its best to write it like so:

static bool execmem_cache_free(void *ptr)
{
struct maple_tree *busy_areas = &execmem_cache.busy_areas;
unsigned long addr = (unsigned long)ptr;
MA_STATE(mas, busy_areas, addr, addr);
void *area;
int err;

guard(mutex)(&execmem_cache.mutex);

area = mas_walk(&mas);
if (!area)
return false;

err = __execmem_cache_free(&mas, ptr, GFP_KERNEL | __GFP_NORETRY);
if (err) {
mas_store_gfp(&mas, pending_free_set(ptr), GFP_KERNEL);
execmem_cache.pending_free_cnt++;
schedule_delayed_work(&execmem_cache_free_work, FREE_DELAY);
return true;
}

schedule_work(&execmem_cache_clean_work);
return true;
}

And now I have to ask what happens if mas_store_gfp() returns an error?