On 4/23/21 4:49 PM, Michal Hocko wrote: > 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. I have one more cleanup for ipc, so if no one objects, I'll fix this case, add my patch and send them together as a separate patch set a bit later. Thank you, Vasily Averin
