On Fri, Oct 16, 2020 at 11:47:02AM +0200, Michal Koutný wrote: > Hello. > > On Wed, Oct 14, 2020 at 08:07:49PM +0100, Richard Palethorpe <rpalethorpe@xxxxxxxx> wrote: > > SLAB objects which outlive their memcg are moved to their parent > > memcg where they may be uncharged. However if they are moved to the > > root memcg, uncharging will result in negative page counter values as > > root has no page counters. > Why do you think those are reparented objects? If those are originally > charged in a non-root cgroup, then the charge value should be propagated up the > hierarchy, including root memcg, so if they're later uncharged in root > after reparenting, it should still break even. (Or did I miss some stock > imbalance?) Looking a bit closer at this code, it's kind of a mess right now. The central try_charge() function charges recursively all the way up to and including the root. But not if it's called directly on the root, in which case it bails and does nothing. kmem and objcg use try_charge(), so they have the same behavior. get_obj_cgroup_from_current() does it's own redundant filtering for root_mem_cgroup, whereas get_mem_cgroup_from_current() does not, but its callsite __memcg_kmem_charge_page() does. We should clean this up one way or another: either charge the root or don't, but do it consistently. Since we export memory.stat at the root now, we should probably just always charge the root instead of special-casing it all over the place and risking bugs. Indeed, it looks like there is at least one bug where the root-level memory.stat shows non-root slab objects, but not root ones, whereas it shows all anon and cache pages, root or no root.
