diff mbox series

of: reserved-mem: expose reserved-mem details via debugfs

Message ID 20230206142714.4151047-1-liumartin@google.com
State Changes Requested, archived
Headers show
Series of: reserved-mem: expose reserved-mem details via debugfs | expand

Checks

Context Check Description
robh/checkpatch warning total: 0 errors, 1 warnings, 48 lines checked
robh/patch-applied fail build log

Commit Message

Martin Liu Feb. 6, 2023, 2:27 p.m. UTC
It's important to know reserved-mem information in mobile world
since reserved memory via device tree keeps increased in platform
(e.g., 45% in our platform). Therefore, it's crucial to know the
reserved memory sizes breakdown for the memory accounting.

This patch shows the reserved memory breakdown under debugfs to
make them visible.

Below is an example output:
cat $debugfs/reserved_mem/show
0x00000009fc400000..0x00000009ffffffff (   61440 KB )   map     reusable test1
0x00000009f9000000..0x00000009fc3fffff (   53248 KB )   map     reusable test2
0x00000000ffdf0000..0x00000000ffffffff (    2112 KB )   map non-reusable test3
0x00000009f6000000..0x00000009f8ffffff (   49152 KB )   map     reusable test4
...
0x00000000fd902000..0x00000000fd909fff (      32 KB ) nomap non-reusable test38
0x00000000fd90a000..0x00000000fd90bfff (       8 KB ) nomap non-reusable test39
Total 39 regions, 1446140 KB

Signed-off-by: Martin Liu <liumartin@google.com>
---
 drivers/of/of_reserved_mem.c | 39 ++++++++++++++++++++++++++++++++++++
 1 file changed, 39 insertions(+)

Comments

Rob Herring Feb. 6, 2023, 3:12 p.m. UTC | #1
On Mon, Feb 6, 2023 at 8:27 AM Martin Liu <liumartin@google.com> wrote:
>
> It's important to know reserved-mem information in mobile world
> since reserved memory via device tree keeps increased in platform
> (e.g., 45% in our platform). Therefore, it's crucial to know the
> reserved memory sizes breakdown for the memory accounting.
>
> This patch shows the reserved memory breakdown under debugfs to
> make them visible.
>
> Below is an example output:
> cat $debugfs/reserved_mem/show
> 0x00000009fc400000..0x00000009ffffffff (   61440 KB )   map     reusable test1
> 0x00000009f9000000..0x00000009fc3fffff (   53248 KB )   map     reusable test2
> 0x00000000ffdf0000..0x00000000ffffffff (    2112 KB )   map non-reusable test3
> 0x00000009f6000000..0x00000009f8ffffff (   49152 KB )   map     reusable test4
> ...
> 0x00000000fd902000..0x00000000fd909fff (      32 KB ) nomap non-reusable test38
> 0x00000000fd90a000..0x00000000fd90bfff (       8 KB ) nomap non-reusable test39
> Total 39 regions, 1446140 KB

This information is pretty much static, why not just print it during
boot? It's also just spitting out information that's straight from the
DT which is also available to userspace (flattened and unflattened).

Is there not something in memblock that provides the same info in a
firmware agnostic way?


> Signed-off-by: Martin Liu <liumartin@google.com>
> ---
>  drivers/of/of_reserved_mem.c | 39 ++++++++++++++++++++++++++++++++++++
>  1 file changed, 39 insertions(+)
>
> diff --git a/drivers/of/of_reserved_mem.c b/drivers/of/of_reserved_mem.c
> index 65f3b02a0e4e..a73228e07c8c 100644
> --- a/drivers/of/of_reserved_mem.c
> +++ b/drivers/of/of_reserved_mem.c
> @@ -23,6 +23,7 @@
>  #include <linux/memblock.h>
>  #include <linux/kmemleak.h>
>  #include <linux/cma.h>
> +#include <linux/debugfs.h>
>
>  #include "of_private.h"
>
> @@ -446,3 +447,41 @@ struct reserved_mem *of_reserved_mem_lookup(struct device_node *np)
>         return NULL;
>  }
>  EXPORT_SYMBOL_GPL(of_reserved_mem_lookup);
> +
> +#if defined(CONFIG_DEBUG_FS)
> +static int of_reserved_mem_debug_show(struct seq_file *m, void *private)
> +{
> +       unsigned int i;
> +       size_t sum = 0;
> +
> +       for (i = 0; i < reserved_mem_count; i++) {
> +               const struct reserved_mem *rmem = &reserved_mem[i];
> +               unsigned long node = rmem->fdt_node;
> +               phys_addr_t end = rmem->base + rmem->size - 1;
> +               bool nomap = (of_get_flat_dt_prop(node, "no-map", NULL)) != NULL;
> +               bool reusable = (of_get_flat_dt_prop(node, "reusable", NULL)) != NULL;

There is no reason to read the flat DT at this point in time after we
have an unflattened tree.

> +
> +               sum += rmem->size;
> +               seq_printf(m, "%pa..%pa ( %7lu KB ) %5s %12s %s\n", &rmem->base,
> +                          &end, rmem->size / 1024,
> +                          nomap ? "nomap" : "map",
> +                          reusable ? "reusable" : "non-reusable",
> +                          rmem->name ? rmem->name : "unknown");
> +       }
> +       seq_printf(m, "Total %d regions, %zu KB\n",
> +                  reserved_mem_count,
> +                  sum / 1024);
> +       return 0;
> +}
> +DEFINE_SHOW_ATTRIBUTE(of_reserved_mem_debug);
> +
> +static int __init of_reserved_mem_init_debugfs(void)
> +{
> +       struct dentry *root = debugfs_create_dir("reserved_mem", NULL);
> +
> +       debugfs_create_file("show", 0444, root,
> +                           NULL, &of_reserved_mem_debug_fops);
> +       return 0;
> +}
> +device_initcall(of_reserved_mem_init_debugfs);

We already have a DT init hook, don't add another random one. Plus,
why does this need to be an early device_initcall?

Rob
kernel test robot Feb. 6, 2023, 5:24 p.m. UTC | #2
Hi Martin,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on robh/for-next]
[also build test WARNING on linus/master v6.2-rc7 next-20230206]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Martin-Liu/of-reserved-mem-expose-reserved-mem-details-via-debugfs/20230206-222927
base:   https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git for-next
patch link:    https://lore.kernel.org/r/20230206142714.4151047-1-liumartin%40google.com
patch subject: [PATCH] of: reserved-mem: expose reserved-mem details via debugfs
config: ia64-allyesconfig (https://download.01.org/0day-ci/archive/20230207/202302070150.S0t7TNwD-lkp@intel.com/config)
compiler: ia64-linux-gcc (GCC) 12.1.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/intel-lab-lkp/linux/commit/5d479d32b3863f2ec8d10d756aa57cf29046e334
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Martin-Liu/of-reserved-mem-expose-reserved-mem-details-via-debugfs/20230206-222927
        git checkout 5d479d32b3863f2ec8d10d756aa57cf29046e334
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=ia64 olddefconfig
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=ia64 SHELL=/bin/bash drivers/

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

   drivers/of/of_reserved_mem.c: In function 'of_reserved_mem_debug_show':
>> drivers/of/of_reserved_mem.c:465:46: warning: format '%lu' expects argument of type 'long unsigned int', but argument 5 has type 'phys_addr_t' {aka 'long long unsigned int'} [-Wformat=]
     465 |                 seq_printf(m, "%pa..%pa ( %7lu KB ) %5s %12s %s\n", &rmem->base,
         |                                           ~~~^
         |                                              |
         |                                              long unsigned int
         |                                           %7llu
     466 |                            &end, rmem->size / 1024,
         |                                  ~~~~~~~~~~~~~~~~~
         |                                             |
         |                                             phys_addr_t {aka long long unsigned int}


vim +465 drivers/of/of_reserved_mem.c

   450	
   451	#if defined(CONFIG_DEBUG_FS)
   452	static int of_reserved_mem_debug_show(struct seq_file *m, void *private)
   453	{
   454		unsigned int i;
   455		size_t sum = 0;
   456	
   457		for (i = 0; i < reserved_mem_count; i++) {
   458			const struct reserved_mem *rmem = &reserved_mem[i];
   459			unsigned long node = rmem->fdt_node;
   460			phys_addr_t end = rmem->base + rmem->size - 1;
   461			bool nomap = (of_get_flat_dt_prop(node, "no-map", NULL)) != NULL;
   462			bool reusable = (of_get_flat_dt_prop(node, "reusable", NULL)) != NULL;
   463	
   464			sum += rmem->size;
 > 465			seq_printf(m, "%pa..%pa ( %7lu KB ) %5s %12s %s\n", &rmem->base,
   466				   &end, rmem->size / 1024,
   467				   nomap ? "nomap" : "map",
   468				   reusable ? "reusable" : "non-reusable",
   469				   rmem->name ? rmem->name : "unknown");
   470		}
   471		seq_printf(m, "Total %d regions, %zu KB\n",
   472			   reserved_mem_count,
   473			   sum / 1024);
   474		return 0;
   475	}
   476	DEFINE_SHOW_ATTRIBUTE(of_reserved_mem_debug);
   477
kernel test robot Feb. 6, 2023, 7:57 p.m. UTC | #3
Hi Martin,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on robh/for-next]
[also build test WARNING on linus/master v6.2-rc7 next-20230206]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Martin-Liu/of-reserved-mem-expose-reserved-mem-details-via-debugfs/20230206-222927
base:   https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git for-next
patch link:    https://lore.kernel.org/r/20230206142714.4151047-1-liumartin%40google.com
patch subject: [PATCH] of: reserved-mem: expose reserved-mem details via debugfs
config: m68k-allyesconfig (https://download.01.org/0day-ci/archive/20230207/202302070307.dJ4f1zXY-lkp@intel.com/config)
compiler: m68k-linux-gcc (GCC) 12.1.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/intel-lab-lkp/linux/commit/5d479d32b3863f2ec8d10d756aa57cf29046e334
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Martin-Liu/of-reserved-mem-expose-reserved-mem-details-via-debugfs/20230206-222927
        git checkout 5d479d32b3863f2ec8d10d756aa57cf29046e334
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=m68k olddefconfig
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=m68k SHELL=/bin/bash drivers/

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

   drivers/of/of_reserved_mem.c: In function 'of_reserved_mem_debug_show':
>> drivers/of/of_reserved_mem.c:465:46: warning: format '%lu' expects argument of type 'long unsigned int', but argument 5 has type 'phys_addr_t' {aka 'unsigned int'} [-Wformat=]
     465 |                 seq_printf(m, "%pa..%pa ( %7lu KB ) %5s %12s %s\n", &rmem->base,
         |                                           ~~~^
         |                                              |
         |                                              long unsigned int
         |                                           %7u
     466 |                            &end, rmem->size / 1024,
         |                                  ~~~~~~~~~~~~~~~~~
         |                                             |
         |                                             phys_addr_t {aka unsigned int}


vim +465 drivers/of/of_reserved_mem.c

   450	
   451	#if defined(CONFIG_DEBUG_FS)
   452	static int of_reserved_mem_debug_show(struct seq_file *m, void *private)
   453	{
   454		unsigned int i;
   455		size_t sum = 0;
   456	
   457		for (i = 0; i < reserved_mem_count; i++) {
   458			const struct reserved_mem *rmem = &reserved_mem[i];
   459			unsigned long node = rmem->fdt_node;
   460			phys_addr_t end = rmem->base + rmem->size - 1;
   461			bool nomap = (of_get_flat_dt_prop(node, "no-map", NULL)) != NULL;
   462			bool reusable = (of_get_flat_dt_prop(node, "reusable", NULL)) != NULL;
   463	
   464			sum += rmem->size;
 > 465			seq_printf(m, "%pa..%pa ( %7lu KB ) %5s %12s %s\n", &rmem->base,
   466				   &end, rmem->size / 1024,
   467				   nomap ? "nomap" : "map",
   468				   reusable ? "reusable" : "non-reusable",
   469				   rmem->name ? rmem->name : "unknown");
   470		}
   471		seq_printf(m, "Total %d regions, %zu KB\n",
   472			   reserved_mem_count,
   473			   sum / 1024);
   474		return 0;
   475	}
   476	DEFINE_SHOW_ATTRIBUTE(of_reserved_mem_debug);
   477
Martin Liu Feb. 7, 2023, 5:05 p.m. UTC | #4
On Mon, Feb 6, 2023 at 11:12 PM Rob Herring <robh+dt@kernel.org> wrote:
>
> On Mon, Feb 6, 2023 at 8:27 AM Martin Liu <liumartin@google.com> wrote:
> >
> > It's important to know reserved-mem information in mobile world
> > since reserved memory via device tree keeps increased in platform
> > (e.g., 45% in our platform). Therefore, it's crucial to know the
> > reserved memory sizes breakdown for the memory accounting.
> >
> > This patch shows the reserved memory breakdown under debugfs to
> > make them visible.
> >
> > Below is an example output:
> > cat $debugfs/reserved_mem/show
> > 0x00000009fc400000..0x00000009ffffffff (   61440 KB )   map     reusable test1
> > 0x00000009f9000000..0x00000009fc3fffff (   53248 KB )   map     reusable test2
> > 0x00000000ffdf0000..0x00000000ffffffff (    2112 KB )   map non-reusable test3
> > 0x00000009f6000000..0x00000009f8ffffff (   49152 KB )   map     reusable test4
> > ...
> > 0x00000000fd902000..0x00000000fd909fff (      32 KB ) nomap non-reusable test38
> > 0x00000000fd90a000..0x00000000fd90bfff (       8 KB ) nomap non-reusable test39
> > Total 39 regions, 1446140 KB
>
> This information is pretty much static, why not just print it during
> boot? It's also just spitting out information that's straight from the
> DT which is also available to userspace (flattened and unflattened).

IIUC, for dynamic allocation cases, we can't get actual allocation layout
from DT.  Also, there could be some adjustment from memblock
(ex. alignment). Therefore, printing it out from the reserved_mem would
be more clear.

However, as you mentioned, once the allocation is done, it should be pretty
static. Thus, printing it during boot should be reasonable. If so, we
could print
them out in fdt_init_reserved_mem() like below. Is my understanding correct?
Thanks :)

@@ -285,6 +285,14 @@ void __init fdt_init_reserved_mem(void)
                                else
                                        memblock_phys_free(rmem->base,
                                                           rmem->size);
+                       } else {
+                               phys_addr_t end = rmem->base + rmem->size - 1;
+                               bool reusable =
(of_get_flat_dt_prop(node, "reusable", NULL)) != NULL;
+                               pr_debug("init reserved node: %pa..%pa
( %lu KB) %s %s %s\n",
+                                        &rmem->base, &end, (unsigned
long)(rmem->size / SZ_1K),
+                                        nomap ? "nomap" : "map",
+                                        reusable ? "reusable" : "non-reusable",
+                                        rmem->name ? rmem->name : "unknown");
                        }
                }
        }
>
> Is there not something in memblock that provides the same info in a
> firmware agnostic way?

memblock doesn't save request's name so we couldn't count for the
memory owner.
>
>
> > Signed-off-by: Martin Liu <liumartin@google.com>
> > ---
> >  drivers/of/of_reserved_mem.c | 39 ++++++++++++++++++++++++++++++++++++
> >  1 file changed, 39 insertions(+)
> >
> > diff --git a/drivers/of/of_reserved_mem.c b/drivers/of/of_reserved_mem.c
> > index 65f3b02a0e4e..a73228e07c8c 100644
> > --- a/drivers/of/of_reserved_mem.c
> > +++ b/drivers/of/of_reserved_mem.c
> > @@ -23,6 +23,7 @@
> >  #include <linux/memblock.h>
> >  #include <linux/kmemleak.h>
> >  #include <linux/cma.h>
> > +#include <linux/debugfs.h>
> >
> >  #include "of_private.h"
> >
> > @@ -446,3 +447,41 @@ struct reserved_mem *of_reserved_mem_lookup(struct device_node *np)
> >         return NULL;
> >  }
> >  EXPORT_SYMBOL_GPL(of_reserved_mem_lookup);
> > +
> > +#if defined(CONFIG_DEBUG_FS)
> > +static int of_reserved_mem_debug_show(struct seq_file *m, void *private)
> > +{
> > +       unsigned int i;
> > +       size_t sum = 0;
> > +
> > +       for (i = 0; i < reserved_mem_count; i++) {
> > +               const struct reserved_mem *rmem = &reserved_mem[i];
> > +               unsigned long node = rmem->fdt_node;
> > +               phys_addr_t end = rmem->base + rmem->size - 1;
> > +               bool nomap = (of_get_flat_dt_prop(node, "no-map", NULL)) != NULL;
> > +               bool reusable = (of_get_flat_dt_prop(node, "reusable", NULL)) != NULL;
>
> There is no reason to read the flat DT at this point in time after we
> have an unflattened tree.

Ack.
>
> > +
> > +               sum += rmem->size;
> > +               seq_printf(m, "%pa..%pa ( %7lu KB ) %5s %12s %s\n", &rmem->base,
> > +                          &end, rmem->size / 1024,
> > +                          nomap ? "nomap" : "map",
> > +                          reusable ? "reusable" : "non-reusable",
> > +                          rmem->name ? rmem->name : "unknown");
> > +       }
> > +       seq_printf(m, "Total %d regions, %zu KB\n",
> > +                  reserved_mem_count,
> > +                  sum / 1024);
> > +       return 0;
> > +}
> > +DEFINE_SHOW_ATTRIBUTE(of_reserved_mem_debug);
> > +
> > +static int __init of_reserved_mem_init_debugfs(void)
> > +{
> > +       struct dentry *root = debugfs_create_dir("reserved_mem", NULL);
> > +
> > +       debugfs_create_file("show", 0444, root,
> > +                           NULL, &of_reserved_mem_debug_fops);
> > +       return 0;
> > +}
> > +device_initcall(of_reserved_mem_init_debugfs);
>
> We already have a DT init hook, don't add another random one. Plus,
> why does this need to be an early device_initcall?

Got it. As we could print them during boot, we probably don't need this. Thanks.
>
> Rob
Rob Herring (Arm) Feb. 7, 2023, 9:04 p.m. UTC | #5
On Wed, Feb 08, 2023 at 01:05:25AM +0800, Martin Liu wrote:
> On Mon, Feb 6, 2023 at 11:12 PM Rob Herring <robh+dt@kernel.org> wrote:
> >
> > On Mon, Feb 6, 2023 at 8:27 AM Martin Liu <liumartin@google.com> wrote:
> > >
> > > It's important to know reserved-mem information in mobile world
> > > since reserved memory via device tree keeps increased in platform
> > > (e.g., 45% in our platform). Therefore, it's crucial to know the
> > > reserved memory sizes breakdown for the memory accounting.
> > >
> > > This patch shows the reserved memory breakdown under debugfs to
> > > make them visible.
> > >
> > > Below is an example output:
> > > cat $debugfs/reserved_mem/show
> > > 0x00000009fc400000..0x00000009ffffffff (   61440 KB )   map     reusable test1
> > > 0x00000009f9000000..0x00000009fc3fffff (   53248 KB )   map     reusable test2
> > > 0x00000000ffdf0000..0x00000000ffffffff (    2112 KB )   map non-reusable test3
> > > 0x00000009f6000000..0x00000009f8ffffff (   49152 KB )   map     reusable test4
> > > ...
> > > 0x00000000fd902000..0x00000000fd909fff (      32 KB ) nomap non-reusable test38
> > > 0x00000000fd90a000..0x00000000fd90bfff (       8 KB ) nomap non-reusable test39
> > > Total 39 regions, 1446140 KB
> >
> > This information is pretty much static, why not just print it during
> > boot? It's also just spitting out information that's straight from the
> > DT which is also available to userspace (flattened and unflattened).
> 
> IIUC, for dynamic allocation cases, we can't get actual allocation layout
> from DT.

Right, so whomever does the allocation should print that out.

>  Also, there could be some adjustment from memblock
> (ex. alignment). Therefore, printing it out from the reserved_mem would
> be more clear.

If memblock is adjusting, then shouldn't memblock print out the 
addresses?

> However, as you mentioned, once the allocation is done, it should be pretty
> static. Thus, printing it during boot should be reasonable. If so, we
> could print
> them out in fdt_init_reserved_mem() like below. Is my understanding correct?

That looks mostly fine to me. If we can do it with the unflattened tree, 
that would be better. I'm not sure off hand if that works here and you 
are just incorrectly using of_get_flat_dt_prop() still, or if it is 
indeed too early.

Rob
Martin Liu Feb. 8, 2023, 8:59 a.m. UTC | #6
On Wed, Feb 8, 2023 at 5:05 AM Rob Herring <robh@kernel.org> wrote:
>
> On Wed, Feb 08, 2023 at 01:05:25AM +0800, Martin Liu wrote:
> > On Mon, Feb 6, 2023 at 11:12 PM Rob Herring <robh+dt@kernel.org> wrote:
> > >
> > > On Mon, Feb 6, 2023 at 8:27 AM Martin Liu <liumartin@google.com> wrote:
> > > >
> > > > It's important to know reserved-mem information in mobile world
> > > > since reserved memory via device tree keeps increased in platform
> > > > (e.g., 45% in our platform). Therefore, it's crucial to know the
> > > > reserved memory sizes breakdown for the memory accounting.
> > > >
> > > > This patch shows the reserved memory breakdown under debugfs to
> > > > make them visible.
> > > >
> > > > Below is an example output:
> > > > cat $debugfs/reserved_mem/show
> > > > 0x00000009fc400000..0x00000009ffffffff (   61440 KB )   map     reusable test1
> > > > 0x00000009f9000000..0x00000009fc3fffff (   53248 KB )   map     reusable test2
> > > > 0x00000000ffdf0000..0x00000000ffffffff (    2112 KB )   map non-reusable test3
> > > > 0x00000009f6000000..0x00000009f8ffffff (   49152 KB )   map     reusable test4
> > > > ...
> > > > 0x00000000fd902000..0x00000000fd909fff (      32 KB ) nomap non-reusable test38
> > > > 0x00000000fd90a000..0x00000000fd90bfff (       8 KB ) nomap non-reusable test39
> > > > Total 39 regions, 1446140 KB
> > >
> > > This information is pretty much static, why not just print it during
> > > boot? It's also just spitting out information that's straight from the
> > > DT which is also available to userspace (flattened and unflattened).
> >
> > IIUC, for dynamic allocation cases, we can't get actual allocation layout
> > from DT.
>
> Right, so whomever does the allocation should print that out.
>
> >  Also, there could be some adjustment from memblock
> > (ex. alignment). Therefore, printing it out from the reserved_mem would
> > be more clear.
>
> If memblock is adjusting, then shouldn't memblock print out the
> addresses?

Yup, memblock can print out the address with _RET_IP_when the debug is on.
In our case, the _RET_IP_ will be early_init_dt_reserve_memory_arch so it's not
useful to find the memory owner. Thus, printing it out in of_reserved_mem
module will be more clear and simple.
>
> > However, as you mentioned, once the allocation is done, it should be pretty
> > static. Thus, printing it during boot should be reasonable. If so, we
> > could print
> > them out in fdt_init_reserved_mem() like below. Is my understanding correct?
>
> That looks mostly fine to me. If we can do it with the unflattened tree,
> that would be better. I'm not sure off hand if that works here and you
> are just incorrectly using of_get_flat_dt_prop() still, or if it is
> indeed too early.

Yup, it's too early when fdt_init_reserved_mem() is called. I thought
that is also
why nomap and prop values are using of_get_flat_dt_prop() to get the property
content in fdt_init_reserved_mem() as well.

Another thing is we should use pr_info() to print them out as they are not the
debug information. They're the memory layout we really want to know during
the boot.

Please let me know if there are any concerns. If not, I'll send a V2
patch. Thanks.

Martin
>
> Rob
diff mbox series

Patch

diff --git a/drivers/of/of_reserved_mem.c b/drivers/of/of_reserved_mem.c
index 65f3b02a0e4e..a73228e07c8c 100644
--- a/drivers/of/of_reserved_mem.c
+++ b/drivers/of/of_reserved_mem.c
@@ -23,6 +23,7 @@ 
 #include <linux/memblock.h>
 #include <linux/kmemleak.h>
 #include <linux/cma.h>
+#include <linux/debugfs.h>
 
 #include "of_private.h"
 
@@ -446,3 +447,41 @@  struct reserved_mem *of_reserved_mem_lookup(struct device_node *np)
 	return NULL;
 }
 EXPORT_SYMBOL_GPL(of_reserved_mem_lookup);
+
+#if defined(CONFIG_DEBUG_FS)
+static int of_reserved_mem_debug_show(struct seq_file *m, void *private)
+{
+	unsigned int i;
+	size_t sum = 0;
+
+	for (i = 0; i < reserved_mem_count; i++) {
+		const struct reserved_mem *rmem = &reserved_mem[i];
+		unsigned long node = rmem->fdt_node;
+		phys_addr_t end = rmem->base + rmem->size - 1;
+		bool nomap = (of_get_flat_dt_prop(node, "no-map", NULL)) != NULL;
+		bool reusable = (of_get_flat_dt_prop(node, "reusable", NULL)) != NULL;
+
+		sum += rmem->size;
+		seq_printf(m, "%pa..%pa ( %7lu KB ) %5s %12s %s\n", &rmem->base,
+			   &end, rmem->size / 1024,
+			   nomap ? "nomap" : "map",
+			   reusable ? "reusable" : "non-reusable",
+			   rmem->name ? rmem->name : "unknown");
+	}
+	seq_printf(m, "Total %d regions, %zu KB\n",
+		   reserved_mem_count,
+		   sum / 1024);
+	return 0;
+}
+DEFINE_SHOW_ATTRIBUTE(of_reserved_mem_debug);
+
+static int __init of_reserved_mem_init_debugfs(void)
+{
+	struct dentry *root = debugfs_create_dir("reserved_mem", NULL);
+
+	debugfs_create_file("show", 0444, root,
+			    NULL, &of_reserved_mem_debug_fops);
+	return 0;
+}
+device_initcall(of_reserved_mem_init_debugfs);
+#endif