On 05/04, Eric W. Biederman wrote: > > Oleg Nesterov <o...@redhat.com> writes: > > > On 05/04, Eric W. Biederman wrote: > >> > >> Oleg Nesterov <o...@redhat.com> writes: > >> > >> > I'd vote for the change in exec_mmap(). This way mm_init_memcg() can just > >> > nullify mm->memcg. > >> > >> There is at least one common path where we need the memory control group > >> properly initialized so memory allocations don't escape the memory > >> control group. > >> > >> do_execveat_common > >> copy_strings > >> get_arg_page > >> get_user_pages_remote > >> __get_user_pages_locked > >> __get_user_pages > >> faultin_page > >> handle_mm_fault > >> count_memcg_event_mm > >> __handle_mm_fault > >> handle_pte_fault > >> do_anonymous_page > >> mem_cgroup_try_charge
Ah yes, indeed. > mm_init_memcg is at the same point as mm_init_owner. So my change did > not introduce any logic changes on when the memory control group became > valid. Not sure, but perhaps I am all confused.... before your patch get_mem_cgroup_from_mm() looks at mm->owner == current (in this case) and mem_cgroup_from_task() should return the correct memcg even if execing task migrates after bprm_mm_init(). At least in the common case when the old mm is not shared. After your patch the memory allocations in copy_strings() won't be accounted correctly, bprm->mm->memcg is wrong if this task migrates. And iiuc your recent "[PATCH 2/2] memcg: Close the race between migration and installing bprm->mm as mm" doesn't fix the problem. No? Perhaps we can change get_mem_cgroup_from_mm() to use mem_cgroup_from_css(current, memory_cgrp_id) if mm->memcg == NULL? Oleg.
