| Message ID | 20210611131153.3731147-2-aisheng.dong@nxp.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/2] of: of_reserved_mem: only call memblock_free for normal reserved memory | expand |
On Fri, Jun 11, 2021 at 7:13 AM Dong Aisheng <aisheng.dong@nxp.com> wrote: > > Since commit 86588296acbf ("fdt: Properly handle "no-map" field in the memory region"), > nomap memory is changed to call memblock_mark_nomap() instead of > memblock_remove(). But it only changed the reserved memory with fixed > addr and size case in early_init_dt_reserve_memory_arch(), not > including the dynamical allocation by size case in > early_init_dt_alloc_reserved_memory_arch(). > > Cc: Rob Herring <robh+dt@kernel.org> > Cc: devicetree@vger.kernel.org Good practice is to Cc the people involved in referenced commits. Adding them now. This code is a minefield so I'd like other eyes on it. > Signed-off-by: Dong Aisheng <aisheng.dong@nxp.com> > --- > drivers/of/of_reserved_mem.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/of/of_reserved_mem.c b/drivers/of/of_reserved_mem.c > index 367f298a83b2..ebba88395bf8 100644 > --- a/drivers/of/of_reserved_mem.c > +++ b/drivers/of/of_reserved_mem.c > @@ -42,7 +42,7 @@ static int __init early_init_dt_alloc_reserved_memory_arch(phys_addr_t size, > > *res_base = base; > if (nomap) > - return memblock_remove(base, size); > + return memblock_mark_nomap(base, size); > > return memblock_reserve(base, size); > } > @@ -276,7 +276,7 @@ void __init fdt_init_reserved_mem(void) > pr_info("node %s compatible matching fail\n", > rmem->name); > if (nomap) > - memblock_add(rmem->base, rmem->size); > + memblock_clear_nomap(rmem->base, rmem->size); > else > memblock_free(rmem->base, rmem->size); > } > -- > 2.25.1 >
> From: Rob Herring <robh+dt@kernel.org> > Sent: Saturday, June 12, 2021 1:11 AM > > On Fri, Jun 11, 2021 at 7:13 AM Dong Aisheng <aisheng.dong@nxp.com> > wrote: > > > > Since commit 86588296acbf ("fdt: Properly handle "no-map" field in the > > memory region"), nomap memory is changed to call > memblock_mark_nomap() > > instead of memblock_remove(). But it only changed the reserved memory > > with fixed addr and size case in early_init_dt_reserve_memory_arch(), > > not including the dynamical allocation by size case in > > early_init_dt_alloc_reserved_memory_arch(). > > > > Cc: Rob Herring <robh+dt@kernel.org> > > Cc: devicetree@vger.kernel.org > > Good practice is to Cc the people involved in referenced commits. > Adding them now. This code is a minefield so I'd like other eyes on it. > Yes, you're right. Thanks for the good suggestion. Let's wait for their comments. Regards Aisheng > > Signed-off-by: Dong Aisheng <aisheng.dong@nxp.com> > > --- > > drivers/of/of_reserved_mem.c | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/of/of_reserved_mem.c > > b/drivers/of/of_reserved_mem.c index 367f298a83b2..ebba88395bf8 > 100644 > > --- a/drivers/of/of_reserved_mem.c > > +++ b/drivers/of/of_reserved_mem.c > > @@ -42,7 +42,7 @@ static int __init > > early_init_dt_alloc_reserved_memory_arch(phys_addr_t size, > > > > *res_base = base; > > if (nomap) > > - return memblock_remove(base, size); > > + return memblock_mark_nomap(base, size); > > > > return memblock_reserve(base, size); } @@ -276,7 +276,7 @@ > > void __init fdt_init_reserved_mem(void) > > pr_info("node %s compatible matching > fail\n", > > rmem->name); > > if (nomap) > > - memblock_add(rmem->base, > rmem->size); > > + > > + memblock_clear_nomap(rmem->base, rmem->size); > > else > > memblock_free(rmem->base, > rmem->size); > > } > > -- > > 2.25.1 > >
On Friday 11 Jun 2021 at 11:10:36 (-0600), Rob Herring wrote: > On Fri, Jun 11, 2021 at 7:13 AM Dong Aisheng <aisheng.dong@nxp.com> wrote: > > > > Since commit 86588296acbf ("fdt: Properly handle "no-map" field in the memory region"), > > nomap memory is changed to call memblock_mark_nomap() instead of > > memblock_remove(). But it only changed the reserved memory with fixed > > addr and size case in early_init_dt_reserve_memory_arch(), not > > including the dynamical allocation by size case in > > early_init_dt_alloc_reserved_memory_arch(). > > > > Cc: Rob Herring <robh+dt@kernel.org> > > Cc: devicetree@vger.kernel.org > > Good practice is to Cc the people involved in referenced commits. > Adding them now. This code is a minefield so I'd like other eyes on > it. Apologies for the delayed reply -- was away last week. I've been starring at this for 15 minutes, and still can't see how it could go wrong, so FWIW: Reviewed-by: Quentin Perret <qperret@google.com> Thanks, Quentin > > Signed-off-by: Dong Aisheng <aisheng.dong@nxp.com> > > --- > > drivers/of/of_reserved_mem.c | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/of/of_reserved_mem.c b/drivers/of/of_reserved_mem.c > > index 367f298a83b2..ebba88395bf8 100644 > > --- a/drivers/of/of_reserved_mem.c > > +++ b/drivers/of/of_reserved_mem.c > > @@ -42,7 +42,7 @@ static int __init early_init_dt_alloc_reserved_memory_arch(phys_addr_t size, > > > > *res_base = base; > > if (nomap) > > - return memblock_remove(base, size); > > + return memblock_mark_nomap(base, size); > > > > return memblock_reserve(base, size); > > } > > @@ -276,7 +276,7 @@ void __init fdt_init_reserved_mem(void) > > pr_info("node %s compatible matching fail\n", > > rmem->name); > > if (nomap) > > - memblock_add(rmem->base, rmem->size); > > + memblock_clear_nomap(rmem->base, rmem->size); > > else > > memblock_free(rmem->base, rmem->size); > > } > > -- > > 2.25.1 > >
On Fri, 11 Jun 2021 21:11:53 +0800, Dong Aisheng wrote: > Since commit 86588296acbf ("fdt: Properly handle "no-map" field in the memory region"), > nomap memory is changed to call memblock_mark_nomap() instead of > memblock_remove(). But it only changed the reserved memory with fixed > addr and size case in early_init_dt_reserve_memory_arch(), not > including the dynamical allocation by size case in > early_init_dt_alloc_reserved_memory_arch(). > > Cc: Rob Herring <robh+dt@kernel.org> > Cc: devicetree@vger.kernel.org > Signed-off-by: Dong Aisheng <aisheng.dong@nxp.com> > --- > drivers/of/of_reserved_mem.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > Applied, thanks!
diff --git a/drivers/of/of_reserved_mem.c b/drivers/of/of_reserved_mem.c index 367f298a83b2..ebba88395bf8 100644 --- a/drivers/of/of_reserved_mem.c +++ b/drivers/of/of_reserved_mem.c @@ -42,7 +42,7 @@ static int __init early_init_dt_alloc_reserved_memory_arch(phys_addr_t size, *res_base = base; if (nomap) - return memblock_remove(base, size); + return memblock_mark_nomap(base, size); return memblock_reserve(base, size); } @@ -276,7 +276,7 @@ void __init fdt_init_reserved_mem(void) pr_info("node %s compatible matching fail\n", rmem->name); if (nomap) - memblock_add(rmem->base, rmem->size); + memblock_clear_nomap(rmem->base, rmem->size); else memblock_free(rmem->base, rmem->size); }
Since commit 86588296acbf ("fdt: Properly handle "no-map" field in the memory region"), nomap memory is changed to call memblock_mark_nomap() instead of memblock_remove(). But it only changed the reserved memory with fixed addr and size case in early_init_dt_reserve_memory_arch(), not including the dynamical allocation by size case in early_init_dt_alloc_reserved_memory_arch(). Cc: Rob Herring <robh+dt@kernel.org> Cc: devicetree@vger.kernel.org Signed-off-by: Dong Aisheng <aisheng.dong@nxp.com> --- drivers/of/of_reserved_mem.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)