From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id 4C51CD18157 for ; Tue, 15 Oct 2024 00:16:21 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id A516F6B0082; Mon, 14 Oct 2024 20:16:20 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id A011C6B0083; Mon, 14 Oct 2024 20:16:20 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 8C88D6B0085; Mon, 14 Oct 2024 20:16:20 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0017.hostedemail.com [216.40.44.17]) by kanga.kvack.org (Postfix) with ESMTP id 6E1626B0082 for ; Mon, 14 Oct 2024 20:16:20 -0400 (EDT) Received: from smtpin12.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay04.hostedemail.com (Postfix) with ESMTP id 6A2BD1A0440 for ; Tue, 15 Oct 2024 00:16:05 +0000 (UTC) X-FDA: 82673919510.12.A1014F4 Received: from mail-ed1-f43.google.com (mail-ed1-f43.google.com [209.85.208.43]) by imf06.hostedemail.com (Postfix) with ESMTP id 52CFC18000C for ; Tue, 15 Oct 2024 00:16:13 +0000 (UTC) Authentication-Results: imf06.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=Iumsq9XD; spf=pass (imf06.hostedemail.com: domain of yosryahmed@google.com designates 209.85.208.43 as permitted sender) smtp.mailfrom=yosryahmed@google.com; dmarc=pass (policy=reject) header.from=google.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1728951306; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=yUwy3k/yvTh1P0jg3KQQzBu8UranTdme7b+VHHlPFEs=; b=t6T+Qbt0btJaBK4DSxXG+CrDOqXCVBc8TFo9PINADvWUf/IGukizIStZP/VH6JOi48Ljnn P4JLCJUXJi2pYLYriJLsRoJm2Tb2lqncemsbyTTrS6QoTmB+8j9/5BYj6pJ8u/uXkyJzXn 0eBVvBgEtp/IBuM7alG3DMWgunjPOoA= ARC-Authentication-Results: i=1; imf06.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=Iumsq9XD; spf=pass (imf06.hostedemail.com: domain of yosryahmed@google.com designates 209.85.208.43 as permitted sender) smtp.mailfrom=yosryahmed@google.com; dmarc=pass (policy=reject) header.from=google.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1728951306; a=rsa-sha256; cv=none; b=I7HofqJonwLa8/X3pOMzCv8RQKB/skRK58lLy7XjH9J63tHTCaIBK8AmvKtLOwCp9tXYYc tghSa2O0auGPPl6H8Ge+vUG9ptQo1IQXzXmZ2nbgp0FYCB89KRGW3ii1efDt/vQAdiX/WJ Ahu5wq9cLHKfi8B8hIl8G3Efuhucyl8= Received: by mail-ed1-f43.google.com with SMTP id 4fb4d7f45d1cf-5c42e7adbddso6350416a12.2 for ; Mon, 14 Oct 2024 17:16:17 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1728951377; x=1729556177; darn=kvack.org; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=yUwy3k/yvTh1P0jg3KQQzBu8UranTdme7b+VHHlPFEs=; b=Iumsq9XDIGgMWy7BeWGloFKpiK/Pj5zNf4vu9TCXxp3zCzspnXEhzlycHtOgxBHk/Y uTp+PydEzQ1IRqp8JKsJZecV325ZcUB0C2ufD49NhJ9CPXOJo9P9rUgYthZX3xG2fePX EW8VNRmuG6VCA8Rr/X8bhgdhxVPknlmjvQY1SinazWl7hnlcH6U0rtj0em4pP1LEvnAy teUSQ5zNh3QMMZ6zV0kOkIULPzmZ7/FbEgJkgo8sIIPcM+9U/XcsGDFRZD7D84BtffOI nQM/eplSLv22svmuK+s7en17PSm0agBk/EZoLn6Lxv7/kWXqYJi/kTUMXCsVnJkk4/U5 BWMQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1728951377; x=1729556177; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=yUwy3k/yvTh1P0jg3KQQzBu8UranTdme7b+VHHlPFEs=; b=mmXWvMKwMzx/nVLiMprCPlXhOCPTdRJIIRfvXtVl9wJeOPQVyieadYNGVDvIcsSjGV ntFOQorUrFdoPqOuxS8Ebq4ehi7b3rEKJfkT5xrnc7ZSEYwBZiydn21SQor+zGa8lU+k 7jWgbO1SU7ZI/2jdd/vtzQCu+raJDKWW6xWagwGD/wPitMFE9PnyouJmLFKmWD7z8akA h5Js2EhbHLvgpVFCRfQtDnIyly0KjDYAZg+9hXncPXAS3SqM3IJ2sHri7Wi0vvY3Kd3W 6ma1BZKl7RiA1LEHlVqKO/oQOvyREodJIoptsBBHoBbOaDUX0qa5jSNffNseIToI81Lt 0pdA== X-Forwarded-Encrypted: i=1; AJvYcCXBQQCU9cEiN/VfxvftDEqdR2JJW+Riff6Cp5cIkO3kHRLBakwmILKAnR25rlafg1Fnri4WPhmEwA==@kvack.org X-Gm-Message-State: AOJu0Yxa7Rw7E4jpN3lYIttdslX+5vAfUo7wGaMpWYqLr+JCv8P9D7Yv q1wGCZe9TzrDRTIobW9IZAv1/lrhP4e2mCkrEITudkyMNKhmxJsKs/qq+FLOFzReL5kVwqA6BIj XNSNIAbdbOYJO+DWFsIokxuCjMX0gDvL1UnMR X-Google-Smtp-Source: AGHT+IFyuclxGr8kzE0yboNkloz1/Ka2CW85rwSfqZCuQkiJTXnSPUvh3/2JlnmcyxILN665SqWviq0+zsn9VXmK3tM= X-Received: by 2002:a17:907:934c:b0:a7d:c148:ec85 with SMTP id a640c23a62f3a-a99e3e9c00cmr875919066b.62.1728951376240; Mon, 14 Oct 2024 17:16:16 -0700 (PDT) MIME-Version: 1.0 References: <20241010003550.3695245-1-shakeel.butt@linux.dev> <20241009210848.43adb0c3@gandalf.local.home> In-Reply-To: From: Yosry Ahmed Date: Mon, 14 Oct 2024 17:15:39 -0700 Message-ID: Subject: Re: [PATCH] memcg: add tracing for memcg stat updates To: Shakeel Butt Cc: Steven Rostedt , Andrew Morton , Johannes Weiner , Michal Hocko , Roman Gushchin , Muchun Song , JP Kobryn , linux-mm@kvack.org, cgroups@vger.kernel.org, linux-kernel@vger.kernel.org, Meta kernel team , Daniel Xu , bpf@vger.kernel.org, Martin KaFai Lau Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Rspamd-Server: rspam03 X-Rspam-User: X-Rspamd-Queue-Id: 52CFC18000C X-Stat-Signature: 7p94ro4x9byr3ate8zy11iso4bmkdyuw X-HE-Tag: 1728951373-237221 X-HE-Meta: U2FsdGVkX1/3C7crx29myAvfMo9oIyPbj1IrGeNW5ck5eqZgmtaX9d9+t4cb8UqmTMwOH/ozFhO4O7qAq1RkZAl8tE7kABKPavQC3y5xfeyFP1so6htpnmW+S7F/IFNfF1r7dz7X2GNyCpYlgPyMlII6DLnj5LXRO07/lyZ/vEqmgUfKMHFn6wksWbwxOPG6OhkDDeMHTYJpPf1fhD83w6ribDw8y7h7Nf15IOeMPn61OvXkGQWH7N2ST3K/QeJI6kyELM18g8e8AaWNumnLA1C5GRCugfskiTctYrT7KsQS7odOTqxsCde7o5W+I2Lm+x21bLV5qEutG4CzlpdWP2IRg7+43odndh6u1FnbFqmWvvplHj/W+FTaaFY5mj1phPxhhW/4QGbGB5qC9TYr2z8K5ct+MGv7njbsqqBo8wpN1nkwXLJU2d+vBsWTOxuC8CYj0E5Z41T1jobdjjiE6lGYjeqaCNYF8AyCVIEkUku1ZB3sPv6Ipvn3j3wkDHhiL58dx9CmPNK0DyhNAHQcvgym0nnqA2s6BIF7bEc7QL0htdKVXAQcTpXE/WNOd6f0Zxp0CC2CVhR2gaX2cRqXLxgrqWqJTsHB6Kc4IqstMVXn+SxbDr56qeuLWfSwjSWLSZF77RHPrwX6E6AgcV9bn9AAZCQkthQlTzzYARE2ALLR+f0oKENmogaXWWeFH41m+BUF+QcI5LiPGdhkPN8dlCfJue7NQSL1o60ZcUAGsdmgtgO1Jz0Kd3Ta24n75ef/qeca34IHhlq+eoxpIwe2ZpyAu4vlVwbcADasbd9D6nW+aqDskdaNxWdj14IuSRV60oyDzUD91uUDdCOVfGZISI/XKknq3LJ2lPfMbdruNelpX7GIdtgLkdAtGIYqs/1J1mIJlGrJrlsVg9aeHgEgAI1UmPQbfT3Ep2EeiCYyB9o6cF++IEHrEsAAcd6cYIN/a3FvjfhoTjfVkw6o1y0 bqZMD7wr Th0yG/3WgerZY2pXvzk2Bg0QhQ6Z4jnSF5EfWWtXc/nGxr/GVgZsOYdIunTrmH8zkR1tZmKNeKEWypWd+bru7e3YBDOg198YrR/UU9QEOafa/LEM/HE2H2UW6QTfc9TOlEUFTTtdssaN1eWOQ4NEv/kIRYYNl+in4a+z6ONPhpw/Xxz9PwZQdmeiM3P6vhsK4JWPt5zrV43sesQzFkcIMvbbGI8wdYPWpZVM0vuaPgYRqJ2G65/nVutmQKnzGZVhRi444aQWtMVbH6RPX99J8slGHWPt5fE8PBEdPb6cxN29JiDSFHSgqFmogOA== X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: List-Subscribe: List-Unsubscribe: On Thu, Oct 10, 2024 at 10:26=E2=80=AFAM Shakeel Butt wrote: > > On Wed, Oct 09, 2024 at 06:24:55PM GMT, Yosry Ahmed wrote: > > On Wed, Oct 9, 2024 at 6:08=E2=80=AFPM Steven Rostedt wrote: > > > > > > On Wed, 9 Oct 2024 17:46:22 -0700 > > > Yosry Ahmed wrote: > > > > > > > > +++ b/mm/memcontrol.c > > > > > @@ -71,6 +71,10 @@ > > > > > > > > > > #include > > > > > > > > > > +#define CREATE_TRACE_POINTS > > > > > +#include > > > > > +#undef CREATE_TRACE_POINTS > > > > > + > > > > > #include > > > > > > > > > > struct cgroup_subsys memory_cgrp_subsys __read_mostly; > > > > > @@ -682,7 +686,9 @@ void __mod_memcg_state(struct mem_cgroup *mem= cg, enum memcg_stat_item idx, > > > > > return; > > > > > > > > > > __this_cpu_add(memcg->vmstats_percpu->state[i], val); > > > > > - memcg_rstat_updated(memcg, memcg_state_val_in_pages(idx, = val)); > > > > > + val =3D memcg_state_val_in_pages(idx, val); > > > > > + memcg_rstat_updated(memcg, val); > > > > > + trace_mod_memcg_state(memcg, idx, val); > > > > > > > > Is it too unreasonable to include the stat name? > > > > > > > > The index has to be correlated with the kernel config and perhaps e= ven > > > > version. It's not a big deal, but if performance is not a concern w= hen > > > > tracing is enabled anyway, maybe we can lookup the name here (or in > > > > TP_fast_assign()). > > > > > > What name? Is it looked up from idx? If so, you can do it on the read= ing of > > Does reading side mean the one reading /sys/kernel/tracing/trace will do > the translation from enums to string? > > > > the trace event where performance is not an issue. See the __print_sy= mbolic() > > > and friends in samples/trace_events/trace-events-sample.h > > > > Yeah they can be found using idx. Thanks for referring us to > > __print_symbolic(), I suppose for this to work we need to construct an > > array of {idx, name}. I think we can replace the existing memory_stats > > and memcg1_stats/memcg1_stat_names arrays with something that we can > > reuse for tracing, so we wouldn't need to consume extra space. > > > > Shakeel, what do you think? > > Cc Daniel & Martin > > I was planning to use bpftrace which can use dwarf/btf to convert the > raw int to its enum string. Martin provided the following command to > extract the translation from the kernel. > > $ bpftool btf dump file /sys/kernel/btf/vmlinux | grep -A10 node_stat_ite= m > [2264] ENUM 'node_stat_item' encoding=3DUNSIGNED size=3D4 vlen=3D46 > 'NR_LRU_BASE' val=3D0 > 'NR_INACTIVE_ANON' val=3D0 > 'NR_ACTIVE_ANON' val=3D1 > 'NR_INACTIVE_FILE' val=3D2 > 'NR_ACTIVE_FILE' val=3D3 > 'NR_UNEVICTABLE' val=3D4 > 'NR_SLAB_RECLAIMABLE_B' val=3D5 > 'NR_SLAB_UNRECLAIMABLE_B' val=3D6 > 'NR_ISOLATED_ANON' val=3D7 > 'NR_ISOLATED_FILE' val=3D8 > ... > > My point is userspace tools can use existing infra to extract this > information. > > However I am not against adding __print_symbolic() (but without any > duplication), so users reading /sys/kernel/tracing/trace directly can > see more useful information as well. Please post a follow up patch after > this one. I briefly looked into this and I think it would be annoying to have this, unfortunately. Even if we rework the existing arrays with memcg stat names to be in a format conforming to tracing, we would need to move them out to a separate header to avoid a circular dependency. Additionally, for __count_memcg_events() things will be more complicated because the names are not in an array in memcontrol.c, but we use vm_event_name() and the relevant names are part of a larger array, vmstat_text, which we would need to rework similarly. I think this would be easier to implement if we can somehow provide a callback that returns the name based on the index, rather than an array. But even then, we would need to specify a different callback for each event, so it won't be as simple as specifying the callback in the event class. All in all, unless we realize there is something that is fundamentally more difficult to do in userspace, I think it's not worth adding this unfortunately :/