diff mbox series

[v2,1/1] lib: add version into /proc/allocinfo output

Message ID 20240514163128.3662251-1-surenb@google.com (mailing list archive)
State New, archived
Headers show
Series [v2,1/1] lib: add version into /proc/allocinfo output | expand

Commit Message

Suren Baghdasaryan May 14, 2024, 4:31 p.m. UTC
Add version string and a header at the beginning of /proc/allocinfo to
allow later format changes. Example output:

> head /proc/allocinfo
allocinfo - version: 1.0
#     <size>  <calls> <tag info>
           0        0 init/main.c:1314 func:do_initcalls
           0        0 init/do_mounts.c:353 func:mount_nodev_root
           0        0 init/do_mounts.c:187 func:mount_root_generic
           0        0 init/do_mounts.c:158 func:do_mount_root
           0        0 init/initramfs.c:493 func:unpack_to_rootfs
           0        0 init/initramfs.c:492 func:unpack_to_rootfs
           0        0 init/initramfs.c:491 func:unpack_to_rootfs
         512        1 arch/x86/events/rapl.c:681 func:init_rapl_pmus
         128        1 arch/x86/events/rapl.c:571 func:rapl_cpu_online

Signed-off-by: Suren Baghdasaryan <surenb@google.com>
---
Changes since v1 [1]:
- Added header with field names, per Pasha Tatashin
- Fixed a spelling error in the changelog

[1] https://lore.kernel.org/all/20240514153532.3622371-1-surenb@google.com/

 Documentation/filesystems/proc.rst |  5 ++--
 lib/alloc_tag.c                    | 48 ++++++++++++++++++++----------
 2 files changed, 36 insertions(+), 17 deletions(-)


base-commit: 7e8aafe0636cdcc5c9699ced05ff1f8ffcb937e2

Comments

Pasha Tatashin May 14, 2024, 4:35 p.m. UTC | #1
On Tue, May 14, 2024 at 10:31 AM Suren Baghdasaryan <surenb@google.com> wrote:
>
> Add version string and a header at the beginning of /proc/allocinfo to
> allow later format changes. Example output:
>
> > head /proc/allocinfo
> allocinfo - version: 1.0
> #     <size>  <calls> <tag info>
>            0        0 init/main.c:1314 func:do_initcalls
>            0        0 init/do_mounts.c:353 func:mount_nodev_root
>            0        0 init/do_mounts.c:187 func:mount_root_generic
>            0        0 init/do_mounts.c:158 func:do_mount_root
>            0        0 init/initramfs.c:493 func:unpack_to_rootfs
>            0        0 init/initramfs.c:492 func:unpack_to_rootfs
>            0        0 init/initramfs.c:491 func:unpack_to_rootfs
>          512        1 arch/x86/events/rapl.c:681 func:init_rapl_pmus
>          128        1 arch/x86/events/rapl.c:571 func:rapl_cpu_online
>
> Signed-off-by: Suren Baghdasaryan <surenb@google.com>

Reviewed-by: Pasha Tatashin <pasha.tatashin@soleen.com>

Thank you,
Pasha

> ---
> Changes since v1 [1]:
> - Added header with field names, per Pasha Tatashin
> - Fixed a spelling error in the changelog
>
> [1] https://lore.kernel.org/all/20240514153532.3622371-1-surenb@google.com/
>
>  Documentation/filesystems/proc.rst |  5 ++--
>  lib/alloc_tag.c                    | 48 ++++++++++++++++++++----------
>  2 files changed, 36 insertions(+), 17 deletions(-)
>
> diff --git a/Documentation/filesystems/proc.rst b/Documentation/filesystems/proc.rst
> index 245269dd6e02..4b71b3903d46 100644
> --- a/Documentation/filesystems/proc.rst
> +++ b/Documentation/filesystems/proc.rst
> @@ -961,13 +961,14 @@ Provides information about memory allocations at all locations in the code
>  base. Each allocation in the code is identified by its source file, line
>  number, module (if originates from a loadable module) and the function calling
>  the allocation. The number of bytes allocated and number of calls at each
> -location are reported.
> +location are reported. The first line indicates the version of the file, the
> +second line is the header listing fields in the file.
>
>  Example output.
>
>  ::
>
> -    > sort -rn /proc/allocinfo
> +    > tail -n +3 /proc/allocinfo | sort -rn
>     127664128    31168 mm/page_ext.c:270 func:alloc_page_ext
>      56373248     4737 mm/slub.c:2259 func:alloc_slab_page
>      14880768     3633 mm/readahead.c:247 func:page_cache_ra_unbounded
> diff --git a/lib/alloc_tag.c b/lib/alloc_tag.c
> index 531dbe2f5456..cbe93939332d 100644
> --- a/lib/alloc_tag.c
> +++ b/lib/alloc_tag.c
> @@ -16,47 +16,61 @@ EXPORT_SYMBOL(_shared_alloc_tag);
>  DEFINE_STATIC_KEY_MAYBE(CONFIG_MEM_ALLOC_PROFILING_ENABLED_BY_DEFAULT,
>                         mem_alloc_profiling_key);
>
> +struct allocinfo_private {
> +       struct codetag_iterator iter;
> +       bool print_header;
> +
> +};
> +
>  static void *allocinfo_start(struct seq_file *m, loff_t *pos)
>  {
> -       struct codetag_iterator *iter;
> +       struct allocinfo_private *priv;
>         struct codetag *ct;
>         loff_t node = *pos;
>
> -       iter = kzalloc(sizeof(*iter), GFP_KERNEL);
> -       m->private = iter;
> -       if (!iter)
> +       priv = kzalloc(sizeof(*priv), GFP_KERNEL);
> +       m->private = priv;
> +       if (!priv)
>                 return NULL;
>
> +       priv->print_header = (node == 0);
>         codetag_lock_module_list(alloc_tag_cttype, true);
> -       *iter = codetag_get_ct_iter(alloc_tag_cttype);
> -       while ((ct = codetag_next_ct(iter)) != NULL && node)
> +       priv->iter = codetag_get_ct_iter(alloc_tag_cttype);
> +       while ((ct = codetag_next_ct(&priv->iter)) != NULL && node)
>                 node--;
>
> -       return ct ? iter : NULL;
> +       return ct ? priv : NULL;
>  }
>
>  static void *allocinfo_next(struct seq_file *m, void *arg, loff_t *pos)
>  {
> -       struct codetag_iterator *iter = (struct codetag_iterator *)arg;
> -       struct codetag *ct = codetag_next_ct(iter);
> +       struct allocinfo_private *priv = (struct allocinfo_private *)arg;
> +       struct codetag *ct = codetag_next_ct(&priv->iter);
>
>         (*pos)++;
>         if (!ct)
>                 return NULL;
>
> -       return iter;
> +       return priv;
>  }
>
>  static void allocinfo_stop(struct seq_file *m, void *arg)
>  {
> -       struct codetag_iterator *iter = (struct codetag_iterator *)m->private;
> +       struct allocinfo_private *priv = (struct allocinfo_private *)m->private;
>
> -       if (iter) {
> +       if (priv) {
>                 codetag_lock_module_list(alloc_tag_cttype, false);
> -               kfree(iter);
> +               kfree(priv);
>         }
>  }
>
> +static void print_allocinfo_header(struct seq_buf *buf)
> +{
> +       /* Output format version, so we can change it. */
> +       seq_buf_printf(buf, "allocinfo - version: 1.0\n");
> +       seq_buf_printf(buf, "#     <size>  <calls> <tag info>\n");
> +}
> +
>  static void alloc_tag_to_text(struct seq_buf *out, struct codetag *ct)
>  {
>         struct alloc_tag *tag = ct_to_alloc_tag(ct);
> @@ -71,13 +85,17 @@ static void alloc_tag_to_text(struct seq_buf *out, struct codetag *ct)
>
>  static int allocinfo_show(struct seq_file *m, void *arg)
>  {
> -       struct codetag_iterator *iter = (struct codetag_iterator *)arg;
> +       struct allocinfo_private *priv = (struct allocinfo_private *)arg;
>         char *bufp;
>         size_t n = seq_get_buf(m, &bufp);
>         struct seq_buf buf;
>
>         seq_buf_init(&buf, bufp, n);
> -       alloc_tag_to_text(&buf, iter->ct);
> +       if (priv->print_header) {
> +               print_allocinfo_header(&buf);
> +               priv->print_header = false;
> +       }
> +       alloc_tag_to_text(&buf, priv->iter.ct);
>         seq_commit(m, seq_buf_used(&buf));
>         return 0;
>  }
>
> base-commit: 7e8aafe0636cdcc5c9699ced05ff1f8ffcb937e2
> --
> 2.45.0.rc1.225.g2a3ae87e7f-goog
>
Kees Cook May 14, 2024, 4:58 p.m. UTC | #2
On Tue, May 14, 2024 at 09:31:28AM -0700, Suren Baghdasaryan wrote:
> Add version string and a header at the beginning of /proc/allocinfo to
> allow later format changes. Example output:
> 
> > head /proc/allocinfo
> allocinfo - version: 1.0
> #     <size>  <calls> <tag info>
>            0        0 init/main.c:1314 func:do_initcalls
>            0        0 init/do_mounts.c:353 func:mount_nodev_root
>            0        0 init/do_mounts.c:187 func:mount_root_generic
>            0        0 init/do_mounts.c:158 func:do_mount_root
>            0        0 init/initramfs.c:493 func:unpack_to_rootfs
>            0        0 init/initramfs.c:492 func:unpack_to_rootfs
>            0        0 init/initramfs.c:491 func:unpack_to_rootfs
>          512        1 arch/x86/events/rapl.c:681 func:init_rapl_pmus
>          128        1 arch/x86/events/rapl.c:571 func:rapl_cpu_online
> 
> Signed-off-by: Suren Baghdasaryan <surenb@google.com>

Ah yeah, good idea. (Do we have versioning like this anywhere else in
our /proc files? It seems a nice thing to add...)

Reviewed-by: Kees Cook <keescook@chromium.org>
Suren Baghdasaryan May 14, 2024, 8:19 p.m. UTC | #3
On Tue, May 14, 2024 at 9:58 AM Kees Cook <keescook@chromium.org> wrote:
>
> On Tue, May 14, 2024 at 09:31:28AM -0700, Suren Baghdasaryan wrote:
> > Add version string and a header at the beginning of /proc/allocinfo to
> > allow later format changes. Example output:
> >
> > > head /proc/allocinfo
> > allocinfo - version: 1.0
> > #     <size>  <calls> <tag info>
> >            0        0 init/main.c:1314 func:do_initcalls
> >            0        0 init/do_mounts.c:353 func:mount_nodev_root
> >            0        0 init/do_mounts.c:187 func:mount_root_generic
> >            0        0 init/do_mounts.c:158 func:do_mount_root
> >            0        0 init/initramfs.c:493 func:unpack_to_rootfs
> >            0        0 init/initramfs.c:492 func:unpack_to_rootfs
> >            0        0 init/initramfs.c:491 func:unpack_to_rootfs
> >          512        1 arch/x86/events/rapl.c:681 func:init_rapl_pmus
> >          128        1 arch/x86/events/rapl.c:571 func:rapl_cpu_online
> >
> > Signed-off-by: Suren Baghdasaryan <surenb@google.com>
>
> Ah yeah, good idea. (Do we have versioning like this anywhere else in
> our /proc files? It seems a nice thing to add...)

Yes, /proc/slabinfo has a similar header that includes a version number.

>
> Reviewed-by: Kees Cook <keescook@chromium.org>

Thanks!

>
> --
> Kees Cook
diff mbox series

Patch

diff --git a/Documentation/filesystems/proc.rst b/Documentation/filesystems/proc.rst
index 245269dd6e02..4b71b3903d46 100644
--- a/Documentation/filesystems/proc.rst
+++ b/Documentation/filesystems/proc.rst
@@ -961,13 +961,14 @@  Provides information about memory allocations at all locations in the code
 base. Each allocation in the code is identified by its source file, line
 number, module (if originates from a loadable module) and the function calling
 the allocation. The number of bytes allocated and number of calls at each
-location are reported.
+location are reported. The first line indicates the version of the file, the
+second line is the header listing fields in the file.
 
 Example output.
 
 ::
 
-    > sort -rn /proc/allocinfo
+    > tail -n +3 /proc/allocinfo | sort -rn
    127664128    31168 mm/page_ext.c:270 func:alloc_page_ext
     56373248     4737 mm/slub.c:2259 func:alloc_slab_page
     14880768     3633 mm/readahead.c:247 func:page_cache_ra_unbounded
diff --git a/lib/alloc_tag.c b/lib/alloc_tag.c
index 531dbe2f5456..cbe93939332d 100644
--- a/lib/alloc_tag.c
+++ b/lib/alloc_tag.c
@@ -16,47 +16,61 @@  EXPORT_SYMBOL(_shared_alloc_tag);
 DEFINE_STATIC_KEY_MAYBE(CONFIG_MEM_ALLOC_PROFILING_ENABLED_BY_DEFAULT,
 			mem_alloc_profiling_key);
 
+struct allocinfo_private {
+	struct codetag_iterator iter;
+	bool print_header;
+
+};
+
 static void *allocinfo_start(struct seq_file *m, loff_t *pos)
 {
-	struct codetag_iterator *iter;
+	struct allocinfo_private *priv;
 	struct codetag *ct;
 	loff_t node = *pos;
 
-	iter = kzalloc(sizeof(*iter), GFP_KERNEL);
-	m->private = iter;
-	if (!iter)
+	priv = kzalloc(sizeof(*priv), GFP_KERNEL);
+	m->private = priv;
+	if (!priv)
 		return NULL;
 
+	priv->print_header = (node == 0);
 	codetag_lock_module_list(alloc_tag_cttype, true);
-	*iter = codetag_get_ct_iter(alloc_tag_cttype);
-	while ((ct = codetag_next_ct(iter)) != NULL && node)
+	priv->iter = codetag_get_ct_iter(alloc_tag_cttype);
+	while ((ct = codetag_next_ct(&priv->iter)) != NULL && node)
 		node--;
 
-	return ct ? iter : NULL;
+	return ct ? priv : NULL;
 }
 
 static void *allocinfo_next(struct seq_file *m, void *arg, loff_t *pos)
 {
-	struct codetag_iterator *iter = (struct codetag_iterator *)arg;
-	struct codetag *ct = codetag_next_ct(iter);
+	struct allocinfo_private *priv = (struct allocinfo_private *)arg;
+	struct codetag *ct = codetag_next_ct(&priv->iter);
 
 	(*pos)++;
 	if (!ct)
 		return NULL;
 
-	return iter;
+	return priv;
 }
 
 static void allocinfo_stop(struct seq_file *m, void *arg)
 {
-	struct codetag_iterator *iter = (struct codetag_iterator *)m->private;
+	struct allocinfo_private *priv = (struct allocinfo_private *)m->private;
 
-	if (iter) {
+	if (priv) {
 		codetag_lock_module_list(alloc_tag_cttype, false);
-		kfree(iter);
+		kfree(priv);
 	}
 }
 
+static void print_allocinfo_header(struct seq_buf *buf)
+{
+	/* Output format version, so we can change it. */
+	seq_buf_printf(buf, "allocinfo - version: 1.0\n");
+	seq_buf_printf(buf, "#     <size>  <calls> <tag info>\n");
+}
+
 static void alloc_tag_to_text(struct seq_buf *out, struct codetag *ct)
 {
 	struct alloc_tag *tag = ct_to_alloc_tag(ct);
@@ -71,13 +85,17 @@  static void alloc_tag_to_text(struct seq_buf *out, struct codetag *ct)
 
 static int allocinfo_show(struct seq_file *m, void *arg)
 {
-	struct codetag_iterator *iter = (struct codetag_iterator *)arg;
+	struct allocinfo_private *priv = (struct allocinfo_private *)arg;
 	char *bufp;
 	size_t n = seq_get_buf(m, &bufp);
 	struct seq_buf buf;
 
 	seq_buf_init(&buf, bufp, n);
-	alloc_tag_to_text(&buf, iter->ct);
+	if (priv->print_header) {
+		print_allocinfo_header(&buf);
+		priv->print_header = false;
+	}
+	alloc_tag_to_text(&buf, priv->iter.ct);
 	seq_commit(m, seq_buf_used(&buf));
 	return 0;
 }