On Mon, Apr 6, 2020 at 9:30 PM Yafang Shao <laoar.shao@xxxxxxxxx> wrote: > > On Mon, Apr 6, 2020 at 9:05 PM Matthew Wilcox <willy@xxxxxxxxxxxxx> wrote: > > > > On Mon, Apr 06, 2020 at 08:54:07AM -0400, Yafang Shao wrote: > > > When I run my memcg testcase which creates lots of memcgs, I found > > > there're unexpected out of memory logs while there're still enough > > > available free memory. The error log is, > > > mkdir: cannot create directory 'foo.65533': Cannot allocate memory > > > > > > The reason is when we try to create more than MEM_CGROUP_ID_MAX memcgs, an > > > -ENOMEM errno will be set by mem_cgroup_css_alloc(), but the right errno > > > should be -ENOSPC, as explained above the function idr_alloc(). > > > > I think idr_alloc() is wrong. I think the right errno to return here is > > EBUSY "Device or resource busy". > > > > Agree with you that EBUSY is better. > I will correct it. > Hi Matthew, 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. > > > -static struct mem_cgroup *mem_cgroup_alloc(void) > > > +static struct mem_cgroup *mem_cgroup_alloc(long *error) > > > > The normal way to do this is to return an ERR_PTR(). See > > include/linux/err.h. > > > > Thanks for your advise. I will take a look at it. > > Thanks Yafang