On Mon, Apr 6, 2020 at 10:25 PM Matthew Wilcox <willy@xxxxxxxxxxxxx> wrote: > > On Mon, Apr 06, 2020 at 10:16:56PM +0800, Yafang Shao wrote: > > On Mon, Apr 6, 2020 at 10:11 PM Matthew Wilcox <willy@xxxxxxxxxxxxx> wrote: > > > > > > On Mon, Apr 06, 2020 at 10:09:55PM +0800, Yafang Shao wrote: > > > > What about ida_alloc_range() ? Should we correct it as well ? > > > > > > > > > > > > * Return: The allocated ID, or %-ENOMEM if memory could not be allocated, > > > > * or %-ENOSPC if there are no free IDs. > > > > */ > > > > int ida_alloc_range(struct ida *ida, unsigned int min, unsigned int max, > > > > gfp_t gfp) > > > > > > > > If there're no free IDs, ida_alloc_range() also returns ENOSPC. > > > > > > Do you want to check all the callers of ida_.*alloc()? > > > > Sorry, I can't your point. > > Changing the value returned by ida_alloc_range() would require checking > every caller to see if it would break anything. That's why I didn't > change it when I rewrote it. > Got it. Thanks for the explaination. > > I just find the mem_cgroup_css_alloc() will use ida too. > > mem_cgroup_css_alloc > > memcg_online_kmem > > memcg_alloc_cache_id > > ida_simple_get > > > > I think we should keep the behavior consistent here. > > ENOSPC doesn't make much sense here. So I'd do: > > id = ida_simple_get(&memcg_cache_ida, > 0, MEMCG_CACHES_MAX_SIZE, GFP_KERNEL); > + if (id == -ENOSPC) > + return -EBUSY; > if (id < 0) > return id; > Understood! Thanks Yafang
