| Message ID | 20190507183804.5512-3-david@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2,1/8] mm/memory_hotplug: Simplify and fix check_hotplug_memory_range() | expand |
On Tue, May 7, 2019 at 11:38 AM David Hildenbrand <david@redhat.com> wrote: > > Will come in handy when wanting to handle errors after > arch_add_memory(). > > Cc: Martin Schwidefsky <schwidefsky@de.ibm.com> > Cc: Heiko Carstens <heiko.carstens@de.ibm.com> > Cc: Andrew Morton <akpm@linux-foundation.org> > Cc: Michal Hocko <mhocko@suse.com> > Cc: Mike Rapoport <rppt@linux.vnet.ibm.com> > Cc: David Hildenbrand <david@redhat.com> > Cc: Vasily Gorbik <gor@linux.ibm.com> > Cc: Oscar Salvador <osalvador@suse.com> > Signed-off-by: David Hildenbrand <david@redhat.com> > --- > arch/s390/mm/init.c | 13 +++++++------ > 1 file changed, 7 insertions(+), 6 deletions(-) > > diff --git a/arch/s390/mm/init.c b/arch/s390/mm/init.c > index 31b1071315d7..1e0cbae69f12 100644 > --- a/arch/s390/mm/init.c > +++ b/arch/s390/mm/init.c > @@ -237,12 +237,13 @@ int arch_add_memory(int nid, u64 start, u64 size, > void arch_remove_memory(int nid, u64 start, u64 size, > struct vmem_altmap *altmap) > { > - /* > - * There is no hardware or firmware interface which could trigger a > - * hot memory remove on s390. So there is nothing that needs to be > - * implemented. > - */ > - BUG(); > + unsigned long start_pfn = start >> PAGE_SHIFT; > + unsigned long nr_pages = size >> PAGE_SHIFT; > + struct zone *zone; > + > + zone = page_zone(pfn_to_page(start_pfn)); Does s390 actually support passing in an altmap? If 'yes', I think it also needs the vmem_altmap_offset() fixup like x86-64: /* With altmap the first mapped page is offset from @start */ if (altmap) page += vmem_altmap_offset(altmap); ...but I suspect it does not support altmap since arch/s390/mm/vmem.c::vmemmap_populate() does not arrange for 'struct page' capacity to be allocated out of an altmap defined page pool. I think it would be enough to disallow any arch_add_memory() on s390 where @altmap is non-NULL. At least until s390 gains ZONE_DEVICE support and can enable the pmem use case.
On 07.05.19 22:46, Dan Williams wrote: > On Tue, May 7, 2019 at 11:38 AM David Hildenbrand <david@redhat.com> wrote: >> >> Will come in handy when wanting to handle errors after >> arch_add_memory(). >> >> Cc: Martin Schwidefsky <schwidefsky@de.ibm.com> >> Cc: Heiko Carstens <heiko.carstens@de.ibm.com> >> Cc: Andrew Morton <akpm@linux-foundation.org> >> Cc: Michal Hocko <mhocko@suse.com> >> Cc: Mike Rapoport <rppt@linux.vnet.ibm.com> >> Cc: David Hildenbrand <david@redhat.com> >> Cc: Vasily Gorbik <gor@linux.ibm.com> >> Cc: Oscar Salvador <osalvador@suse.com> >> Signed-off-by: David Hildenbrand <david@redhat.com> >> --- >> arch/s390/mm/init.c | 13 +++++++------ >> 1 file changed, 7 insertions(+), 6 deletions(-) >> >> diff --git a/arch/s390/mm/init.c b/arch/s390/mm/init.c >> index 31b1071315d7..1e0cbae69f12 100644 >> --- a/arch/s390/mm/init.c >> +++ b/arch/s390/mm/init.c >> @@ -237,12 +237,13 @@ int arch_add_memory(int nid, u64 start, u64 size, >> void arch_remove_memory(int nid, u64 start, u64 size, >> struct vmem_altmap *altmap) >> { >> - /* >> - * There is no hardware or firmware interface which could trigger a >> - * hot memory remove on s390. So there is nothing that needs to be >> - * implemented. >> - */ >> - BUG(); >> + unsigned long start_pfn = start >> PAGE_SHIFT; >> + unsigned long nr_pages = size >> PAGE_SHIFT; >> + struct zone *zone; >> + >> + zone = page_zone(pfn_to_page(start_pfn)); > > Does s390 actually support passing in an altmap? If 'yes', I think it > also needs the vmem_altmap_offset() fixup like x86-64: > > /* With altmap the first mapped page is offset from @start */ > if (altmap) > page += vmem_altmap_offset(altmap); > > ...but I suspect it does not support altmap since > arch/s390/mm/vmem.c::vmemmap_populate() does not arrange for 'struct > page' capacity to be allocated out of an altmap defined page pool. > > I think it would be enough to disallow any arch_add_memory() on s390 > where @altmap is non-NULL. At least until s390 gains ZONE_DEVICE > support and can enable the pmem use case. > As far as I know, it doesn't yet, however I guess this could change once virtio-pmem is supported? Thanks!
On Tue, May 7, 2019 at 1:47 PM David Hildenbrand <david@redhat.com> wrote: > > On 07.05.19 22:46, Dan Williams wrote: > > On Tue, May 7, 2019 at 11:38 AM David Hildenbrand <david@redhat.com> wrote: > >> > >> Will come in handy when wanting to handle errors after > >> arch_add_memory(). > >> > >> Cc: Martin Schwidefsky <schwidefsky@de.ibm.com> > >> Cc: Heiko Carstens <heiko.carstens@de.ibm.com> > >> Cc: Andrew Morton <akpm@linux-foundation.org> > >> Cc: Michal Hocko <mhocko@suse.com> > >> Cc: Mike Rapoport <rppt@linux.vnet.ibm.com> > >> Cc: David Hildenbrand <david@redhat.com> > >> Cc: Vasily Gorbik <gor@linux.ibm.com> > >> Cc: Oscar Salvador <osalvador@suse.com> > >> Signed-off-by: David Hildenbrand <david@redhat.com> > >> --- > >> arch/s390/mm/init.c | 13 +++++++------ > >> 1 file changed, 7 insertions(+), 6 deletions(-) > >> > >> diff --git a/arch/s390/mm/init.c b/arch/s390/mm/init.c > >> index 31b1071315d7..1e0cbae69f12 100644 > >> --- a/arch/s390/mm/init.c > >> +++ b/arch/s390/mm/init.c > >> @@ -237,12 +237,13 @@ int arch_add_memory(int nid, u64 start, u64 size, > >> void arch_remove_memory(int nid, u64 start, u64 size, > >> struct vmem_altmap *altmap) > >> { > >> - /* > >> - * There is no hardware or firmware interface which could trigger a > >> - * hot memory remove on s390. So there is nothing that needs to be > >> - * implemented. > >> - */ > >> - BUG(); > >> + unsigned long start_pfn = start >> PAGE_SHIFT; > >> + unsigned long nr_pages = size >> PAGE_SHIFT; > >> + struct zone *zone; > >> + > >> + zone = page_zone(pfn_to_page(start_pfn)); > > > > Does s390 actually support passing in an altmap? If 'yes', I think it > > also needs the vmem_altmap_offset() fixup like x86-64: > > > > /* With altmap the first mapped page is offset from @start */ > > if (altmap) > > page += vmem_altmap_offset(altmap); > > > > ...but I suspect it does not support altmap since > > arch/s390/mm/vmem.c::vmemmap_populate() does not arrange for 'struct > > page' capacity to be allocated out of an altmap defined page pool. > > > > I think it would be enough to disallow any arch_add_memory() on s390 > > where @altmap is non-NULL. At least until s390 gains ZONE_DEVICE > > support and can enable the pmem use case. > > > > As far as I know, it doesn't yet, however I guess this could change once > virtio-pmem is supported? I would expect and request virtio-pmem remain a non-starter on s390 until s390 gains ZONE_DEVICE support. As it stands virtio-pmem is just another flavor of the general pmem driver and the pmem driver currently only exports ZONE_DEVICE pfns tagged by the PTE_DEVMAP pte-flag and PFN_DEV+PFN_MAP pfn_t-flags. A hamstrung version of DAX (CONFIG_FS_DAX_LIMITED) is enabled for the s390/dcssblk driver, but that requires that the driver indicate this limited use case via the PTE_SPECIAL pte-flag and PFN_SPECIAL pfn_t-flag. I otherwise do not want to see CONFIG_FS_DAX_LIMITED spread outside of the s390/dcssblk use case.
On 07.05.19 22:57, Dan Williams wrote: > On Tue, May 7, 2019 at 1:47 PM David Hildenbrand <david@redhat.com> wrote: >> >> On 07.05.19 22:46, Dan Williams wrote: >>> On Tue, May 7, 2019 at 11:38 AM David Hildenbrand <david@redhat.com> wrote: >>>> >>>> Will come in handy when wanting to handle errors after >>>> arch_add_memory(). >>>> >>>> Cc: Martin Schwidefsky <schwidefsky@de.ibm.com> >>>> Cc: Heiko Carstens <heiko.carstens@de.ibm.com> >>>> Cc: Andrew Morton <akpm@linux-foundation.org> >>>> Cc: Michal Hocko <mhocko@suse.com> >>>> Cc: Mike Rapoport <rppt@linux.vnet.ibm.com> >>>> Cc: David Hildenbrand <david@redhat.com> >>>> Cc: Vasily Gorbik <gor@linux.ibm.com> >>>> Cc: Oscar Salvador <osalvador@suse.com> >>>> Signed-off-by: David Hildenbrand <david@redhat.com> >>>> --- >>>> arch/s390/mm/init.c | 13 +++++++------ >>>> 1 file changed, 7 insertions(+), 6 deletions(-) >>>> >>>> diff --git a/arch/s390/mm/init.c b/arch/s390/mm/init.c >>>> index 31b1071315d7..1e0cbae69f12 100644 >>>> --- a/arch/s390/mm/init.c >>>> +++ b/arch/s390/mm/init.c >>>> @@ -237,12 +237,13 @@ int arch_add_memory(int nid, u64 start, u64 size, >>>> void arch_remove_memory(int nid, u64 start, u64 size, >>>> struct vmem_altmap *altmap) >>>> { >>>> - /* >>>> - * There is no hardware or firmware interface which could trigger a >>>> - * hot memory remove on s390. So there is nothing that needs to be >>>> - * implemented. >>>> - */ >>>> - BUG(); >>>> + unsigned long start_pfn = start >> PAGE_SHIFT; >>>> + unsigned long nr_pages = size >> PAGE_SHIFT; >>>> + struct zone *zone; >>>> + >>>> + zone = page_zone(pfn_to_page(start_pfn)); >>> >>> Does s390 actually support passing in an altmap? If 'yes', I think it >>> also needs the vmem_altmap_offset() fixup like x86-64: >>> >>> /* With altmap the first mapped page is offset from @start */ >>> if (altmap) >>> page += vmem_altmap_offset(altmap); >>> >>> ...but I suspect it does not support altmap since >>> arch/s390/mm/vmem.c::vmemmap_populate() does not arrange for 'struct >>> page' capacity to be allocated out of an altmap defined page pool. >>> >>> I think it would be enough to disallow any arch_add_memory() on s390 >>> where @altmap is non-NULL. At least until s390 gains ZONE_DEVICE >>> support and can enable the pmem use case. >>> >> >> As far as I know, it doesn't yet, however I guess this could change once >> virtio-pmem is supported? > > I would expect and request virtio-pmem remain a non-starter on s390 > until s390 gains ZONE_DEVICE support. As it stands virtio-pmem is just > another flavor of the general pmem driver and the pmem driver > currently only exports ZONE_DEVICE pfns tagged by the PTE_DEVMAP > pte-flag and PFN_DEV+PFN_MAP pfn_t-flags. Yes, I think ZONE_DEVICE will be the way to go. On real HW, there will never be anything mapped into the physical address space besides system ram. However with virtio-pmem in virtual environments, we have the option to change that.
diff --git a/arch/s390/mm/init.c b/arch/s390/mm/init.c index 31b1071315d7..1e0cbae69f12 100644 --- a/arch/s390/mm/init.c +++ b/arch/s390/mm/init.c @@ -237,12 +237,13 @@ int arch_add_memory(int nid, u64 start, u64 size, void arch_remove_memory(int nid, u64 start, u64 size, struct vmem_altmap *altmap) { - /* - * There is no hardware or firmware interface which could trigger a - * hot memory remove on s390. So there is nothing that needs to be - * implemented. - */ - BUG(); + unsigned long start_pfn = start >> PAGE_SHIFT; + unsigned long nr_pages = size >> PAGE_SHIFT; + struct zone *zone; + + zone = page_zone(pfn_to_page(start_pfn)); + __remove_pages(zone, start_pfn, nr_pages, altmap); + vmem_remove_mapping(start, size); } #endif #endif /* CONFIG_MEMORY_HOTPLUG */
Will come in handy when wanting to handle errors after arch_add_memory(). Cc: Martin Schwidefsky <schwidefsky@de.ibm.com> Cc: Heiko Carstens <heiko.carstens@de.ibm.com> Cc: Andrew Morton <akpm@linux-foundation.org> Cc: Michal Hocko <mhocko@suse.com> Cc: Mike Rapoport <rppt@linux.vnet.ibm.com> Cc: David Hildenbrand <david@redhat.com> Cc: Vasily Gorbik <gor@linux.ibm.com> Cc: Oscar Salvador <osalvador@suse.com> Signed-off-by: David Hildenbrand <david@redhat.com> --- arch/s390/mm/init.c | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-)