From: Miles Chen <[email protected]>
When we use more than 32 entries in /resered-memory,
there will be an error message: "not enough space all defined regions.".
We can increase MAX_RESERVED_REGIONS to fix this.
commit 22f8cc6e3373 ("drivers: of: increase MAX_RESERVED_REGIONS to 32")
increased MAX_RESERVED_REGIONS to 32 but I'm not sure if increasing
MAX_RESERVED_REGIONS to 64 is suitable for everyone.
In this RFC patch, CONFIG_MAX_OF_RESERVED_REGIONS is added and used as
MAX_RESERVED_REGIONS. Add a sanity test to make sure that
MAX_RESERVED_REGIONS is less than INIT_MEMBLOCK_REGIONS.
Users can configure CONFIG_MAX_OF_RESERVED_REGIONS according to their
need.
Signed-off-by: Miles Chen <[email protected]>
---
drivers/of/Kconfig | 9 +++++++++
drivers/of/of_reserved_mem.c | 5 ++++-
2 files changed, 13 insertions(+), 1 deletion(-)
diff --git a/drivers/of/Kconfig b/drivers/of/Kconfig
index ad3fcad..63fff2e 100644
--- a/drivers/of/Kconfig
+++ b/drivers/of/Kconfig
@@ -86,6 +86,15 @@ config OF_RESERVED_MEM
help
Helpers to allow for reservation of memory regions
+config MAX_OF_RESERVED_REGIONS
+ int "the max number of devicetree reserved memory regions"
+ depends on OF_RESERVED_MEM
+ default 32
+ help
+ Setup the max number of devicetree reserved memory regions
+ If unsure, leave the default value "32".
+
+
config OF_RESOLVE
bool
diff --git a/drivers/of/of_reserved_mem.c b/drivers/of/of_reserved_mem.c
index 1977ee0..18a7729 100644
--- a/drivers/of/of_reserved_mem.c
+++ b/drivers/of/of_reserved_mem.c
@@ -22,7 +22,7 @@
#include <linux/slab.h>
#include <linux/memblock.h>
-#define MAX_RESERVED_REGIONS 32
+#define MAX_RESERVED_REGIONS CONFIG_MAX_OF_RESERVED_REGIONS
static struct reserved_mem reserved_mem[MAX_RESERVED_REGIONS];
static int reserved_mem_count;
@@ -246,6 +246,9 @@ void __init fdt_init_reserved_mem(void)
{
int i;
+ /* MAX_RESERVED_REGIONS should be bounded by INIT_MEMBLOCK_REGIONS */
+ BUILD_BUG_ON(MAX_RESERVED_REGIONS > INIT_MEMBLOCK_REGIONS);
+
/* check for overlapping reserved regions */
__rmem_check_for_overlap();
--
1.9.1
On Wed, Nov 21, 2018 at 2:11 AM <[email protected]> wrote:
>
> From: Miles Chen <[email protected]>
>
> When we use more than 32 entries in /resered-memory,
> there will be an error message: "not enough space all defined regions.".
> We can increase MAX_RESERVED_REGIONS to fix this.
>
> commit 22f8cc6e3373 ("drivers: of: increase MAX_RESERVED_REGIONS to 32")
> increased MAX_RESERVED_REGIONS to 32 but I'm not sure if increasing
> MAX_RESERVED_REGIONS to 64 is suitable for everyone.
>
> In this RFC patch, CONFIG_MAX_OF_RESERVED_REGIONS is added and used as
> MAX_RESERVED_REGIONS. Add a sanity test to make sure that
> MAX_RESERVED_REGIONS is less than INIT_MEMBLOCK_REGIONS.
> Users can configure CONFIG_MAX_OF_RESERVED_REGIONS according to their
> need.
I don't want a kconfig option for this. I think it should be dynamic instead.
The current flow is like this:
for each reserved node:
- call memblock_reserve
- Add info to reserved_mem array
I think we should change it to:
for each reserved node:
- call memblock_reserve
- count number of nodes
Alloc array using memblock_alloc
for each reserved node:
- Add info to reserved_mem array
Rob
On Wed, 2018-11-21 at 10:39 -0600, Rob Herring wrote:
> On Wed, Nov 21, 2018 at 2:11 AM <[email protected]> wrote:
> >
> > From: Miles Chen <[email protected]>
> >
> > When we use more than 32 entries in /resered-memory,
> > there will be an error message: "not enough space all defined regions.".
> > We can increase MAX_RESERVED_REGIONS to fix this.
> >
> > commit 22f8cc6e3373 ("drivers: of: increase MAX_RESERVED_REGIONS to 32")
> > increased MAX_RESERVED_REGIONS to 32 but I'm not sure if increasing
> > MAX_RESERVED_REGIONS to 64 is suitable for everyone.
> >
> > In this RFC patch, CONFIG_MAX_OF_RESERVED_REGIONS is added and used as
> > MAX_RESERVED_REGIONS. Add a sanity test to make sure that
> > MAX_RESERVED_REGIONS is less than INIT_MEMBLOCK_REGIONS.
> > Users can configure CONFIG_MAX_OF_RESERVED_REGIONS according to their
> > need.
>
> I don't want a kconfig option for this. I think it should be dynamic instead.
>
> The current flow is like this:
>
> for each reserved node:
> - call memblock_reserve
> - Add info to reserved_mem array
>
> I think we should change it to:
>
> for each reserved node:
> - call memblock_reserve
> - count number of nodes
>
> Alloc array using memblock_alloc
>
> for each reserved node:
> - Add info to reserved_mem array
>
thanks for your comment.
I reviewed the flow and it might be easier to count the
nodes and setup array first:
for each reserved node:
- count number of nodes
Alloc array using memblock_alloc
for each reserved node:
- call memblock_reserve
- Add info to reserved_mem array
What do you think?
> Rob
On Wed, Nov 21, 2018 at 8:51 PM Miles Chen <[email protected]> wrote:
>
> On Wed, 2018-11-21 at 10:39 -0600, Rob Herring wrote:
> > On Wed, Nov 21, 2018 at 2:11 AM <[email protected]> wrote:
> > >
> > > From: Miles Chen <[email protected]>
> > >
> > > When we use more than 32 entries in /resered-memory,
> > > there will be an error message: "not enough space all defined regions.".
> > > We can increase MAX_RESERVED_REGIONS to fix this.
> > >
> > > commit 22f8cc6e3373 ("drivers: of: increase MAX_RESERVED_REGIONS to 32")
> > > increased MAX_RESERVED_REGIONS to 32 but I'm not sure if increasing
> > > MAX_RESERVED_REGIONS to 64 is suitable for everyone.
> > >
> > > In this RFC patch, CONFIG_MAX_OF_RESERVED_REGIONS is added and used as
> > > MAX_RESERVED_REGIONS. Add a sanity test to make sure that
> > > MAX_RESERVED_REGIONS is less than INIT_MEMBLOCK_REGIONS.
> > > Users can configure CONFIG_MAX_OF_RESERVED_REGIONS according to their
> > > need.
> >
> > I don't want a kconfig option for this. I think it should be dynamic instead.
> >
> > The current flow is like this:
> >
> > for each reserved node:
> > - call memblock_reserve
> > - Add info to reserved_mem array
> >
> > I think we should change it to:
> >
> > for each reserved node:
> > - call memblock_reserve
> > - count number of nodes
> >
> > Alloc array using memblock_alloc
> >
> > for each reserved node:
> > - Add info to reserved_mem array
> >
>
> thanks for your comment.
>
> I reviewed the flow and it might be easier to count the
> nodes and setup array first:
>
> for each reserved node:
> - count number of nodes
>
> Alloc array using memblock_alloc
>
>
> for each reserved node:
> - call memblock_reserve
The order here is wrong. It is important that you reserve the memory
blocks before doing any allocations.
> - Add info to reserved_mem array
>
> What do you think?
>
> > Rob
>
>
On Sat, 2018-11-24 at 14:56 -0600, Rob Herring wrote:
> On Wed, Nov 21, 2018 at 8:51 PM Miles Chen <[email protected]> wrote:
> >
> > On Wed, 2018-11-21 at 10:39 -0600, Rob Herring wrote:
> > > On Wed, Nov 21, 2018 at 2:11 AM <[email protected]> wrote:
> > > >
> > > > From: Miles Chen <[email protected]>
> > > >
> > > > When we use more than 32 entries in /resered-memory,
> > > > there will be an error message: "not enough space all defined regions.".
> > > > We can increase MAX_RESERVED_REGIONS to fix this.
> > > >
> > > > commit 22f8cc6e3373 ("drivers: of: increase MAX_RESERVED_REGIONS to 32")
> > > > increased MAX_RESERVED_REGIONS to 32 but I'm not sure if increasing
> > > > MAX_RESERVED_REGIONS to 64 is suitable for everyone.
> > > >
> > > > In this RFC patch, CONFIG_MAX_OF_RESERVED_REGIONS is added and used as
> > > > MAX_RESERVED_REGIONS. Add a sanity test to make sure that
> > > > MAX_RESERVED_REGIONS is less than INIT_MEMBLOCK_REGIONS.
> > > > Users can configure CONFIG_MAX_OF_RESERVED_REGIONS according to their
> > > > need.
> > >
> > > I don't want a kconfig option for this. I think it should be dynamic instead.
> > >
> > > The current flow is like this:
> > >
> > > for each reserved node:
> > > - call memblock_reserve
> > > - Add info to reserved_mem array
> > >
> > > I think we should change it to:
> > >
> > > for each reserved node:
> > > - call memblock_reserve
> > > - count number of nodes
> > >
> > > Alloc array using memblock_alloc
> > >
> > > for each reserved node:
> > > - Add info to reserved_mem array
> > >
> >
> > thanks for your comment.
> >
> > I reviewed the flow and it might be easier to count the
> > nodes and setup array first:
> >
> > for each reserved node:
> > - count number of nodes
> >
> > Alloc array using memblock_alloc
> >
> >
> > for each reserved node:
> > - call memblock_reserve
>
> The order here is wrong. It is important that you reserve the memory
> blocks before doing any allocations.
thanks for pointing that out. You are right. I'll follow your
suggestion.
>
> > - Add info to reserved_mem array
> >
> > What do you think?
> >
> > > Rob
> >
> >
On Mon, 2018-11-26 at 09:33 +0800, Miles Chen wrote:
> On Sat, 2018-11-24 at 14:56 -0600, Rob Herring wrote:
> > On Wed, Nov 21, 2018 at 8:51 PM Miles Chen <[email protected]> wrote:
> > >
> > > On Wed, 2018-11-21 at 10:39 -0600, Rob Herring wrote:
> > > > On Wed, Nov 21, 2018 at 2:11 AM <[email protected]> wrote:
> > > > >
> > > > > From: Miles Chen <[email protected]>
> > > > >
> > > > > When we use more than 32 entries in /resered-memory,
> > > > > there will be an error message: "not enough space all defined regions.".
> > > > > We can increase MAX_RESERVED_REGIONS to fix this.
> > > > >
> > > > > commit 22f8cc6e3373 ("drivers: of: increase MAX_RESERVED_REGIONS to 32")
> > > > > increased MAX_RESERVED_REGIONS to 32 but I'm not sure if increasing
> > > > > MAX_RESERVED_REGIONS to 64 is suitable for everyone.
> > > > >
> > > > > In this RFC patch, CONFIG_MAX_OF_RESERVED_REGIONS is added and used as
> > > > > MAX_RESERVED_REGIONS. Add a sanity test to make sure that
> > > > > MAX_RESERVED_REGIONS is less than INIT_MEMBLOCK_REGIONS.
> > > > > Users can configure CONFIG_MAX_OF_RESERVED_REGIONS according to their
> > > > > need.
> > > >
> > > > I don't want a kconfig option for this. I think it should be dynamic instead.
> > > >
> > > > The current flow is like this:
> > > >
> > > > for each reserved node:
> > > > - call memblock_reserve
> > > > - Add info to reserved_mem array
> > > >
> > > > I think we should change it to:
> > > >
> > > > for each reserved node:
> > > > - call memblock_reserve
> > > > - count number of nodes
> > > >
> > > > Alloc array using memblock_alloc
> > > >
> > > > for each reserved node:
> > > > - Add info to reserved_mem array
> > > >
> > >
> > > thanks for your comment.
> > >
> > > I reviewed the flow and it might be easier to count the
> > > nodes and setup array first:
> > >
> > > for each reserved node:
> > > - count number of nodes
> > >
> > > Alloc array using memblock_alloc
> > >
> > >
> > > for each reserved node:
> > > - call memblock_reserve
> >
> > The order here is wrong. It is important that you reserve the memory
> > blocks before doing any allocations.
>
> thanks for pointing that out. You are right. I'll follow your
> suggestion.
I got a problem while implementing this change: the array allocation
is done successfully but the kernel does not create mappings for it yet,
so the array is not accessible. (kernel will create the mapping later in
paging_init())
For example: aarch64 setup_arch()
setup_arch()
{
memblock_init(); /* early_init_fdt_scan_reserved_mem() is called */
paging_init();
}
Could you give me some advice, please?
Miles
>
> >
> > > - Add info to reserved_mem array
> > >
> > > What do you think?
> > >
> > > > Rob
> > >
> > >
>
>