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. -- Michal Hocko SUSE Labs
