| Message ID | 20201029162718.29910-4-david@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | powernv/memtrace: don't abuse memory hot(un)plug infrastructure for memory allocations | expand |
On Thu, Oct 29, 2020 at 05:27:17PM +0100, David Hildenbrand wrote: > Let's revert what we did in case seomthing goes wrong and we return an > error. Dumb question, but should not we do this for other arches as well?
On Wed, Nov 04, 2020 at 10:50:07AM +0100, osalvador wrote: > On Thu, Oct 29, 2020 at 05:27:17PM +0100, David Hildenbrand wrote: > > Let's revert what we did in case seomthing goes wrong and we return an > > error. > > Dumb question, but should not we do this for other arches as well? It seems arm64 and s390 already do that. x86 could have its arch_add_memory() improved though :) > -- > Oscar Salvador > SUSE L3
On Wed, Nov 04, 2020 at 02:06:51PM +0200, Mike Rapoport wrote: > On Wed, Nov 04, 2020 at 10:50:07AM +0100, osalvador wrote: > > On Thu, Oct 29, 2020 at 05:27:17PM +0100, David Hildenbrand wrote: > > > Let's revert what we did in case seomthing goes wrong and we return an > > > error. > > > > Dumb question, but should not we do this for other arches as well? > > It seems arm64 and s390 already do that. > x86 could have its arch_add_memory() improved though :) Right, I only stared at x86 and see it did not have it. I guess we want to have all arches aligned with this. Thanks
On Thu, Oct 29, 2020 at 05:27:17PM +0100, David Hildenbrand wrote: > Let's revert what we did in case seomthing goes wrong and we return an > error. > > Cc: Michael Ellerman <mpe@ellerman.id.au> > Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org> > Cc: Paul Mackerras <paulus@samba.org> > Cc: Rashmica Gupta <rashmica.g@gmail.com> > Cc: Andrew Morton <akpm@linux-foundation.org> > Cc: Mike Rapoport <rppt@kernel.org> > Cc: Michal Hocko <mhocko@suse.com> > Cc: Oscar Salvador <osalvador@suse.de> > Cc: Wei Yang <richard.weiyang@linux.alibaba.com> > Signed-off-by: David Hildenbrand <david@redhat.com> Reviewed-by: Oscar Salvador <osalvador@suse.de> > --- > arch/powerpc/mm/mem.c | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/arch/powerpc/mm/mem.c b/arch/powerpc/mm/mem.c > index 685028451dd2..69b3e8072261 100644 > --- a/arch/powerpc/mm/mem.c > +++ b/arch/powerpc/mm/mem.c > @@ -165,7 +165,10 @@ int __ref arch_add_memory(int nid, u64 start, u64 size, > rc = arch_create_linear_mapping(nid, start, size, params); > if (rc) > return rc; > - return __add_pages(nid, start_pfn, nr_pages, params); > + rc = __add_pages(nid, start_pfn, nr_pages, params); > + if (rc) > + arch_remove_linear_mapping(start, size); > + return rc; > } > > void __ref arch_remove_memory(int nid, u64 start, u64 size, > -- > 2.26.2 >
On 04.11.20 13:11, Oscar Salvador wrote: > On Wed, Nov 04, 2020 at 02:06:51PM +0200, Mike Rapoport wrote: >> On Wed, Nov 04, 2020 at 10:50:07AM +0100, osalvador wrote: >>> On Thu, Oct 29, 2020 at 05:27:17PM +0100, David Hildenbrand wrote: >>>> Let's revert what we did in case seomthing goes wrong and we return an >>>> error. >>> >>> Dumb question, but should not we do this for other arches as well? >> >> It seems arm64 and s390 already do that. >> x86 could have its arch_add_memory() improved though :) > > Right, I only stared at x86 and see it did not have it. > I guess we want to have all arches aligned with this. The ultimate goal would be to get rid of arch-specific arch_add_memory() implementations completely, providing arch_create_linear_mapping() / arch_remove_linear_mapping() instead (as indicated in patch #1). The x86 variant certainly needs love, but I'll keep this patch set powerpc specific, so it can go via the powerpc tree in one piece. I'll add unifying these implementations onto my todo list. Thanks!
diff --git a/arch/powerpc/mm/mem.c b/arch/powerpc/mm/mem.c index 685028451dd2..69b3e8072261 100644 --- a/arch/powerpc/mm/mem.c +++ b/arch/powerpc/mm/mem.c @@ -165,7 +165,10 @@ int __ref arch_add_memory(int nid, u64 start, u64 size, rc = arch_create_linear_mapping(nid, start, size, params); if (rc) return rc; - return __add_pages(nid, start_pfn, nr_pages, params); + rc = __add_pages(nid, start_pfn, nr_pages, params); + if (rc) + arch_remove_linear_mapping(start, size); + return rc; } void __ref arch_remove_memory(int nid, u64 start, u64 size,
Let's revert what we did in case seomthing goes wrong and we return an error. Cc: Michael Ellerman <mpe@ellerman.id.au> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org> Cc: Paul Mackerras <paulus@samba.org> Cc: Rashmica Gupta <rashmica.g@gmail.com> Cc: Andrew Morton <akpm@linux-foundation.org> Cc: Mike Rapoport <rppt@kernel.org> Cc: Michal Hocko <mhocko@suse.com> Cc: Oscar Salvador <osalvador@suse.de> Cc: Wei Yang <richard.weiyang@linux.alibaba.com> Signed-off-by: David Hildenbrand <david@redhat.com> --- arch/powerpc/mm/mem.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)