Hey Shakeel! On Thu, Feb 20, 2020 at 10:14:45AM -0800, Shakeel Butt wrote: > On Thu, Feb 20, 2020 at 8:52 AM Dan Schatzberg <schatzberg.dan@xxxxxxxxx> wrote: > > > > memalloc_use_memcg() worked for kernel allocations but was silently > > ignored for user pages. > > > > This patch establishes a precedence order for who gets charged: > > > > 1. If there is a memcg associated with the page already, that memcg is > > charged. This happens during swapin. > > > > 2. If an explicit mm is passed, mm->memcg is charged. This happens > > during page faults, which can be triggered in remote VMs (eg gup). > > > > 3. Otherwise consult the current process context. If it has configured > > a current->active_memcg, use that. > > What if css_tryget_online(current->active_memcg) in > get_mem_cgroup_from_current() fails? Do we want to change this to > css_tryget() and even if that fails should we fallback to > root_mem_cgroup or current->mm->memcg? Good questions. I think we can switch to css_tryget(). If a cgroup goes offline between issuing the IO and the loop layer executing that IO, the resources used could end up in the root instead of the closest ancestor of the offlined group. However, the risk of that actually happening and causing problems is probably pretty small, and the behavior isn't really worse than before Dan's patches. Would you mind sending a separate patch for this? AFAICS similar concerns apply to all users of foreign charging. As for tryget failing: can that actually happen? AFAICS, all current users acquire a reference first (get_memcg_from_somewhere()) that they assign to current->active_memcg. We should probably codify this rule and do WARN_ON(!css_tryget()) /* current->active_memcg must hold a ref */
