| Message ID | 1675312377-4782-1-git-send-email-zhaoyang.huang@unisoc.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | mm: introduce entrance for root_mem_cgroup's current | expand |
On Wed, Feb 1, 2023 at 8:34 PM zhaoyang.huang <zhaoyang.huang@unisoc.com> wrote: > > From: Zhaoyang Huang <zhaoyang.huang@unisoc.com> > > Introducing memory.root_current for the memory charges on root_mem_cgroup. Why and how are you planning to use it?
On Thu, Feb 2, 2023 at 2:30 PM Shakeel Butt <shakeelb@google.com> wrote: > > On Wed, Feb 1, 2023 at 8:34 PM zhaoyang.huang <zhaoyang.huang@unisoc.com> wrote: > > > > From: Zhaoyang Huang <zhaoyang.huang@unisoc.com> > > > > Introducing memory.root_current for the memory charges on root_mem_cgroup. > > Why and how are you planning to use it? root_mem_cgroup is lack of such statistic entry for debug purpose, which is the counterpart of memory.current within other memcgs
On Thu 02-02-23 12:32:57, zhaoyang.huang wrote: > From: Zhaoyang Huang <zhaoyang.huang@unisoc.com> > > Introducing memory.root_current for the memory charges on root_mem_cgroup. Charges are not currently accounted for the root memcg universally. See try_charge which is used for all user space and skmem charges. I am not 100% sure about objcg based accounting because there is no explicit check for the root memcg but this might be hidden somewhere as well. That means that the patch as is doesn't really provide and usable value. The root exemption has been removed in the past but that has been reverted due to a regression. See ce00a967377b ("mm: memcontrol: revert use of root_mem_cgroup res_counter") for more. > Signed-off-by: Zhaoyang Huang <zhaoyang.huang@unisoc.com> > --- > mm/memcontrol.c | 5 +++++ > 1 file changed, 5 insertions(+) > > diff --git a/mm/memcontrol.c b/mm/memcontrol.c > index ab457f0..158d4232 100644 > --- a/mm/memcontrol.c > +++ b/mm/memcontrol.c > @@ -6681,6 +6681,11 @@ static ssize_t memory_reclaim(struct kernfs_open_file *of, char *buf, > > static struct cftype memory_files[] = { > { > + .name = "root_current", > + .flags = CFTYPE_ONLY_ON_ROOT, > + .read_u64 = memory_current_read, > + }, > + { > .name = "current", > .flags = CFTYPE_NOT_ON_ROOT, > .read_u64 = memory_current_read, > -- > 1.9.1
On Thu, Feb 2, 2023 at 12:27 AM Michal Hocko <mhocko@suse.com> wrote: > > On Thu 02-02-23 12:32:57, zhaoyang.huang wrote: > > From: Zhaoyang Huang <zhaoyang.huang@unisoc.com> > > > > Introducing memory.root_current for the memory charges on root_mem_cgroup. > > Charges are not currently accounted for the root memcg universally. See > try_charge which is used for all user space and skmem charges. I am not > 100% sure about objcg based accounting because there is no explicit > check for the root memcg but this might be hidden somewhere as well. Yes in __get_obj_cgroup_from_memcg(). However the reason to use try_charge_memcg() to bypass root check was to avoid the race with reparenting. More details in c5c8b16b596e ("mm: memcontrol: fix root_mem_cgroup charging"). > > That means that the patch as is doesn't really provide and usable value. > The root exemption has been removed in the past but that has been > reverted due to a regression. See ce00a967377b ("mm: memcontrol: revert > use of root_mem_cgroup res_counter") for more. > One advantage I can see is if someone is looking for usage for all top containers (alive or zombie) but I wanted to know if that was the real motivation behind the patch.
On Thu 02-02-23 10:24:08, Shakeel Butt wrote: > On Thu, Feb 2, 2023 at 12:27 AM Michal Hocko <mhocko@suse.com> wrote: > > > > On Thu 02-02-23 12:32:57, zhaoyang.huang wrote: > > > From: Zhaoyang Huang <zhaoyang.huang@unisoc.com> > > > > > > Introducing memory.root_current for the memory charges on root_mem_cgroup. > > > > Charges are not currently accounted for the root memcg universally. See > > try_charge which is used for all user space and skmem charges. I am not > > 100% sure about objcg based accounting because there is no explicit > > check for the root memcg but this might be hidden somewhere as well. > > Yes in __get_obj_cgroup_from_memcg(). However the reason to use > try_charge_memcg() to bypass root check was to avoid the race with > reparenting. More details in c5c8b16b596e ("mm: memcontrol: fix > root_mem_cgroup charging"). Thanks for the pointer! > > That means that the patch as is doesn't really provide and usable value. > > The root exemption has been removed in the past but that has been > > reverted due to a regression. See ce00a967377b ("mm: memcontrol: revert > > use of root_mem_cgroup res_counter") for more. > > > > One advantage I can see is if someone is looking for usage for all top > containers (alive or zombie) but I wanted to know if that was the real > motivation behind the patch. Isn't that just a global stats that we already display via /proc files?
On Thu, Feb 02, 2023 at 09:27:39AM +0100, Michal Hocko wrote: > On Thu 02-02-23 12:32:57, zhaoyang.huang wrote: > > From: Zhaoyang Huang <zhaoyang.huang@unisoc.com> > > > > Introducing memory.root_current for the memory charges on root_mem_cgroup. > > Charges are not currently accounted for the root memcg universally. See > try_charge which is used for all user space and skmem charges. I am not > 100% sure about objcg based accounting because there is no explicit > check for the root memcg but this might be hidden somewhere as well. root_mem_cgroup has no corresponding obj_cgroup: see memcg_online_kmem(). Thanks
On Fri, Feb 03, 2023 at 09:58:56AM +0100, Michal Hocko wrote: [...] > > > > One advantage I can see is if someone is looking for usage for all top > > containers (alive or zombie) but I wanted to know if that was the real > > motivation behind the patch. > > Isn't that just a global stats that we already display via /proc files? > Things are a bit complicated for kernel memory. Let's take a simple example where there are no processes in the root memcg. In this case the user memory stats should be similar to the global stats under /proc because we always charge user memory. However the kernel memory has to be opted-in to be accounted. So, we have a lot of allocations which are in the global stats but not in the memcg stats. We can traverse the top level memcgs to get kernel stats and subtract it from the global stats which will give the sum of zombie kernel memory and unaccounted kernel memory. For debugging and history/analysis purpose, differentiating between zombie and unaccounted makes sense.
diff --git a/mm/memcontrol.c b/mm/memcontrol.c index ab457f0..158d4232 100644 --- a/mm/memcontrol.c +++ b/mm/memcontrol.c @@ -6681,6 +6681,11 @@ static ssize_t memory_reclaim(struct kernfs_open_file *of, char *buf, static struct cftype memory_files[] = { { + .name = "root_current", + .flags = CFTYPE_ONLY_ON_ROOT, + .read_u64 = memory_current_read, + }, + { .name = "current", .flags = CFTYPE_NOT_ON_ROOT, .read_u64 = memory_current_read,