On Fri 23-04-21 15:40:58, Michal Hocko wrote: > On Fri 23-04-21 15:32:01, Vasily Averin wrote: > > On 4/23/21 3:16 PM, Alexey Dobriyan wrote: > > > On Thu, Apr 22, 2021 at 01:37:02PM +0300, Vasily Averin wrote: > > >> When user creates IPC objects it forces kernel to allocate memory for > > >> these long-living objects. > > >> > > >> It makes sense to account them to restrict the host's memory consumption > > >> from inside the memcg-limited container. > > >> > > >> This patch enables accounting for IPC shared memory segments, messages > > >> semaphores and semaphore's undo lists. > > > > > >> --- a/ipc/msg.c > > >> +++ b/ipc/msg.c > > >> @@ -147,7 +147,7 @@ static int newque(struct ipc_namespace *ns, struct ipc_params *params) > > >> key_t key = params->key; > > >> int msgflg = params->flg; > > >> > > >> - msq = kvmalloc(sizeof(*msq), GFP_KERNEL); > > >> + msq = kvmalloc(sizeof(*msq), GFP_KERNEL_ACCOUNT); > > > > > > Why this requires vmalloc? struct msg_queue is not big at all. > > > > > >> --- a/ipc/shm.c > > >> +++ b/ipc/shm.c > > >> @@ -619,7 +619,7 @@ static int newseg(struct ipc_namespace *ns, struct ipc_params *params) > > >> ns->shm_tot + numpages > ns->shm_ctlall) > > >> return -ENOSPC; > > >> > > >> - shp = kvmalloc(sizeof(*shp), GFP_KERNEL); > > >> + shp = kvmalloc(sizeof(*shp), GFP_KERNEL_ACCOUNT); > > > > > > Same question. > > > Kmem caches can be GFP_ACCOUNT by default. > > > > It is side effect: previously all these objects was allocated via ipc_alloc/ipc_alloc_rcu > > function called kvmalloc inside. > > > > Should I replace it to kmalloc right in this patch? > > I would say those are two independent things. I would agree that > kvmalloc is bogus here. The allocation would try SLAB allocator first > but if it fails (as kvmalloc doesn't try really hard) then it would > fragment memory without a good reason which looks like a bug to me. I have dug into history and this all seems to be just code pattern copy&paste thing. In the past it was ipc_alloc_rcu which was a common code for all IPCs and that code was conditional on rcu_use_vmalloc (HDRLEN_KMALLOC + size > PAGE_SIZE). Later changed to kvmalloc and then helper removed and all users to use kvmalloc. It seems that only semaphores can grow large enough to care about kvmalloc though. -- Michal Hocko SUSE Labs
