Re: [PATCH] memcg: protect concurrent access to mem_cgroup_idr

[Matthew Wilcox <willy@infradead.org>]

On Fri, Aug 02, 2024 at 04:58:22PM -0700, Shakeel Butt wrote:
>  #define MEM_CGROUP_ID_MAX	((1UL << MEM_CGROUP_ID_SHIFT) - 1)
>  static DEFINE_IDR(mem_cgroup_idr);
> +static DEFINE_SPINLOCK(memcg_idr_lock);
> +
> +static int mem_cgroup_alloc_id(void)
> +{
> +	int ret;
> +
> +	idr_preload(GFP_KERNEL);
> +	spin_lock(&memcg_idr_lock);
> +	ret = idr_alloc(&mem_cgroup_idr, NULL, 1, MEM_CGROUP_ID_MAX + 1,
> +			GFP_NOWAIT);
> +	spin_unlock(&memcg_idr_lock);
> +	idr_preload_end();
> +	return ret;
> +}

You know, this works much better as an xarray:

static int mem_cgroup_alloc_id(void)
{
	u32 id;
	int ret;

	ret = xa_alloc(&mem_cgroup_ids, &id, XA_LIMIT(1, MEM_CGROUP_ID_MAX),
			GFP_KERNEL);
	if (ret < 0)
		return ret;
	return id;
}

No messing around with preloading, the spinlock is built in and the MAX
works the way you want it to.