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 <[email protected]>
---
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;
+
+ 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
--
2.39.1.519.gcb327c4b5f-goog
On Mon, Feb 6, 2023 at 8:27 AM Martin Liu <[email protected]> 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 <[email protected]>
> ---
> 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
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/[email protected]/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 <[email protected]>
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
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests
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/[email protected]/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 <[email protected]>
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
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests
On Mon, Feb 6, 2023 at 11:12 PM Rob Herring <[email protected]> wrote:
>
> On Mon, Feb 6, 2023 at 8:27 AM Martin Liu <[email protected]> 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 <[email protected]>
> > ---
> > 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
On Wed, Feb 08, 2023 at 01:05:25AM +0800, Martin Liu wrote:
> On Mon, Feb 6, 2023 at 11:12 PM Rob Herring <[email protected]> wrote:
> >
> > On Mon, Feb 6, 2023 at 8:27 AM Martin Liu <[email protected]> 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
On Wed, Feb 8, 2023 at 5:05 AM Rob Herring <[email protected]> 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 <[email protected]> wrote:
> > >
> > > On Mon, Feb 6, 2023 at 8:27 AM Martin Liu <[email protected]> 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