| Message ID | 20190625075227.15193-1-osalvador@suse.de (mailing list archive) |
|---|---|
| Headers | show |
| Series | Allocate memmap from hotadded memory | expand |
On 25.06.19 09:52, Oscar Salvador wrote: > Hi, > > It has been while since I sent previous version [1]. > > In this version I added some feedback I got back then, like letting > the caller decide whether he wants allocating per memory block or > per memory range (patch#2), and having the chance to disable vmemmap when > users want to expose all hotpluggable memory to userspace (patch#5). > > [Testing] > > While I could test last version on powerpc, and Huawei's fellows helped me out > testing it on arm64, this time I could only test it on x86_64. > The codebase is quite the same, so I would not expect surprises. > > - x86_64: small and large memblocks (128MB, 1G and 2G) > - Kernel module that adds memory spanning multiple memblocks > and remove that memory in a different granularity. > > So far, only acpi memory hotplug uses the new flag. > The other callers can be changed depending on their needs. > > Of course, more testing and feedback is appreciated. > > [Coverletter] > > This is another step to make memory hotplug more usable. The primary > goal of this patchset is to reduce memory overhead of the hot-added > memory (at least for SPARSEMEM_VMEMMAP memory model). The current way we use > to populate memmap (struct page array) has two main drawbacks: Mental note: How will it be handled if a caller specifies "Allocate memmap from hotadded memory", but we are running under SPARSEMEM where we can't do this. > > a) it consumes an additional memory until the hotadded memory itself is > onlined and > b) memmap might end up on a different numa node which is especially true > for movable_node configuration. > > a) it is a problem especially for memory hotplug based memory "ballooning" > solutions when the delay between physical memory hotplug and the > onlining can lead to OOM and that led to introduction of hacks like auto > onlining (see 31bc3858ea3e ("memory-hotplug: add automatic onlining > policy for the newly added memory")). > > b) can have performance drawbacks. > > Another minor case is that I have seen hot-add operations failing on archs > because they were running out of order-x pages. > E.g On powerpc, in certain configurations, we use order-8 pages, > and given 64KB base pagesize, that is 16MB. > If we run out of those, we just fail the operation and we cannot add > more memory. At least for SPARSEMEM, we fallback to vmalloc() to work around this issue. I haven't looked into the populate_section_memmap() internals yet. Can you point me at the code that performs this allocation? > We could fallback to base pages as x86_64 does, but we can do better. > > One way to mitigate all these issues is to simply allocate memmap array > (which is the largest memory footprint of the physical memory hotplug) > from the hot-added memory itself. SPARSEMEM_VMEMMAP memory model allows > us to map any pfn range so the memory doesn't need to be online to be > usable for the array. See patch 3 for more details. > This feature is only usable when CONFIG_SPARSEMEM_VMEMMAP is set. > > [Overall design]: > > Implementation wise we reuse vmem_altmap infrastructure to override > the default allocator used by vmemap_populate. Once the memmap is > allocated we need a way to mark altmap pfns used for the allocation. > If MHP_MEMMAP_{DEVICE,MEMBLOCK} flag was passed, we set up the layout of the > altmap structure at the beginning of __add_pages(), and then we call > mark_vmemmap_pages(). > > The flags are either MHP_MEMMAP_DEVICE or MHP_MEMMAP_MEMBLOCK, and only differ > in the way they allocate vmemmap pages within the memory blocks. > > MHP_MEMMAP_MEMBLOCK: > - With this flag, we will allocate vmemmap pages in each memory block. > This means that if we hot-add a range that spans multiple memory blocks, > we will use the beginning of each memory block for the vmemmap pages. > This strategy is good for cases where the caller wants the flexiblity > to hot-remove memory in a different granularity than when it was added. > > MHP_MEMMAP_DEVICE: > - With this flag, we will store all vmemmap pages at the beginning of > hot-added memory. > > So it is a tradeoff of flexiblity vs contigous memory. > More info on the above can be found in patch#2. > > Depending on which flag is passed (MHP_MEMMAP_DEVICE or MHP_MEMMAP_MEMBLOCK), > mark_vmemmap_pages() gets called at a different stage. > With MHP_MEMMAP_MEMBLOCK, we call it once we have populated the sections > fitting in a single memblock, while with MHP_MEMMAP_DEVICE we wait until all > sections have been populated. > > mark_vmemmap_pages() marks the pages as vmemmap and sets some metadata: > > The current layout of the Vmemmap pages are: > > [Head->refcount] : Nr sections used by this altmap > [Head->private] : Nr of vmemmap pages > [Tail->freelist] : Pointer to the head page > > This is done to easy the computation we need in some places. > E.g: > > Example 1) > We hot-add 1GB on x86_64 (memory block 128MB) using > MHP_MEMMAP_DEVICE: > > head->_refcount = 8 sections > head->private = 4096 vmemmap pages > tail's->freelist = head > > Example 2) > We hot-add 1GB on x86_64 using MHP_MEMMAP_MEMBLOCK: > > [at the beginning of each memblock] > head->_refcount = 1 section > head->private = 512 vmemmap pages > tail's->freelist = head > > We have the refcount because when using MHP_MEMMAP_DEVICE, we need to know > how much do we have to defer the call to vmemmap_free(). > The thing is that the first pages of the hot-added range are used to create > the memmap mapping, so we cannot remove those first, otherwise we would blow up > when accessing the other pages. So, assuming we add_memory(1GB, MHP_MEMMAP_DEVICE) and then remove_memory(128MB) of the added memory, this will work? add_memory(8GB, MHP_MEMMAP_DEVICE) For 8GB, we will need exactly 128MB of memmap if I did the math right. So exactly one section. This section will still be marked as being online (although not pages on it are actually online)? > > What we do is that since when we hot-remove a memory-range, sections are being > removed sequentially, we wait until we hit the last section, and then we free > the hole range to vmemmap_free backwards. > We know that it is the last section because in every pass we > decrease head->_refcount, and when it reaches 0, we got our last section. > > We also have to be careful about those pages during online and offline > operations. They are simply skipped, so online will keep them > reserved and so unusable for any other purpose and offline ignores them > so they do not block the offline operation. I assume that they will still be dumped normally by user space. (as they are described by a "memory resource" and not PG_Offline) > > One thing worth mention is that vmemmap pages residing in movable memory is not a > show-stopper for that memory to be offlined/migrated away. > Vmemmap pages are just ignored in that case and they stick around until sections > referred by those vmemmap pages are hot-removed. > > [1] https://patchwork.kernel.org/cover/10875017/ > > Oscar Salvador (5): > drivers/base/memory: Remove unneeded check in > remove_memory_block_devices > mm,memory_hotplug: Introduce MHP_VMEMMAP_FLAGS > mm,memory_hotplug: Introduce Vmemmap page helpers > mm,memory_hotplug: allocate memmap from the added memory range for > sparse-vmemmap > mm,memory_hotplug: Allow userspace to enable/disable vmemmap > > arch/arm64/mm/mmu.c | 5 +- > arch/powerpc/mm/init_64.c | 7 ++ > arch/s390/mm/init.c | 6 ++ > arch/x86/mm/init_64.c | 10 +++ > drivers/acpi/acpi_memhotplug.c | 2 +- > drivers/base/memory.c | 41 +++++++++-- > drivers/dax/kmem.c | 2 +- > drivers/hv/hv_balloon.c | 2 +- > drivers/s390/char/sclp_cmd.c | 2 +- > drivers/xen/balloon.c | 2 +- > include/linux/memory_hotplug.h | 31 ++++++++- > include/linux/memremap.h | 2 +- > include/linux/page-flags.h | 34 +++++++++ > mm/compaction.c | 7 ++ > mm/memory_hotplug.c | 152 ++++++++++++++++++++++++++++++++++------- > mm/page_alloc.c | 22 +++++- > mm/page_isolation.c | 14 +++- > mm/sparse.c | 93 +++++++++++++++++++++++++ > mm/util.c | 2 + > 19 files changed, 394 insertions(+), 42 deletions(-) > Thanks for doing this, this will be very helpful :)
On 25.06.19 10:25, David Hildenbrand wrote: > On 25.06.19 09:52, Oscar Salvador wrote: >> Hi, >> >> It has been while since I sent previous version [1]. >> >> In this version I added some feedback I got back then, like letting >> the caller decide whether he wants allocating per memory block or >> per memory range (patch#2), and having the chance to disable vmemmap when >> users want to expose all hotpluggable memory to userspace (patch#5). >> >> [Testing] >> >> While I could test last version on powerpc, and Huawei's fellows helped me out >> testing it on arm64, this time I could only test it on x86_64. >> The codebase is quite the same, so I would not expect surprises. >> >> - x86_64: small and large memblocks (128MB, 1G and 2G) >> - Kernel module that adds memory spanning multiple memblocks >> and remove that memory in a different granularity. >> >> So far, only acpi memory hotplug uses the new flag. >> The other callers can be changed depending on their needs. >> >> Of course, more testing and feedback is appreciated. >> >> [Coverletter] >> >> This is another step to make memory hotplug more usable. The primary >> goal of this patchset is to reduce memory overhead of the hot-added >> memory (at least for SPARSEMEM_VMEMMAP memory model). The current way we use >> to populate memmap (struct page array) has two main drawbacks: > > Mental note: How will it be handled if a caller specifies "Allocate > memmap from hotadded memory", but we are running under SPARSEMEM where > we can't do this. > >> >> a) it consumes an additional memory until the hotadded memory itself is >> onlined and >> b) memmap might end up on a different numa node which is especially true >> for movable_node configuration. >> >> a) it is a problem especially for memory hotplug based memory "ballooning" >> solutions when the delay between physical memory hotplug and the >> onlining can lead to OOM and that led to introduction of hacks like auto >> onlining (see 31bc3858ea3e ("memory-hotplug: add automatic onlining >> policy for the newly added memory")). >> >> b) can have performance drawbacks. >> >> Another minor case is that I have seen hot-add operations failing on archs >> because they were running out of order-x pages. >> E.g On powerpc, in certain configurations, we use order-8 pages, >> and given 64KB base pagesize, that is 16MB. >> If we run out of those, we just fail the operation and we cannot add >> more memory. > > At least for SPARSEMEM, we fallback to vmalloc() to work around this > issue. I haven't looked into the populate_section_memmap() internals > yet. Can you point me at the code that performs this allocation? > >> We could fallback to base pages as x86_64 does, but we can do better. >> >> One way to mitigate all these issues is to simply allocate memmap array >> (which is the largest memory footprint of the physical memory hotplug) >> from the hot-added memory itself. SPARSEMEM_VMEMMAP memory model allows >> us to map any pfn range so the memory doesn't need to be online to be >> usable for the array. See patch 3 for more details. >> This feature is only usable when CONFIG_SPARSEMEM_VMEMMAP is set. >> >> [Overall design]: >> >> Implementation wise we reuse vmem_altmap infrastructure to override >> the default allocator used by vmemap_populate. Once the memmap is >> allocated we need a way to mark altmap pfns used for the allocation. >> If MHP_MEMMAP_{DEVICE,MEMBLOCK} flag was passed, we set up the layout of the >> altmap structure at the beginning of __add_pages(), and then we call >> mark_vmemmap_pages(). >> >> The flags are either MHP_MEMMAP_DEVICE or MHP_MEMMAP_MEMBLOCK, and only differ >> in the way they allocate vmemmap pages within the memory blocks. >> >> MHP_MEMMAP_MEMBLOCK: >> - With this flag, we will allocate vmemmap pages in each memory block. >> This means that if we hot-add a range that spans multiple memory blocks, >> we will use the beginning of each memory block for the vmemmap pages. >> This strategy is good for cases where the caller wants the flexiblity >> to hot-remove memory in a different granularity than when it was added. >> >> MHP_MEMMAP_DEVICE: >> - With this flag, we will store all vmemmap pages at the beginning of >> hot-added memory. >> >> So it is a tradeoff of flexiblity vs contigous memory. >> More info on the above can be found in patch#2. >> >> Depending on which flag is passed (MHP_MEMMAP_DEVICE or MHP_MEMMAP_MEMBLOCK), >> mark_vmemmap_pages() gets called at a different stage. >> With MHP_MEMMAP_MEMBLOCK, we call it once we have populated the sections >> fitting in a single memblock, while with MHP_MEMMAP_DEVICE we wait until all >> sections have been populated. >> >> mark_vmemmap_pages() marks the pages as vmemmap and sets some metadata: >> >> The current layout of the Vmemmap pages are: >> >> [Head->refcount] : Nr sections used by this altmap >> [Head->private] : Nr of vmemmap pages >> [Tail->freelist] : Pointer to the head page >> >> This is done to easy the computation we need in some places. >> E.g: >> >> Example 1) >> We hot-add 1GB on x86_64 (memory block 128MB) using >> MHP_MEMMAP_DEVICE: >> >> head->_refcount = 8 sections >> head->private = 4096 vmemmap pages >> tail's->freelist = head >> >> Example 2) >> We hot-add 1GB on x86_64 using MHP_MEMMAP_MEMBLOCK: >> >> [at the beginning of each memblock] >> head->_refcount = 1 section >> head->private = 512 vmemmap pages >> tail's->freelist = head >> >> We have the refcount because when using MHP_MEMMAP_DEVICE, we need to know >> how much do we have to defer the call to vmemmap_free(). >> The thing is that the first pages of the hot-added range are used to create >> the memmap mapping, so we cannot remove those first, otherwise we would blow up >> when accessing the other pages. > > So, assuming we add_memory(1GB, MHP_MEMMAP_DEVICE) and then > remove_memory(128MB) of the added memory, this will work? Hmm, I guess this won't work - especially when removing the first 128MB first, where the memmap resides. Do we need MHP_MEMMAP_DEVICE at this point or could we start with MHP_MEMMAP_MEMBLOCK? That "smells" like being the easier case.
On Tue, Jun 25, 2019 at 10:25:48AM +0200, David Hildenbrand wrote: > > [Coverletter] > > > > This is another step to make memory hotplug more usable. The primary > > goal of this patchset is to reduce memory overhead of the hot-added > > memory (at least for SPARSEMEM_VMEMMAP memory model). The current way we use > > to populate memmap (struct page array) has two main drawbacks: First off, thanks for looking into this :-) > > Mental note: How will it be handled if a caller specifies "Allocate > memmap from hotadded memory", but we are running under SPARSEMEM where > we can't do this. In add_memory_resource(), we have a call to mhp_check_correct_flags(), which is in charge of checking if the flags passed are compliant with our configuration among other things. It also checks if both flags were passed (_MEMBLOCK|_DEVICE). If a) any of the flags were specified and we are not on CONFIG_SPARSEMEM_VMEMMAP, b) the flags are colliding with each other or c) the flags just do not make sense, we print out a warning and drop the flags to 0, so we just ignore them. I just realized that I can adjust the check even more (something for the next version). But to answer your question, flags are ignored under !CONFIG_SPARSEMEM_VMEMMAP. > > > > > a) it consumes an additional memory until the hotadded memory itself is > > onlined and > > b) memmap might end up on a different numa node which is especially true > > for movable_node configuration. > > > > a) it is a problem especially for memory hotplug based memory "ballooning" > > solutions when the delay between physical memory hotplug and the > > onlining can lead to OOM and that led to introduction of hacks like auto > > onlining (see 31bc3858ea3e ("memory-hotplug: add automatic onlining > > policy for the newly added memory")). > > > > b) can have performance drawbacks. > > > > Another minor case is that I have seen hot-add operations failing on archs > > because they were running out of order-x pages. > > E.g On powerpc, in certain configurations, we use order-8 pages, > > and given 64KB base pagesize, that is 16MB. > > If we run out of those, we just fail the operation and we cannot add > > more memory. > > At least for SPARSEMEM, we fallback to vmalloc() to work around this > issue. I haven't looked into the populate_section_memmap() internals > yet. Can you point me at the code that performs this allocation? Yes, on SPARSEMEM we first try to allocate the pages physical configuous, and then fallback to vmalloc. This is because on CONFIG_SPARSEMEM memory model, the translations pfn_to_page/ page_to_pfn do not expect the memory to be contiguous. But that is not the case on CONFIG_SPARSEMEM_VMEMMAP. We do expect the memory to be physical contigous there, that is why a simply pfn_to_page/page_to_pfn is a matter of adding/substracting vmemmap/pfn. Powerpc code is at: https://elixir.bootlin.com/linux/v5.2-rc6/source/arch/powerpc/mm/init_64.c#L175 > So, assuming we add_memory(1GB, MHP_MEMMAP_DEVICE) and then > remove_memory(128MB) of the added memory, this will work? No, MHP_MEMMAP_DEVICE is meant to be used when hot-adding and hot-removing work in the same granularity. This is because all memmap pages will be stored at the beginning of the memory range. Allowing hot-removing in a different granularity on MHP_MEMMAP_DEVICE would imply a lot of extra work. For example, we would have to parse the vmemmap-head of the hot-removed range, and punch a hole in there to clear the vmemmap pages, and then be very carefull when deleting those pagetables. So I followed Michal's advice, and I decided to let the caller specify if he either wants to allocate per memory block or per hot-added range(device). Where per memory block, allows us to do: add_memory(1GB, MHP_MEMMAP_MEMBLOCK) remove_memory(128MB) > add_memory(8GB, MHP_MEMMAP_DEVICE) > > For 8GB, we will need exactly 128MB of memmap if I did the math right. > So exactly one section. This section will still be marked as being > online (although not pages on it are actually online)? Yap, 8GB will fill the first section with vmemmap pages. It will be marked as online, yes. This is not to diverge too much from what we have right now, and starting treat some sections different than others. E.g: Early sections that are used for memmap pages on early boot stage. > > > > What we do is that since when we hot-remove a memory-range, sections are being > > removed sequentially, we wait until we hit the last section, and then we free > > the hole range to vmemmap_free backwards. > > We know that it is the last section because in every pass we > > decrease head->_refcount, and when it reaches 0, we got our last section. > > > > We also have to be careful about those pages during online and offline > > operations. They are simply skipped, so online will keep them > > reserved and so unusable for any other purpose and offline ignores them > > so they do not block the offline operation. > > I assume that they will still be dumped normally by user space. (as they > are described by a "memory resource" and not PG_Offline) They are PG_Reserved. Anyway, you mean by crash-tool?
On 26.06.19 10:03, Oscar Salvador wrote: > On Tue, Jun 25, 2019 at 10:25:48AM +0200, David Hildenbrand wrote: >>> [Coverletter] >>> >>> This is another step to make memory hotplug more usable. The primary >>> goal of this patchset is to reduce memory overhead of the hot-added >>> memory (at least for SPARSEMEM_VMEMMAP memory model). The current way we use >>> to populate memmap (struct page array) has two main drawbacks: > > First off, thanks for looking into this :-) Thanks for working on this ;) > >> >> Mental note: How will it be handled if a caller specifies "Allocate >> memmap from hotadded memory", but we are running under SPARSEMEM where >> we can't do this. > > In add_memory_resource(), we have a call to mhp_check_correct_flags(), which is > in charge of checking if the flags passed are compliant with our configuration > among other things. > It also checks if both flags were passed (_MEMBLOCK|_DEVICE). > > If a) any of the flags were specified and we are not on CONFIG_SPARSEMEM_VMEMMAP, > b) the flags are colliding with each other or c) the flags just do not make sense, > we print out a warning and drop the flags to 0, so we just ignore them. > > I just realized that I can adjust the check even more (something for the next > version). > > But to answer your question, flags are ignored under !CONFIG_SPARSEMEM_VMEMMAP. So it is indeed a hint only. > >> >>> >>> a) it consumes an additional memory until the hotadded memory itself is >>> onlined and >>> b) memmap might end up on a different numa node which is especially true >>> for movable_node configuration. >>> >>> a) it is a problem especially for memory hotplug based memory "ballooning" >>> solutions when the delay between physical memory hotplug and the >>> onlining can lead to OOM and that led to introduction of hacks like auto >>> onlining (see 31bc3858ea3e ("memory-hotplug: add automatic onlining >>> policy for the newly added memory")). >>> >>> b) can have performance drawbacks. >>> >>> Another minor case is that I have seen hot-add operations failing on archs >>> because they were running out of order-x pages. >>> E.g On powerpc, in certain configurations, we use order-8 pages, >>> and given 64KB base pagesize, that is 16MB. >>> If we run out of those, we just fail the operation and we cannot add >>> more memory. >> >> At least for SPARSEMEM, we fallback to vmalloc() to work around this >> issue. I haven't looked into the populate_section_memmap() internals >> yet. Can you point me at the code that performs this allocation? > > Yes, on SPARSEMEM we first try to allocate the pages physical configuous, and > then fallback to vmalloc. > This is because on CONFIG_SPARSEMEM memory model, the translations pfn_to_page/ > page_to_pfn do not expect the memory to be contiguous. > > But that is not the case on CONFIG_SPARSEMEM_VMEMMAP. > We do expect the memory to be physical contigous there, that is why a simply > pfn_to_page/page_to_pfn is a matter of adding/substracting vmemmap/pfn. Yeas, I explored that last week but didn't figure out where the actual vmmap population code resided - thanks :) > > Powerpc code is at: > > https://elixir.bootlin.com/linux/v5.2-rc6/source/arch/powerpc/mm/init_64.c#L175 > > > >> So, assuming we add_memory(1GB, MHP_MEMMAP_DEVICE) and then >> remove_memory(128MB) of the added memory, this will work? > > No, MHP_MEMMAP_DEVICE is meant to be used when hot-adding and hot-removing work > in the same granularity. > This is because all memmap pages will be stored at the beginning of the memory > range. > Allowing hot-removing in a different granularity on MHP_MEMMAP_DEVICE would imply > a lot of extra work. > For example, we would have to parse the vmemmap-head of the hot-removed range, > and punch a hole in there to clear the vmemmap pages, and then be very carefull > when deleting those pagetables. > > So I followed Michal's advice, and I decided to let the caller specify if he > either wants to allocate per memory block or per hot-added range(device). > Where per memory block, allows us to do: > > add_memory(1GB, MHP_MEMMAP_MEMBLOCK) > remove_memory(128MB) Back then, I already mentioned that we might have some users that remove_memory() they never added in a granularity it wasn't added. My concerns back then were never fully sorted out. arch/powerpc/platforms/powernv/memtrace.c - Will remove memory in memory block size chunks it never added - What if that memory resides on a DIMM added via MHP_MEMMAP_DEVICE? Will it at least bail out? Or simply break? IOW: I am not yet 100% convinced that MHP_MEMMAP_DEVICE is save to be introduced.
On Wed, Jun 26, 2019 at 10:11:06AM +0200, David Hildenbrand wrote: > Back then, I already mentioned that we might have some users that > remove_memory() they never added in a granularity it wasn't added. My > concerns back then were never fully sorted out. > > arch/powerpc/platforms/powernv/memtrace.c > > - Will remove memory in memory block size chunks it never added > - What if that memory resides on a DIMM added via MHP_MEMMAP_DEVICE? > > Will it at least bail out? Or simply break? > > IOW: I am not yet 100% convinced that MHP_MEMMAP_DEVICE is save to be > introduced. Uhm, I will take a closer look and see if I can clear your concerns. TBH, I did not try to use arch/powerpc/platforms/powernv/memtrace.c yet. I will get back to you once I tried it out.
On Wed, Jun 26, 2019 at 10:15:16AM +0200, Oscar Salvador wrote: > On Wed, Jun 26, 2019 at 10:11:06AM +0200, David Hildenbrand wrote: > > Back then, I already mentioned that we might have some users that > > remove_memory() they never added in a granularity it wasn't added. My > > concerns back then were never fully sorted out. > > > > arch/powerpc/platforms/powernv/memtrace.c > > > > - Will remove memory in memory block size chunks it never added > > - What if that memory resides on a DIMM added via MHP_MEMMAP_DEVICE? > > > > Will it at least bail out? Or simply break? > > > > IOW: I am not yet 100% convinced that MHP_MEMMAP_DEVICE is save to be > > introduced. > > Uhm, I will take a closer look and see if I can clear your concerns. > TBH, I did not try to use arch/powerpc/platforms/powernv/memtrace.c > yet. > > I will get back to you once I tried it out. On a second though, it would be quite trivial to implement a check in remove_memory() that does not allow to remove memory used with MHP_MEMMAP_DEVICE in a different granularity: +static bool check_vmemmap_granularity(u64 start, u64 size); +{ + unsigned long pfn; + unsigned int nr_pages; + struct page *p; + + pfn = PHYS_PFN(start); + p = pfn_to_page(pfn); + nr_pages = size >> PAGE_SIZE; + + if (PageVmemmap(p)) { + struct page *h = vmemmap_get_head(p); + unsigned long sections = (unsigned long)h->private; + + if (sections * PAGES_PER_SECTION > nr_pages) + fail; + } + no_fail; +} + + static int __ref try_remove_memory(int nid, u64 start, u64 size) { int rc = 0; BUG_ON(check_hotplug_memory_range(start, size)); mem_hotplug_begin(); + rc = check_vmemmap_granularity(start, size); + if (rc) + goto done; The above is quite hacky, but it gives an idea. I will try the code from arch/powerpc/platforms/powernv/memtrace.c and see how can I implement a check.
On 26.06.19 10:15, Oscar Salvador wrote: > On Wed, Jun 26, 2019 at 10:11:06AM +0200, David Hildenbrand wrote: >> Back then, I already mentioned that we might have some users that >> remove_memory() they never added in a granularity it wasn't added. My >> concerns back then were never fully sorted out. >> >> arch/powerpc/platforms/powernv/memtrace.c >> >> - Will remove memory in memory block size chunks it never added >> - What if that memory resides on a DIMM added via MHP_MEMMAP_DEVICE? >> >> Will it at least bail out? Or simply break? >> >> IOW: I am not yet 100% convinced that MHP_MEMMAP_DEVICE is save to be >> introduced. > > Uhm, I will take a closer look and see if I can clear your concerns. > TBH, I did not try to use arch/powerpc/platforms/powernv/memtrace.c > yet. > > I will get back to you once I tried it out. > BTW, I consider the code in arch/powerpc/platforms/powernv/memtrace.c very ugly and dangerous. We should never allow to manually offline/online pages / hack into memory block states. What I would want to see here is rather: 1. User space offlines the blocks to be used 2. memtrace installs a hotplug notifier and hinders the blocks it wants to use from getting onlined. 3. memory is not added/removed/onlined/offlined in memtrace code. CCing the DEVs.
On 26.06.19 10:27, Oscar Salvador wrote: > On Wed, Jun 26, 2019 at 10:15:16AM +0200, Oscar Salvador wrote: >> On Wed, Jun 26, 2019 at 10:11:06AM +0200, David Hildenbrand wrote: >>> Back then, I already mentioned that we might have some users that >>> remove_memory() they never added in a granularity it wasn't added. My >>> concerns back then were never fully sorted out. >>> >>> arch/powerpc/platforms/powernv/memtrace.c >>> >>> - Will remove memory in memory block size chunks it never added >>> - What if that memory resides on a DIMM added via MHP_MEMMAP_DEVICE? >>> >>> Will it at least bail out? Or simply break? >>> >>> IOW: I am not yet 100% convinced that MHP_MEMMAP_DEVICE is save to be >>> introduced. >> >> Uhm, I will take a closer look and see if I can clear your concerns. >> TBH, I did not try to use arch/powerpc/platforms/powernv/memtrace.c >> yet. >> >> I will get back to you once I tried it out. > > On a second though, it would be quite trivial to implement a check in > remove_memory() that does not allow to remove memory used with MHP_MEMMAP_DEVICE > in a different granularity: > > +static bool check_vmemmap_granularity(u64 start, u64 size); > +{ > + unsigned long pfn; > + unsigned int nr_pages; > + struct page *p; > + > + pfn = PHYS_PFN(start); > + p = pfn_to_page(pfn); > + nr_pages = size >> PAGE_SIZE; > + > + if (PageVmemmap(p)) { > + struct page *h = vmemmap_get_head(p); > + unsigned long sections = (unsigned long)h->private; > + > + if (sections * PAGES_PER_SECTION > nr_pages) > + fail; > + } > + no_fail; > +} > + > + > static int __ref try_remove_memory(int nid, u64 start, u64 size) > { > int rc = 0; > > BUG_ON(check_hotplug_memory_range(start, size)); > > mem_hotplug_begin(); > > + rc = check_vmemmap_granularity(start, size); > + if (rc) > + goto done; > > > The above is quite hacky, but it gives an idea. > I will try the code from arch/powerpc/platforms/powernv/memtrace.c and see how > can I implement a check. > Yeah, I would consider such a safety check mandatory for MHP_MEMMAP_DEVICE.
Hi David, Sorry for the late reply. On Wed, 2019-06-26 at 10:28 +0200, David Hildenbrand wrote: > On 26.06.19 10:15, Oscar Salvador wrote: > > On Wed, Jun 26, 2019 at 10:11:06AM +0200, David Hildenbrand wrote: > > > Back then, I already mentioned that we might have some users that > > > remove_memory() they never added in a granularity it wasn't > > > added. My > > > concerns back then were never fully sorted out. > > > > > > arch/powerpc/platforms/powernv/memtrace.c > > > > > > - Will remove memory in memory block size chunks it never added > > > - What if that memory resides on a DIMM added via > > > MHP_MEMMAP_DEVICE? > > > > > > Will it at least bail out? Or simply break? > > > > > > IOW: I am not yet 100% convinced that MHP_MEMMAP_DEVICE is save > > > to be > > > introduced. > > > > Uhm, I will take a closer look and see if I can clear your > > concerns. > > TBH, I did not try to use arch/powerpc/platforms/powernv/memtrace.c > > yet. > > > > I will get back to you once I tried it out. > > > > BTW, I consider the code in arch/powerpc/platforms/powernv/memtrace.c > very ugly and dangerous. Yes it would be nice to clean this up. > We should never allow to manually > offline/online pages / hack into memory block states. > > What I would want to see here is rather: > > 1. User space offlines the blocks to be used > 2. memtrace installs a hotplug notifier and hinders the blocks it > wants > to use from getting onlined. > 3. memory is not added/removed/onlined/offlined in memtrace code. > I remember looking into doing it a similar way. I can't recall the details but my issue was probably 'how does userspace indicate to the kernel that this memory being offlined should be removed'? I don't know the mm code nor how the notifiers work very well so I can't quite see how the above would work. I'm assuming memtrace would register a hotplug notifier and when memory is offlined from userspace, the callback func in memtrace would be called if the priority was high enough? But how do we know that the memory being offlined is intended for usto touch? Is there a way to offline memory from userspace not using sysfs or have I missed something in the sysfs interface? On a second read, perhaps you are assuming that memtrace is used after adding new memory at runtime? If so, that is not the case. If not, then would you be able to clarify what I'm not seeing? Thanks. > CCing the DEVs. >
On Tue, Jul 02, 2019 at 04:42:34PM +1000, Rashmica Gupta wrote: > Hi David, > > Sorry for the late reply. > > On Wed, 2019-06-26 at 10:28 +0200, David Hildenbrand wrote: > > On 26.06.19 10:15, Oscar Salvador wrote: > > > On Wed, Jun 26, 2019 at 10:11:06AM +0200, David Hildenbrand wrote: > > > > Back then, I already mentioned that we might have some users that > > > > remove_memory() they never added in a granularity it wasn't > > > > added. My > > > > concerns back then were never fully sorted out. > > > > > > > > arch/powerpc/platforms/powernv/memtrace.c > > > > > > > > - Will remove memory in memory block size chunks it never added > > > > - What if that memory resides on a DIMM added via > > > > MHP_MEMMAP_DEVICE? > > > > > > > > Will it at least bail out? Or simply break? > > > > > > > > IOW: I am not yet 100% convinced that MHP_MEMMAP_DEVICE is save > > > > to be > > > > introduced. > > > > > > Uhm, I will take a closer look and see if I can clear your > > > concerns. > > > TBH, I did not try to use arch/powerpc/platforms/powernv/memtrace.c > > > yet. > > > > > > I will get back to you once I tried it out. > > > > > > > BTW, I consider the code in arch/powerpc/platforms/powernv/memtrace.c > > very ugly and dangerous. > > Yes it would be nice to clean this up. > > > We should never allow to manually > > offline/online pages / hack into memory block states. > > > > What I would want to see here is rather: > > > > 1. User space offlines the blocks to be used > > 2. memtrace installs a hotplug notifier and hinders the blocks it > > wants > > to use from getting onlined. > > 3. memory is not added/removed/onlined/offlined in memtrace code. > > > > I remember looking into doing it a similar way. I can't recall the > details but my issue was probably 'how does userspace indicate to > the kernel that this memory being offlined should be removed'? > > I don't know the mm code nor how the notifiers work very well so I > can't quite see how the above would work. I'm assuming memtrace would > register a hotplug notifier and when memory is offlined from userspace, > the callback func in memtrace would be called if the priority was high > enough? But how do we know that the memory being offlined is intended > for usto touch? Is there a way to offline memory from userspace not > using sysfs or have I missed something in the sysfs interface? > > On a second read, perhaps you are assuming that memtrace is used after > adding new memory at runtime? If so, that is not the case. If not, then > would you be able to clarify what I'm not seeing? Hi Rashmica, let us go the easy way here. Could you please explain: 1) How memtrace works 2) Why it was designed, what is the goal of the interface? 3) When it is supposed to be used? I have seen a couple of reports in the past from people running memtrace and failing to do so sometimes, and back then I could not grasp why people was using it, or under which circumstances was nice to have. So it would be nice to have a detailed explanation from the person who wrote it. Thanks
On Tue, Jul 2, 2019 at 5:48 PM Oscar Salvador <osalvador@suse.de> wrote: > On Tue, Jul 02, 2019 at 04:42:34PM +1000, Rashmica Gupta wrote: > > Hi David, > > > > Sorry for the late reply. > > > > On Wed, 2019-06-26 at 10:28 +0200, David Hildenbrand wrote: > > > On 26.06.19 10:15, Oscar Salvador wrote: > > > > On Wed, Jun 26, 2019 at 10:11:06AM +0200, David Hildenbrand wrote: > > > > > Back then, I already mentioned that we might have some users that > > > > > remove_memory() they never added in a granularity it wasn't > > > > > added. My > > > > > concerns back then were never fully sorted out. > > > > > > > > > > arch/powerpc/platforms/powernv/memtrace.c > > > > > > > > > > - Will remove memory in memory block size chunks it never added > > > > > - What if that memory resides on a DIMM added via > > > > > MHP_MEMMAP_DEVICE? > > > > > > > > > > Will it at least bail out? Or simply break? > > > > > > > > > > IOW: I am not yet 100% convinced that MHP_MEMMAP_DEVICE is save > > > > > to be > > > > > introduced. > > > > > > > > Uhm, I will take a closer look and see if I can clear your > > > > concerns. > > > > TBH, I did not try to use arch/powerpc/platforms/powernv/memtrace.c > > > > yet. > > > > > > > > I will get back to you once I tried it out. > > > > > > > > > > BTW, I consider the code in arch/powerpc/platforms/powernv/memtrace.c > > > very ugly and dangerous. > > > > Yes it would be nice to clean this up. > > > > > We should never allow to manually > > > offline/online pages / hack into memory block states. > > > > > > What I would want to see here is rather: > > > > > > 1. User space offlines the blocks to be used > > > 2. memtrace installs a hotplug notifier and hinders the blocks it > > > wants > > > to use from getting onlined. > > > 3. memory is not added/removed/onlined/offlined in memtrace code. > > > > > > > I remember looking into doing it a similar way. I can't recall the > > details but my issue was probably 'how does userspace indicate to > > the kernel that this memory being offlined should be removed'? > > > > I don't know the mm code nor how the notifiers work very well so I > > can't quite see how the above would work. I'm assuming memtrace would > > register a hotplug notifier and when memory is offlined from userspace, > > the callback func in memtrace would be called if the priority was high > > enough? But how do we know that the memory being offlined is intended > > for usto touch? Is there a way to offline memory from userspace not > > using sysfs or have I missed something in the sysfs interface? > > > > On a second read, perhaps you are assuming that memtrace is used after > > adding new memory at runtime? If so, that is not the case. If not, then > > would you be able to clarify what I'm not seeing? > > Hi Rashmica, > > let us go the easy way here. > Could you please explain: > > Sure! > 1) How memtrace works > You write the size of the chunk of memory you want into the debugfs file and memtrace will attempt to find a contiguous section of memory of that size that can be offlined. If it finds that, then the memory is removed from the kernel's mappings. If you want a different size, then you write that to the debugsfs file and memtrace will re-add the memory it first removed and then try to offline and remove the a chunk of the new size. > 2) Why it was designed, what is the goal of the interface? > 3) When it is supposed to be used? > > There is a hardware debugging facility (htm) on some power chips. To use this you need a contiguous portion of memory for the output to be dumped to - and we obviously don't want this memory to be simultaneously used by the kernel. At boot time we can portion off a section of memory for this (and not tell the kernel about it), but sometimes you want to be able to use the hardware debugging facilities and you haven't done this and you don't want to reboot your machine - and memtrace is the solution for this. If you're curious one tool that uses this debugging facility is here: https://github.com/open-power/pdbg. Relevant files are libpdbg/htm.c and src/htm.c. I have seen a couple of reports in the past from people running memtrace > and failing to do so sometimes, and back then I could not grasp why people > was using it, or under which circumstances was nice to have. > So it would be nice to have a detailed explanation from the person who > wrote > it. > > Is that enough detail? > Thanks > > -- > Oscar Salvador > SUSE L3 >
Woops, looks like my phone doesn't send plain text emails :/ On Tue, Jul 2, 2019 at 6:52 PM Rashmica Gupta <rashmica.g@gmail.com> wrote: > > On Tue, Jul 2, 2019 at 5:48 PM Oscar Salvador <osalvador@suse.de> wrote: >> >> On Tue, Jul 02, 2019 at 04:42:34PM +1000, Rashmica Gupta wrote: >> > Hi David, >> > >> > Sorry for the late reply. >> > >> > On Wed, 2019-06-26 at 10:28 +0200, David Hildenbrand wrote: >> > > On 26.06.19 10:15, Oscar Salvador wrote: >> > > > On Wed, Jun 26, 2019 at 10:11:06AM +0200, David Hildenbrand wrote: >> > > > > Back then, I already mentioned that we might have some users that >> > > > > remove_memory() they never added in a granularity it wasn't >> > > > > added. My >> > > > > concerns back then were never fully sorted out. >> > > > > >> > > > > arch/powerpc/platforms/powernv/memtrace.c >> > > > > >> > > > > - Will remove memory in memory block size chunks it never added >> > > > > - What if that memory resides on a DIMM added via >> > > > > MHP_MEMMAP_DEVICE? >> > > > > >> > > > > Will it at least bail out? Or simply break? >> > > > > >> > > > > IOW: I am not yet 100% convinced that MHP_MEMMAP_DEVICE is save >> > > > > to be >> > > > > introduced. >> > > > >> > > > Uhm, I will take a closer look and see if I can clear your >> > > > concerns. >> > > > TBH, I did not try to use arch/powerpc/platforms/powernv/memtrace.c >> > > > yet. >> > > > >> > > > I will get back to you once I tried it out. >> > > > >> > > >> > > BTW, I consider the code in arch/powerpc/platforms/powernv/memtrace.c >> > > very ugly and dangerous. >> > >> > Yes it would be nice to clean this up. >> > >> > > We should never allow to manually >> > > offline/online pages / hack into memory block states. >> > > >> > > What I would want to see here is rather: >> > > >> > > 1. User space offlines the blocks to be used >> > > 2. memtrace installs a hotplug notifier and hinders the blocks it >> > > wants >> > > to use from getting onlined. >> > > 3. memory is not added/removed/onlined/offlined in memtrace code. >> > > >> > >> > I remember looking into doing it a similar way. I can't recall the >> > details but my issue was probably 'how does userspace indicate to >> > the kernel that this memory being offlined should be removed'? >> > >> > I don't know the mm code nor how the notifiers work very well so I >> > can't quite see how the above would work. I'm assuming memtrace would >> > register a hotplug notifier and when memory is offlined from userspace, >> > the callback func in memtrace would be called if the priority was high >> > enough? But how do we know that the memory being offlined is intended >> > for usto touch? Is there a way to offline memory from userspace not >> > using sysfs or have I missed something in the sysfs interface? >> > >> > On a second read, perhaps you are assuming that memtrace is used after >> > adding new memory at runtime? If so, that is not the case. If not, then >> > would you be able to clarify what I'm not seeing? >> >> Hi Rashmica, >> >> let us go the easy way here. >> Could you please explain: >> > > Sure! > >> >> 1) How memtrace works > > > You write the size of the chunk of memory you want into the debugfs file > and memtrace will attempt to find a contiguous section of memory of that size > that can be offlined. If it finds that, then the memory is removed from the > kernel's mappings. If you want a different size, then you write that to the > debugsfs file and memtrace will re-add the memory it first removed and then > try to offline and remove the a chunk of the new size. > > >> >> 2) Why it was designed, what is the goal of the interface? >> 3) When it is supposed to be used? >> > > There is a hardware debugging facility (htm) on some power chips. To use > this you need a contiguous portion of memory for the output to be dumped > to - and we obviously don't want this memory to be simultaneously used by > the kernel. > > At boot time we can portion off a section of memory for this (and not tell the > kernel about it), but sometimes you want to be able to use the hardware > debugging facilities and you haven't done this and you don't want to reboot > your machine - and memtrace is the solution for this. > > If you're curious one tool that uses this debugging facility is here: > https://github.com/open-power/pdbg. Relevant files are libpdbg/htm.c and src/htm.c. > > >> I have seen a couple of reports in the past from people running memtrace >> and failing to do so sometimes, and back then I could not grasp why people >> was using it, or under which circumstances was nice to have. >> So it would be nice to have a detailed explanation from the person who wrote >> it. >> > > Is that enough detail? > >> >> Thanks >> >> -- >> Oscar Salvador >> SUSE L3
On 02.07.19 08:42, Rashmica Gupta wrote: > Hi David, > > Sorry for the late reply. Hi, sorry I was on PTO :) > > On Wed, 2019-06-26 at 10:28 +0200, David Hildenbrand wrote: >> On 26.06.19 10:15, Oscar Salvador wrote: >>> On Wed, Jun 26, 2019 at 10:11:06AM +0200, David Hildenbrand wrote: >>>> Back then, I already mentioned that we might have some users that >>>> remove_memory() they never added in a granularity it wasn't >>>> added. My >>>> concerns back then were never fully sorted out. >>>> >>>> arch/powerpc/platforms/powernv/memtrace.c >>>> >>>> - Will remove memory in memory block size chunks it never added >>>> - What if that memory resides on a DIMM added via >>>> MHP_MEMMAP_DEVICE? >>>> >>>> Will it at least bail out? Or simply break? >>>> >>>> IOW: I am not yet 100% convinced that MHP_MEMMAP_DEVICE is save >>>> to be >>>> introduced. >>> >>> Uhm, I will take a closer look and see if I can clear your >>> concerns. >>> TBH, I did not try to use arch/powerpc/platforms/powernv/memtrace.c >>> yet. >>> >>> I will get back to you once I tried it out. >>> >> >> BTW, I consider the code in arch/powerpc/platforms/powernv/memtrace.c >> very ugly and dangerous. > > Yes it would be nice to clean this up. > >> We should never allow to manually >> offline/online pages / hack into memory block states. >> >> What I would want to see here is rather: >> >> 1. User space offlines the blocks to be used >> 2. memtrace installs a hotplug notifier and hinders the blocks it >> wants >> to use from getting onlined. >> 3. memory is not added/removed/onlined/offlined in memtrace code. >> > > I remember looking into doing it a similar way. I can't recall the > details but my issue was probably 'how does userspace indicate to > the kernel that this memory being offlined should be removed'? Instead of indicating a "size", indicate the offline memory blocks that the driver should use. E.g. by memory block id for each node 0:20-24,1:30-32 Of course, other interfaces might make sense. You can then start using these memory blocks and hinder them from getting onlined (as a safety net) via memory notifiers. That would at least avoid you having to call add_memory/remove_memory/offline_pages/device_online/modifying memblock states manually. (binding the memory block devices to a driver would be nicer, but the infrastructure is not really there yet - we have no such drivers in place yet) > > I don't know the mm code nor how the notifiers work very well so I > can't quite see how the above would work. I'm assuming memtrace would > register a hotplug notifier and when memory is offlined from userspace, > the callback func in memtrace would be called if the priority was high > enough? But how do we know that the memory being offlined is intended > for usto touch? Is there a way to offline memory from userspace not > using sysfs or have I missed something in the sysfs interface? The notifier would really only be used to hinder onlining as a safety net. User space prepares (offlines) the memory blocks and then tells the drivers which memory blocks to use. > > On a second read, perhaps you are assuming that memtrace is used after > adding new memory at runtime? If so, that is not the case. If not, then > would you be able to clarify what I'm not seeing? The main problem I see is that you are calling add_memory/remove_memory() on memory your device driver doesn't own. It could reside on a DIMM if I am not mistaking (or later on paravirtualized memory devices like virtio-mem if I ever get to implement them ;) ). How is it guaranteed that the memory you are allocating does not reside on a DIMM for example added via add_memory() by the ACPI driver?
On Tue, 2019-07-16 at 14:28 +0200, David Hildenbrand wrote: > On 02.07.19 08:42, Rashmica Gupta wrote: > > Hi David, > > > > Sorry for the late reply. > > Hi, > > sorry I was on PTO :) > > > On Wed, 2019-06-26 at 10:28 +0200, David Hildenbrand wrote: > > > On 26.06.19 10:15, Oscar Salvador wrote: > > > > On Wed, Jun 26, 2019 at 10:11:06AM +0200, David Hildenbrand > > > > wrote: > > > > > Back then, I already mentioned that we might have some users > > > > > that > > > > > remove_memory() they never added in a granularity it wasn't > > > > > added. My > > > > > concerns back then were never fully sorted out. > > > > > > > > > > arch/powerpc/platforms/powernv/memtrace.c > > > > > > > > > > - Will remove memory in memory block size chunks it never > > > > > added > > > > > - What if that memory resides on a DIMM added via > > > > > MHP_MEMMAP_DEVICE? > > > > > > > > > > Will it at least bail out? Or simply break? > > > > > > > > > > IOW: I am not yet 100% convinced that MHP_MEMMAP_DEVICE is > > > > > save > > > > > to be > > > > > introduced. > > > > > > > > Uhm, I will take a closer look and see if I can clear your > > > > concerns. > > > > TBH, I did not try to use > > > > arch/powerpc/platforms/powernv/memtrace.c > > > > yet. > > > > > > > > I will get back to you once I tried it out. > > > > > > > > > > BTW, I consider the code in > > > arch/powerpc/platforms/powernv/memtrace.c > > > very ugly and dangerous. > > > > Yes it would be nice to clean this up. > > > > > We should never allow to manually > > > offline/online pages / hack into memory block states. > > > > > > What I would want to see here is rather: > > > > > > 1. User space offlines the blocks to be used > > > 2. memtrace installs a hotplug notifier and hinders the blocks it > > > wants > > > to use from getting onlined. > > > 3. memory is not added/removed/onlined/offlined in memtrace code. > > > > > > > I remember looking into doing it a similar way. I can't recall the > > details but my issue was probably 'how does userspace indicate to > > the kernel that this memory being offlined should be removed'? > > Instead of indicating a "size", indicate the offline memory blocks > that > the driver should use. E.g. by memory block id for each node > > 0:20-24,1:30-32 > > Of course, other interfaces might make sense. > > You can then start using these memory blocks and hinder them from > getting onlined (as a safety net) via memory notifiers. > > That would at least avoid you having to call > add_memory/remove_memory/offline_pages/device_online/modifying > memblock > states manually. I see what you're saying and that definitely sounds safer. We would still need to call remove_memory and add_memory from memtrace as just offlining memory doesn't remove it from the linear page tables (if it's still in the page tables then hardware can prefetch it and if hardware tracing is using it then the box checkstops). > > (binding the memory block devices to a driver would be nicer, but the > infrastructure is not really there yet - we have no such drivers in > place yet) > > > I don't know the mm code nor how the notifiers work very well so I > > can't quite see how the above would work. I'm assuming memtrace > > would > > register a hotplug notifier and when memory is offlined from > > userspace, > > the callback func in memtrace would be called if the priority was > > high > > enough? But how do we know that the memory being offlined is > > intended > > for usto touch? Is there a way to offline memory from userspace not > > using sysfs or have I missed something in the sysfs interface? > > The notifier would really only be used to hinder onlining as a safety > net. User space prepares (offlines) the memory blocks and then tells > the > drivers which memory blocks to use. > > > On a second read, perhaps you are assuming that memtrace is used > > after > > adding new memory at runtime? If so, that is not the case. If not, > > then > > would you be able to clarify what I'm not seeing? > > The main problem I see is that you are calling > add_memory/remove_memory() on memory your device driver doesn't own. > It > could reside on a DIMM if I am not mistaking (or later on > paravirtualized memory devices like virtio-mem if I ever get to > implement them ;) ). This is just for baremetal/powernv so shouldn't affect virtual memory devices. > > How is it guaranteed that the memory you are allocating does not > reside > on a DIMM for example added via add_memory() by the ACPI driver? Good point. We don't have ACPI on powernv but currently this would try to remove memory from any online memory node, not just the ones that are backed by RAM. oops.
>> Of course, other interfaces might make sense. >> >> You can then start using these memory blocks and hinder them from >> getting onlined (as a safety net) via memory notifiers. >> >> That would at least avoid you having to call >> add_memory/remove_memory/offline_pages/device_online/modifying >> memblock >> states manually. > > I see what you're saying and that definitely sounds safer. > > We would still need to call remove_memory and add_memory from memtrace > as > just offlining memory doesn't remove it from the linear page tables > (if > it's still in the page tables then hardware can prefetch it and if > hardware tracing is using it then the box checkstops). That prefetching part is interesting (and nasty as well). If we could at least get rid of the manual onlining/offlining, I would be able to sleep better at night ;) One step at a time. > >> >> (binding the memory block devices to a driver would be nicer, but the >> infrastructure is not really there yet - we have no such drivers in >> place yet) >> >>> I don't know the mm code nor how the notifiers work very well so I >>> can't quite see how the above would work. I'm assuming memtrace >>> would >>> register a hotplug notifier and when memory is offlined from >>> userspace, >>> the callback func in memtrace would be called if the priority was >>> high >>> enough? But how do we know that the memory being offlined is >>> intended >>> for usto touch? Is there a way to offline memory from userspace not >>> using sysfs or have I missed something in the sysfs interface? >> >> The notifier would really only be used to hinder onlining as a safety >> net. User space prepares (offlines) the memory blocks and then tells >> the >> drivers which memory blocks to use. >> >>> On a second read, perhaps you are assuming that memtrace is used >>> after >>> adding new memory at runtime? If so, that is not the case. If not, >>> then >>> would you be able to clarify what I'm not seeing? >> >> The main problem I see is that you are calling >> add_memory/remove_memory() on memory your device driver doesn't own. >> It >> could reside on a DIMM if I am not mistaking (or later on >> paravirtualized memory devices like virtio-mem if I ever get to >> implement them ;) ). > > This is just for baremetal/powernv so shouldn't affect virtual memory > devices. Good to now. > >> >> How is it guaranteed that the memory you are allocating does not >> reside >> on a DIMM for example added via add_memory() by the ACPI driver? > > Good point. We don't have ACPI on powernv but currently this would try > to remove memory from any online memory node, not just the ones that > are backed by RAM. oops. Okay, so essentially no memory hotplug/unplug along with memtrace. (can we document that somewhere?). I think add_memory()/try_remove_memory() could be tolerable in these environments (as it's only boot memory).
On Mon, 2019-07-29 at 10:06 +0200, David Hildenbrand wrote: > > > Of course, other interfaces might make sense. > > > > > > You can then start using these memory blocks and hinder them from > > > getting onlined (as a safety net) via memory notifiers. > > > > > > That would at least avoid you having to call > > > add_memory/remove_memory/offline_pages/device_online/modifying > > > memblock > > > states manually. > > > > I see what you're saying and that definitely sounds safer. > > > > We would still need to call remove_memory and add_memory from > > memtrace > > as > > just offlining memory doesn't remove it from the linear page tables > > (if > > it's still in the page tables then hardware can prefetch it and if > > hardware tracing is using it then the box checkstops). > > That prefetching part is interesting (and nasty as well). If we could > at > least get rid of the manual onlining/offlining, I would be able to > sleep > better at night ;) One step at a time. > Ok, I'll get to that soon :) > > > (binding the memory block devices to a driver would be nicer, but > > > the > > > infrastructure is not really there yet - we have no such drivers > > > in > > > place yet) > > > > > > > I don't know the mm code nor how the notifiers work very well > > > > so I > > > > can't quite see how the above would work. I'm assuming memtrace > > > > would > > > > register a hotplug notifier and when memory is offlined from > > > > userspace, > > > > the callback func in memtrace would be called if the priority > > > > was > > > > high > > > > enough? But how do we know that the memory being offlined is > > > > intended > > > > for usto touch? Is there a way to offline memory from userspace > > > > not > > > > using sysfs or have I missed something in the sysfs interface? > > > > > > The notifier would really only be used to hinder onlining as a > > > safety > > > net. User space prepares (offlines) the memory blocks and then > > > tells > > > the > > > drivers which memory blocks to use. > > > > > > > On a second read, perhaps you are assuming that memtrace is > > > > used > > > > after > > > > adding new memory at runtime? If so, that is not the case. If > > > > not, > > > > then > > > > would you be able to clarify what I'm not seeing? > > > > > > The main problem I see is that you are calling > > > add_memory/remove_memory() on memory your device driver doesn't > > > own. > > > It > > > could reside on a DIMM if I am not mistaking (or later on > > > paravirtualized memory devices like virtio-mem if I ever get to > > > implement them ;) ). > > > > This is just for baremetal/powernv so shouldn't affect virtual > > memory > > devices. > > Good to now. > > > > How is it guaranteed that the memory you are allocating does not > > > reside > > > on a DIMM for example added via add_memory() by the ACPI driver? > > > > Good point. We don't have ACPI on powernv but currently this would > > try > > to remove memory from any online memory node, not just the ones > > that > > are backed by RAM. oops. > > Okay, so essentially no memory hotplug/unplug along with memtrace. > (can > we document that somewhere?). I think > add_memory()/try_remove_memory() > could be tolerable in these environments (as it's only boot memory). > Sure thing.
On Mon, 2019-07-29 at 10:06 +0200, David Hildenbrand wrote: > > > Of course, other interfaces might make sense. > > > > > > You can then start using these memory blocks and hinder them from > > > getting onlined (as a safety net) via memory notifiers. > > > > > > That would at least avoid you having to call > > > add_memory/remove_memory/offline_pages/device_online/modifying > > > memblock > > > states manually. > > > > I see what you're saying and that definitely sounds safer. > > > > We would still need to call remove_memory and add_memory from > > memtrace > > as > > just offlining memory doesn't remove it from the linear page tables > > (if > > it's still in the page tables then hardware can prefetch it and if > > hardware tracing is using it then the box checkstops). > > That prefetching part is interesting (and nasty as well). If we could > at > least get rid of the manual onlining/offlining, I would be able to > sleep > better at night ;) One step at a time. > What are your thoughts on adding remove to state_store in drivers/base/memory.c? And an accompanying add? So then userspace could do "echo remove > memory34/state"? Then most of the memtrace code could be moved to a userspace tool. The only bit that we would need to keep in the kernel is setting up debugfs files in memtrace_init_debugfs.
On 31.07.19 04:21, Rashmica Gupta wrote: > On Mon, 2019-07-29 at 10:06 +0200, David Hildenbrand wrote: >>>> Of course, other interfaces might make sense. >>>> >>>> You can then start using these memory blocks and hinder them from >>>> getting onlined (as a safety net) via memory notifiers. >>>> >>>> That would at least avoid you having to call >>>> add_memory/remove_memory/offline_pages/device_online/modifying >>>> memblock >>>> states manually. >>> >>> I see what you're saying and that definitely sounds safer. >>> >>> We would still need to call remove_memory and add_memory from >>> memtrace >>> as >>> just offlining memory doesn't remove it from the linear page tables >>> (if >>> it's still in the page tables then hardware can prefetch it and if >>> hardware tracing is using it then the box checkstops). >> >> That prefetching part is interesting (and nasty as well). If we could >> at >> least get rid of the manual onlining/offlining, I would be able to >> sleep >> better at night ;) One step at a time. >> > > What are your thoughts on adding remove to state_store in > drivers/base/memory.c? And an accompanying add? So then userspace could > do "echo remove > memory34/state"? I consider such an interface dangerous (removing random memory you don't own, especially in the context of Oscars work some blocks would suddenly not be removable again) and only of limited used. The requirement of memtrace is really special, and it only works somewhat reliably with boot memory. FWIW, we do have a "probe" interface on some architectures but decided that a "remove" interface belongs into a debug/test kernel module. The memory block device API is already messed up enough (e.g., "removable" property which doesn't indicate at all if memory can be removed, only if it could be offlined) - we decided to keep changes at a minimum for now - rather clean up than add new stuff. > Then most of the memtrace code could be moved to a userspace tool. The > only bit that we would need to keep in the kernel is setting up debugfs > files in memtrace_init_debugfs. The nice thing is, with remove_memory() you will now get an error in case all memory blocks are not offline yet. So it only boils down to calling add_memory()/remove_memory() without even checking the state of the blocks. (only nids have to be handled manually correctly)
On Tue 02-07-19 18:52:01, Rashmica Gupta wrote: [...] > > 2) Why it was designed, what is the goal of the interface? > > 3) When it is supposed to be used? > > > > > There is a hardware debugging facility (htm) on some power chips. To use > this you need a contiguous portion of memory for the output to be dumped > to - and we obviously don't want this memory to be simultaneously used by > the kernel. How much memory are we talking about here? Just curious.
On Wed, 2019-07-31 at 14:08 +0200, Michal Hocko wrote: > On Tue 02-07-19 18:52:01, Rashmica Gupta wrote: > [...] > > > 2) Why it was designed, what is the goal of the interface? > > > 3) When it is supposed to be used? > > > > > > > > There is a hardware debugging facility (htm) on some power chips. > > To use > > this you need a contiguous portion of memory for the output to be > > dumped > > to - and we obviously don't want this memory to be simultaneously > > used by > > the kernel. > > How much memory are we talking about here? Just curious. From what I've seen a couple of GB per node, so maybe 2-10GB total.
On Thu 01-08-19 09:06:40, Rashmica Gupta wrote: > On Wed, 2019-07-31 at 14:08 +0200, Michal Hocko wrote: > > On Tue 02-07-19 18:52:01, Rashmica Gupta wrote: > > [...] > > > > 2) Why it was designed, what is the goal of the interface? > > > > 3) When it is supposed to be used? > > > > > > > > > > > There is a hardware debugging facility (htm) on some power chips. > > > To use > > > this you need a contiguous portion of memory for the output to be > > > dumped > > > to - and we obviously don't want this memory to be simultaneously > > > used by > > > the kernel. > > > > How much memory are we talking about here? Just curious. > > From what I've seen a couple of GB per node, so maybe 2-10GB total. OK, that is really a lot to keep around unused just in case the debugging is going to be used. I am still not sure the current approach of (ab)using memory hotplug is ideal. Sure there is some overlap but you shouldn't really need to offline the required memory range at all. All you need is to isolate the memory from any existing user and the page allocator. Have you checked alloc_contig_range?
On 01.08.19 09:17, Michal Hocko wrote: > On Thu 01-08-19 09:06:40, Rashmica Gupta wrote: >> On Wed, 2019-07-31 at 14:08 +0200, Michal Hocko wrote: >>> On Tue 02-07-19 18:52:01, Rashmica Gupta wrote: >>> [...] >>>>> 2) Why it was designed, what is the goal of the interface? >>>>> 3) When it is supposed to be used? >>>>> >>>>> >>>> There is a hardware debugging facility (htm) on some power chips. >>>> To use >>>> this you need a contiguous portion of memory for the output to be >>>> dumped >>>> to - and we obviously don't want this memory to be simultaneously >>>> used by >>>> the kernel. >>> >>> How much memory are we talking about here? Just curious. >> >> From what I've seen a couple of GB per node, so maybe 2-10GB total. > > OK, that is really a lot to keep around unused just in case the > debugging is going to be used. > > I am still not sure the current approach of (ab)using memory hotplug is > ideal. Sure there is some overlap but you shouldn't really need to > offline the required memory range at all. All you need is to isolate the > memory from any existing user and the page allocator. Have you checked > alloc_contig_range? > Rashmica mentioned somewhere in this thread that the virtual mapping must not be in place, otherwise the HW might prefetch some of this memory, leading to errors with memtrace (which checks that in HW).
On Thu 01-08-19 09:18:47, David Hildenbrand wrote: > On 01.08.19 09:17, Michal Hocko wrote: > > On Thu 01-08-19 09:06:40, Rashmica Gupta wrote: > >> On Wed, 2019-07-31 at 14:08 +0200, Michal Hocko wrote: > >>> On Tue 02-07-19 18:52:01, Rashmica Gupta wrote: > >>> [...] > >>>>> 2) Why it was designed, what is the goal of the interface? > >>>>> 3) When it is supposed to be used? > >>>>> > >>>>> > >>>> There is a hardware debugging facility (htm) on some power chips. > >>>> To use > >>>> this you need a contiguous portion of memory for the output to be > >>>> dumped > >>>> to - and we obviously don't want this memory to be simultaneously > >>>> used by > >>>> the kernel. > >>> > >>> How much memory are we talking about here? Just curious. > >> > >> From what I've seen a couple of GB per node, so maybe 2-10GB total. > > > > OK, that is really a lot to keep around unused just in case the > > debugging is going to be used. > > > > I am still not sure the current approach of (ab)using memory hotplug is > > ideal. Sure there is some overlap but you shouldn't really need to > > offline the required memory range at all. All you need is to isolate the > > memory from any existing user and the page allocator. Have you checked > > alloc_contig_range? > > > > Rashmica mentioned somewhere in this thread that the virtual mapping > must not be in place, otherwise the HW might prefetch some of this > memory, leading to errors with memtrace (which checks that in HW). Does anything prevent from unmapping the pfn range from the direct mapping?
On 01.08.19 09:24, Michal Hocko wrote: > On Thu 01-08-19 09:18:47, David Hildenbrand wrote: >> On 01.08.19 09:17, Michal Hocko wrote: >>> On Thu 01-08-19 09:06:40, Rashmica Gupta wrote: >>>> On Wed, 2019-07-31 at 14:08 +0200, Michal Hocko wrote: >>>>> On Tue 02-07-19 18:52:01, Rashmica Gupta wrote: >>>>> [...] >>>>>>> 2) Why it was designed, what is the goal of the interface? >>>>>>> 3) When it is supposed to be used? >>>>>>> >>>>>>> >>>>>> There is a hardware debugging facility (htm) on some power chips. >>>>>> To use >>>>>> this you need a contiguous portion of memory for the output to be >>>>>> dumped >>>>>> to - and we obviously don't want this memory to be simultaneously >>>>>> used by >>>>>> the kernel. >>>>> >>>>> How much memory are we talking about here? Just curious. >>>> >>>> From what I've seen a couple of GB per node, so maybe 2-10GB total. >>> >>> OK, that is really a lot to keep around unused just in case the >>> debugging is going to be used. >>> >>> I am still not sure the current approach of (ab)using memory hotplug is >>> ideal. Sure there is some overlap but you shouldn't really need to >>> offline the required memory range at all. All you need is to isolate the >>> memory from any existing user and the page allocator. Have you checked >>> alloc_contig_range? >>> >> >> Rashmica mentioned somewhere in this thread that the virtual mapping >> must not be in place, otherwise the HW might prefetch some of this >> memory, leading to errors with memtrace (which checks that in HW). > > Does anything prevent from unmapping the pfn range from the direct > mapping? I am not sure about the implications of having pfn_valid()/pfn_present()/pfn_online() return true but accessing it results in crashes. (suspend, kdump, whatever other technology touches online memory) (sounds more like a hack to me than just going ahead and removing/readding the memory via a clean interface we have)
On 01.08.19 09:26, David Hildenbrand wrote: > On 01.08.19 09:24, Michal Hocko wrote: >> On Thu 01-08-19 09:18:47, David Hildenbrand wrote: >>> On 01.08.19 09:17, Michal Hocko wrote: >>>> On Thu 01-08-19 09:06:40, Rashmica Gupta wrote: >>>>> On Wed, 2019-07-31 at 14:08 +0200, Michal Hocko wrote: >>>>>> On Tue 02-07-19 18:52:01, Rashmica Gupta wrote: >>>>>> [...] >>>>>>>> 2) Why it was designed, what is the goal of the interface? >>>>>>>> 3) When it is supposed to be used? >>>>>>>> >>>>>>>> >>>>>>> There is a hardware debugging facility (htm) on some power chips. >>>>>>> To use >>>>>>> this you need a contiguous portion of memory for the output to be >>>>>>> dumped >>>>>>> to - and we obviously don't want this memory to be simultaneously >>>>>>> used by >>>>>>> the kernel. >>>>>> >>>>>> How much memory are we talking about here? Just curious. >>>>> >>>>> From what I've seen a couple of GB per node, so maybe 2-10GB total. >>>> >>>> OK, that is really a lot to keep around unused just in case the >>>> debugging is going to be used. >>>> >>>> I am still not sure the current approach of (ab)using memory hotplug is >>>> ideal. Sure there is some overlap but you shouldn't really need to >>>> offline the required memory range at all. All you need is to isolate the >>>> memory from any existing user and the page allocator. Have you checked >>>> alloc_contig_range? >>>> >>> >>> Rashmica mentioned somewhere in this thread that the virtual mapping >>> must not be in place, otherwise the HW might prefetch some of this >>> memory, leading to errors with memtrace (which checks that in HW). >> >> Does anything prevent from unmapping the pfn range from the direct >> mapping? > > I am not sure about the implications of having > pfn_valid()/pfn_present()/pfn_online() return true but accessing it > results in crashes. (suspend, kdump, whatever other technology touches > online memory) (oneidea: we could of course go ahead and mark the pages PG_offline before unmapping the pfn range to work around these issues) > > (sounds more like a hack to me than just going ahead and > removing/readding the memory via a clean interface we have) >
On Thu 01-08-19 09:26:35, David Hildenbrand wrote: > On 01.08.19 09:24, Michal Hocko wrote: > > On Thu 01-08-19 09:18:47, David Hildenbrand wrote: > >> On 01.08.19 09:17, Michal Hocko wrote: > >>> On Thu 01-08-19 09:06:40, Rashmica Gupta wrote: > >>>> On Wed, 2019-07-31 at 14:08 +0200, Michal Hocko wrote: > >>>>> On Tue 02-07-19 18:52:01, Rashmica Gupta wrote: > >>>>> [...] > >>>>>>> 2) Why it was designed, what is the goal of the interface? > >>>>>>> 3) When it is supposed to be used? > >>>>>>> > >>>>>>> > >>>>>> There is a hardware debugging facility (htm) on some power chips. > >>>>>> To use > >>>>>> this you need a contiguous portion of memory for the output to be > >>>>>> dumped > >>>>>> to - and we obviously don't want this memory to be simultaneously > >>>>>> used by > >>>>>> the kernel. > >>>>> > >>>>> How much memory are we talking about here? Just curious. > >>>> > >>>> From what I've seen a couple of GB per node, so maybe 2-10GB total. > >>> > >>> OK, that is really a lot to keep around unused just in case the > >>> debugging is going to be used. > >>> > >>> I am still not sure the current approach of (ab)using memory hotplug is > >>> ideal. Sure there is some overlap but you shouldn't really need to > >>> offline the required memory range at all. All you need is to isolate the > >>> memory from any existing user and the page allocator. Have you checked > >>> alloc_contig_range? > >>> > >> > >> Rashmica mentioned somewhere in this thread that the virtual mapping > >> must not be in place, otherwise the HW might prefetch some of this > >> memory, leading to errors with memtrace (which checks that in HW). > > > > Does anything prevent from unmapping the pfn range from the direct > > mapping? > > I am not sure about the implications of having > pfn_valid()/pfn_present()/pfn_online() return true but accessing it > results in crashes. (suspend, kdump, whatever other technology touches > online memory) If those pages are marked as Reserved then nobody should be touching them anyway. > (sounds more like a hack to me than just going ahead and > removing/readding the memory via a clean interface we have) Right, but the interface that we have is quite restricted in what it can really offline.
On Thu 01-08-19 09:31:09, David Hildenbrand wrote: > On 01.08.19 09:26, David Hildenbrand wrote: [...] > > I am not sure about the implications of having > > pfn_valid()/pfn_present()/pfn_online() return true but accessing it > > results in crashes. (suspend, kdump, whatever other technology touches > > online memory) > > (oneidea: we could of course go ahead and mark the pages PG_offline > before unmapping the pfn range to work around these issues) PG_reserved and an elevated reference count should be enough to drive any pfn walker out. Pfn walkers shouldn't touch any page unless they know and recognize their type.
On Thu 01-08-19 09:39:57, Michal Hocko wrote: > On Thu 01-08-19 09:31:09, David Hildenbrand wrote: > > On 01.08.19 09:26, David Hildenbrand wrote: > [...] > > > I am not sure about the implications of having > > > pfn_valid()/pfn_present()/pfn_online() return true but accessing it > > > results in crashes. (suspend, kdump, whatever other technology touches > > > online memory) > > > > (oneidea: we could of course go ahead and mark the pages PG_offline > > before unmapping the pfn range to work around these issues) > > PG_reserved and an elevated reference count should be enough to drive > any pfn walker out. Pfn walkers shouldn't touch any page unless they > know and recognize their type. Btw. this shouldn't be much different from DEBUG_PAGE_ALLOC in principle. The memory is valid, but not mapped to the kernel virtual space. Nobody should be really touching it anyway.
On 01.08.19 09:34, Michal Hocko wrote: > On Thu 01-08-19 09:26:35, David Hildenbrand wrote: >> On 01.08.19 09:24, Michal Hocko wrote: >>> On Thu 01-08-19 09:18:47, David Hildenbrand wrote: >>>> On 01.08.19 09:17, Michal Hocko wrote: >>>>> On Thu 01-08-19 09:06:40, Rashmica Gupta wrote: >>>>>> On Wed, 2019-07-31 at 14:08 +0200, Michal Hocko wrote: >>>>>>> On Tue 02-07-19 18:52:01, Rashmica Gupta wrote: >>>>>>> [...] >>>>>>>>> 2) Why it was designed, what is the goal of the interface? >>>>>>>>> 3) When it is supposed to be used? >>>>>>>>> >>>>>>>>> >>>>>>>> There is a hardware debugging facility (htm) on some power chips. >>>>>>>> To use >>>>>>>> this you need a contiguous portion of memory for the output to be >>>>>>>> dumped >>>>>>>> to - and we obviously don't want this memory to be simultaneously >>>>>>>> used by >>>>>>>> the kernel. >>>>>>> >>>>>>> How much memory are we talking about here? Just curious. >>>>>> >>>>>> From what I've seen a couple of GB per node, so maybe 2-10GB total. >>>>> >>>>> OK, that is really a lot to keep around unused just in case the >>>>> debugging is going to be used. >>>>> >>>>> I am still not sure the current approach of (ab)using memory hotplug is >>>>> ideal. Sure there is some overlap but you shouldn't really need to >>>>> offline the required memory range at all. All you need is to isolate the >>>>> memory from any existing user and the page allocator. Have you checked >>>>> alloc_contig_range? >>>>> >>>> >>>> Rashmica mentioned somewhere in this thread that the virtual mapping >>>> must not be in place, otherwise the HW might prefetch some of this >>>> memory, leading to errors with memtrace (which checks that in HW). >>> >>> Does anything prevent from unmapping the pfn range from the direct >>> mapping? >> >> I am not sure about the implications of having >> pfn_valid()/pfn_present()/pfn_online() return true but accessing it >> results in crashes. (suspend, kdump, whatever other technology touches >> online memory) > > If those pages are marked as Reserved then nobody should be touching > them anyway. Which is not true as I remember we already discussed - I even documented what PG_reserved can mean after that discussion in page-flags.h (e.g., memmap of boot memory) - that's why we introduced PG_offline after all.
On Thu 01-08-19 09:50:29, David Hildenbrand wrote: > On 01.08.19 09:34, Michal Hocko wrote: > > On Thu 01-08-19 09:26:35, David Hildenbrand wrote: > >> On 01.08.19 09:24, Michal Hocko wrote: > >>> On Thu 01-08-19 09:18:47, David Hildenbrand wrote: > >>>> On 01.08.19 09:17, Michal Hocko wrote: > >>>>> On Thu 01-08-19 09:06:40, Rashmica Gupta wrote: > >>>>>> On Wed, 2019-07-31 at 14:08 +0200, Michal Hocko wrote: > >>>>>>> On Tue 02-07-19 18:52:01, Rashmica Gupta wrote: > >>>>>>> [...] > >>>>>>>>> 2) Why it was designed, what is the goal of the interface? > >>>>>>>>> 3) When it is supposed to be used? > >>>>>>>>> > >>>>>>>>> > >>>>>>>> There is a hardware debugging facility (htm) on some power chips. > >>>>>>>> To use > >>>>>>>> this you need a contiguous portion of memory for the output to be > >>>>>>>> dumped > >>>>>>>> to - and we obviously don't want this memory to be simultaneously > >>>>>>>> used by > >>>>>>>> the kernel. > >>>>>>> > >>>>>>> How much memory are we talking about here? Just curious. > >>>>>> > >>>>>> From what I've seen a couple of GB per node, so maybe 2-10GB total. > >>>>> > >>>>> OK, that is really a lot to keep around unused just in case the > >>>>> debugging is going to be used. > >>>>> > >>>>> I am still not sure the current approach of (ab)using memory hotplug is > >>>>> ideal. Sure there is some overlap but you shouldn't really need to > >>>>> offline the required memory range at all. All you need is to isolate the > >>>>> memory from any existing user and the page allocator. Have you checked > >>>>> alloc_contig_range? > >>>>> > >>>> > >>>> Rashmica mentioned somewhere in this thread that the virtual mapping > >>>> must not be in place, otherwise the HW might prefetch some of this > >>>> memory, leading to errors with memtrace (which checks that in HW). > >>> > >>> Does anything prevent from unmapping the pfn range from the direct > >>> mapping? > >> > >> I am not sure about the implications of having > >> pfn_valid()/pfn_present()/pfn_online() return true but accessing it > >> results in crashes. (suspend, kdump, whatever other technology touches > >> online memory) > > > > If those pages are marked as Reserved then nobody should be touching > > them anyway. > > Which is not true as I remember we already discussed - I even documented > what PG_reserved can mean after that discussion in page-flags.h (e.g., > memmap of boot memory) - that's why we introduced PG_offline after all. Sorry, my statement was imprecise. What I meant is what we have documented: * PG_reserved is set for special pages. The "struct page" of such a page * should in general not be touched (e.g. set dirty) except by its owner. the owner part is important.
On 01.08.19 09:48, Michal Hocko wrote: > On Thu 01-08-19 09:39:57, Michal Hocko wrote: >> On Thu 01-08-19 09:31:09, David Hildenbrand wrote: >>> On 01.08.19 09:26, David Hildenbrand wrote: >> [...] >>>> I am not sure about the implications of having >>>> pfn_valid()/pfn_present()/pfn_online() return true but accessing it >>>> results in crashes. (suspend, kdump, whatever other technology touches >>>> online memory) >>> >>> (oneidea: we could of course go ahead and mark the pages PG_offline >>> before unmapping the pfn range to work around these issues) >> >> PG_reserved and an elevated reference count should be enough to drive >> any pfn walker out. Pfn walkers shouldn't touch any page unless they >> know and recognize their type. > > Btw. this shouldn't be much different from DEBUG_PAGE_ALLOC in > principle. The memory is valid, but not mapped to the kernel virtual > space. Nobody should be really touching it anyway. > I guess that could work (I am happy with anything that gets rid of offline_pages()/device_online() here :D ). So for each node, alloc_contig_range() (if I remember correctly, all pages in the range have to be in the same zone), set them PG_reserved (+ maybe something else, we'll have to see). Then, unmap them. The reverse when freeing the memory. Guess this should leave the current user space interface unmodified. I can see that guard pages use a special page type (PageGuard), not sure if something like that is really required. We'll have to see.
Hi, It has been while since I sent previous version [1]. In this version I added some feedback I got back then, like letting the caller decide whether he wants allocating per memory block or per memory range (patch#2), and having the chance to disable vmemmap when users want to expose all hotpluggable memory to userspace (patch#5). [Testing] While I could test last version on powerpc, and Huawei's fellows helped me out testing it on arm64, this time I could only test it on x86_64. The codebase is quite the same, so I would not expect surprises. - x86_64: small and large memblocks (128MB, 1G and 2G) - Kernel module that adds memory spanning multiple memblocks and remove that memory in a different granularity. So far, only acpi memory hotplug uses the new flag. The other callers can be changed depending on their needs. Of course, more testing and feedback is appreciated. [Coverletter] This is another step to make memory hotplug more usable. The primary goal of this patchset is to reduce memory overhead of the hot-added memory (at least for SPARSEMEM_VMEMMAP memory model). The current way we use to populate memmap (struct page array) has two main drawbacks: a) it consumes an additional memory until the hotadded memory itself is onlined and b) memmap might end up on a different numa node which is especially true for movable_node configuration. a) it is a problem especially for memory hotplug based memory "ballooning" solutions when the delay between physical memory hotplug and the onlining can lead to OOM and that led to introduction of hacks like auto onlining (see 31bc3858ea3e ("memory-hotplug: add automatic onlining policy for the newly added memory")). b) can have performance drawbacks. Another minor case is that I have seen hot-add operations failing on archs because they were running out of order-x pages. E.g On powerpc, in certain configurations, we use order-8 pages, and given 64KB base pagesize, that is 16MB. If we run out of those, we just fail the operation and we cannot add more memory. We could fallback to base pages as x86_64 does, but we can do better. One way to mitigate all these issues is to simply allocate memmap array (which is the largest memory footprint of the physical memory hotplug) from the hot-added memory itself. SPARSEMEM_VMEMMAP memory model allows us to map any pfn range so the memory doesn't need to be online to be usable for the array. See patch 3 for more details. This feature is only usable when CONFIG_SPARSEMEM_VMEMMAP is set. [Overall design]: Implementation wise we reuse vmem_altmap infrastructure to override the default allocator used by vmemap_populate. Once the memmap is allocated we need a way to mark altmap pfns used for the allocation. If MHP_MEMMAP_{DEVICE,MEMBLOCK} flag was passed, we set up the layout of the altmap structure at the beginning of __add_pages(), and then we call mark_vmemmap_pages(). The flags are either MHP_MEMMAP_DEVICE or MHP_MEMMAP_MEMBLOCK, and only differ in the way they allocate vmemmap pages within the memory blocks. MHP_MEMMAP_MEMBLOCK: - With this flag, we will allocate vmemmap pages in each memory block. This means that if we hot-add a range that spans multiple memory blocks, we will use the beginning of each memory block for the vmemmap pages. This strategy is good for cases where the caller wants the flexiblity to hot-remove memory in a different granularity than when it was added. MHP_MEMMAP_DEVICE: - With this flag, we will store all vmemmap pages at the beginning of hot-added memory. So it is a tradeoff of flexiblity vs contigous memory. More info on the above can be found in patch#2. Depending on which flag is passed (MHP_MEMMAP_DEVICE or MHP_MEMMAP_MEMBLOCK), mark_vmemmap_pages() gets called at a different stage. With MHP_MEMMAP_MEMBLOCK, we call it once we have populated the sections fitting in a single memblock, while with MHP_MEMMAP_DEVICE we wait until all sections have been populated. mark_vmemmap_pages() marks the pages as vmemmap and sets some metadata: The current layout of the Vmemmap pages are: [Head->refcount] : Nr sections used by this altmap [Head->private] : Nr of vmemmap pages [Tail->freelist] : Pointer to the head page This is done to easy the computation we need in some places. E.g: Example 1) We hot-add 1GB on x86_64 (memory block 128MB) using MHP_MEMMAP_DEVICE: head->_refcount = 8 sections head->private = 4096 vmemmap pages tail's->freelist = head Example 2) We hot-add 1GB on x86_64 using MHP_MEMMAP_MEMBLOCK: [at the beginning of each memblock] head->_refcount = 1 section head->private = 512 vmemmap pages tail's->freelist = head We have the refcount because when using MHP_MEMMAP_DEVICE, we need to know how much do we have to defer the call to vmemmap_free(). The thing is that the first pages of the hot-added range are used to create the memmap mapping, so we cannot remove those first, otherwise we would blow up when accessing the other pages. What we do is that since when we hot-remove a memory-range, sections are being removed sequentially, we wait until we hit the last section, and then we free the hole range to vmemmap_free backwards. We know that it is the last section because in every pass we decrease head->_refcount, and when it reaches 0, we got our last section. We also have to be careful about those pages during online and offline operations. They are simply skipped, so online will keep them reserved and so unusable for any other purpose and offline ignores them so they do not block the offline operation. One thing worth mention is that vmemmap pages residing in movable memory is not a show-stopper for that memory to be offlined/migrated away. Vmemmap pages are just ignored in that case and they stick around until sections referred by those vmemmap pages are hot-removed. [1] https://patchwork.kernel.org/cover/10875017/ Oscar Salvador (5): drivers/base/memory: Remove unneeded check in remove_memory_block_devices mm,memory_hotplug: Introduce MHP_VMEMMAP_FLAGS mm,memory_hotplug: Introduce Vmemmap page helpers mm,memory_hotplug: allocate memmap from the added memory range for sparse-vmemmap mm,memory_hotplug: Allow userspace to enable/disable vmemmap arch/arm64/mm/mmu.c | 5 +- arch/powerpc/mm/init_64.c | 7 ++ arch/s390/mm/init.c | 6 ++ arch/x86/mm/init_64.c | 10 +++ drivers/acpi/acpi_memhotplug.c | 2 +- drivers/base/memory.c | 41 +++++++++-- drivers/dax/kmem.c | 2 +- drivers/hv/hv_balloon.c | 2 +- drivers/s390/char/sclp_cmd.c | 2 +- drivers/xen/balloon.c | 2 +- include/linux/memory_hotplug.h | 31 ++++++++- include/linux/memremap.h | 2 +- include/linux/page-flags.h | 34 +++++++++ mm/compaction.c | 7 ++ mm/memory_hotplug.c | 152 ++++++++++++++++++++++++++++++++++------- mm/page_alloc.c | 22 +++++- mm/page_isolation.c | 14 +++- mm/sparse.c | 93 +++++++++++++++++++++++++ mm/util.c | 2 + 19 files changed, 394 insertions(+), 42 deletions(-)