| Message ID | 20230613-vv-kmem_memmap-v1-0-f6de9c6af2c6@intel.com |
|---|---|
| Headers | show |
| Series | mm: use memmap_on_memory semantics for dax/kmem | expand |
On 16.06.23 00:00, Vishal Verma wrote: > The dax/kmem driver can potentially hot-add large amounts of memory > originating from CXL memory expanders, or NVDIMMs, or other 'device > memories'. There is a chance there isn't enough regular system memory > available to fit ythe memmap for this new memory. It's therefore > desirable, if all other conditions are met, for the kmem managed memory > to place its memmap on the newly added memory itself. > > Arrange for this by first allowing for a module parameter override for > the mhp_supports_memmap_on_memory() test using a flag, adjusting the > only other caller of this interface in dirvers/acpi/acpi_memoryhotplug.c, > exporting the symbol so it can be called by kmem.c, and finally changing > the kmem driver to add_memory() in chunks of memory_block_size_bytes(). 1) Why is the override a requirement here? Just let the admin configure it then then add conditional support for kmem. 2) I recall that there are cases where we don't want the memmap to land on slow memory (which online_movable would achieve). Just imagine the slow PMEM case. So this might need another configuration knob on the kmem side. I recall some discussions on doing that chunk handling internally (so kmem can just perform one add_memory() and we'd split that up internally).
On Fri, 2023-06-16 at 09:44 +0200, David Hildenbrand wrote: > On 16.06.23 00:00, Vishal Verma wrote: > > The dax/kmem driver can potentially hot-add large amounts of memory > > originating from CXL memory expanders, or NVDIMMs, or other 'device > > memories'. There is a chance there isn't enough regular system memory > > available to fit ythe memmap for this new memory. It's therefore > > desirable, if all other conditions are met, for the kmem managed memory > > to place its memmap on the newly added memory itself. > > > > Arrange for this by first allowing for a module parameter override for > > the mhp_supports_memmap_on_memory() test using a flag, adjusting the > > only other caller of this interface in dirvers/acpi/acpi_memoryhotplug.c, > > exporting the symbol so it can be called by kmem.c, and finally changing > > the kmem driver to add_memory() in chunks of memory_block_size_bytes(). > > 1) Why is the override a requirement here? Just let the admin configure > it then then add conditional support for kmem. Configure it in the current way using the module parameter to memory_hotplug? The whole module param check feels a bit awkward, especially since memory_hotplug is builtin, the only way to supply the param is on the kernel command line as opposed to a modprobe config. The goal with extending mhp_supports_memmap_on_memory() to check for support with or without consideration for the module param is that it makes it a bit more flexible for callers like kmem. > 2) I recall that there are cases where we don't want the memmap to land > on slow memory (which online_movable would achieve). Just imagine the > slow PMEM case. So this might need another configuration knob on the > kmem side. > > I recall some discussions on doing that chunk handling internally (so > kmem can just perform one add_memory() and we'd split that up internally). > Another config knob isn't unreasonable - though the thinking in making this behavior the new default policy was that with CXL based memory expanders, the performance delta from main memory wouldn't be as big as the pmem - main memory delta. With pmem devices being phased out, it's not clear we'd need a knob, and it can always be added if it ends up becoming necessary. The other comments about doing the per-memblock loop internally, and fixing up the removal paths all sound good, and I've started reworking those - thanks for taking a look!
On 21.06.23 21:32, Verma, Vishal L wrote: > On Fri, 2023-06-16 at 09:44 +0200, David Hildenbrand wrote: >> On 16.06.23 00:00, Vishal Verma wrote: >>> The dax/kmem driver can potentially hot-add large amounts of memory >>> originating from CXL memory expanders, or NVDIMMs, or other 'device >>> memories'. There is a chance there isn't enough regular system memory >>> available to fit ythe memmap for this new memory. It's therefore >>> desirable, if all other conditions are met, for the kmem managed memory >>> to place its memmap on the newly added memory itself. >>> >>> Arrange for this by first allowing for a module parameter override for >>> the mhp_supports_memmap_on_memory() test using a flag, adjusting the >>> only other caller of this interface in dirvers/acpi/acpi_memoryhotplug.c, >>> exporting the symbol so it can be called by kmem.c, and finally changing >>> the kmem driver to add_memory() in chunks of memory_block_size_bytes(). >> >> 1) Why is the override a requirement here? Just let the admin configure >> it then then add conditional support for kmem. > > Configure it in the current way using the module parameter to > memory_hotplug? The whole module param check feels a bit awkward, > especially since memory_hotplug is builtin, the only way to supply the > param is on the kernel command line as opposed to a modprobe config. Yes, and that's nothing special. Runtime toggling is not implemented. > > The goal with extending mhp_supports_memmap_on_memory() to check for > support with or without consideration for the module param is that it > makes it a bit more flexible for callers like kmem. Not convinced yet that the global parameter should be bypassed TBH. And if so, this should be a separate patch on top that is completely optional for the remainder of the series. In any case, there has to be some admin control over that, because 1) You usually don't want vmemmap on potentially slow memory 2) Using memmap-on-memory prohibits gigantic pages from forming on that memory (when runtime-allocating them). So "just doing that" without any config knob is problematic.
David Hildenbrand <david@redhat.com> writes: > On 16.06.23 00:00, Vishal Verma wrote: >> The dax/kmem driver can potentially hot-add large amounts of memory >> originating from CXL memory expanders, or NVDIMMs, or other 'device >> memories'. There is a chance there isn't enough regular system memory >> available to fit ythe memmap for this new memory. It's therefore >> desirable, if all other conditions are met, for the kmem managed memory >> to place its memmap on the newly added memory itself. >> >> Arrange for this by first allowing for a module parameter override for >> the mhp_supports_memmap_on_memory() test using a flag, adjusting the >> only other caller of this interface in dirvers/acpi/acpi_memoryhotplug.c, >> exporting the symbol so it can be called by kmem.c, and finally changing >> the kmem driver to add_memory() in chunks of memory_block_size_bytes(). > > 1) Why is the override a requirement here? Just let the admin > configure it then then add conditional support for kmem. > > 2) I recall that there are cases where we don't want the memmap to > land on slow memory (which online_movable would achieve). Just imagine > the slow PMEM case. So this might need another configuration knob on > the kmem side. From my memory, the case where you don't want the memmap to land on *persistent memory* is when the device is small (such as NVDIMM-N), and you want to reserve as much space as possible for the application data. This has nothing to do with the speed of access. -Jeff
On 13.07.23 21:12, Jeff Moyer wrote: > David Hildenbrand <david@redhat.com> writes: > >> On 16.06.23 00:00, Vishal Verma wrote: >>> The dax/kmem driver can potentially hot-add large amounts of memory >>> originating from CXL memory expanders, or NVDIMMs, or other 'device >>> memories'. There is a chance there isn't enough regular system memory >>> available to fit ythe memmap for this new memory. It's therefore >>> desirable, if all other conditions are met, for the kmem managed memory >>> to place its memmap on the newly added memory itself. >>> >>> Arrange for this by first allowing for a module parameter override for >>> the mhp_supports_memmap_on_memory() test using a flag, adjusting the >>> only other caller of this interface in dirvers/acpi/acpi_memoryhotplug.c, >>> exporting the symbol so it can be called by kmem.c, and finally changing >>> the kmem driver to add_memory() in chunks of memory_block_size_bytes(). >> >> 1) Why is the override a requirement here? Just let the admin >> configure it then then add conditional support for kmem. >> >> 2) I recall that there are cases where we don't want the memmap to >> land on slow memory (which online_movable would achieve). Just imagine >> the slow PMEM case. So this might need another configuration knob on >> the kmem side. > > From my memory, the case where you don't want the memmap to land on > *persistent memory* is when the device is small (such as NVDIMM-N), and > you want to reserve as much space as possible for the application data. > This has nothing to do with the speed of access. Now that you mention it, I also do remember the origin of the altmap -- to achieve exactly that: place the memmap on the device. commit 4b94ffdc4163bae1ec73b6e977ffb7a7da3d06d3 Author: Dan Williams <dan.j.williams@intel.com> Date: Fri Jan 15 16:56:22 2016 -0800 x86, mm: introduce vmem_altmap to augment vmemmap_populate() In support of providing struct page for large persistent memory capacities, use struct vmem_altmap to change the default policy for allocating memory for the memmap array. The default vmemmap_populate() allocates page table storage area from the page allocator. Given persistent memory capacities relative to DRAM it may not be feasible to store the memmap in 'System Memory'. Instead vmem_altmap represents pre-allocated "device pages" to satisfy vmemmap_alloc_block_buf() requests. In PFN_MODE_PMEM (and only then), we use the altmap (don't see a way to configure it). BUT that case is completely different from the "System RAM" mode. The memmap of an NVDIMM in pmem mode is barely used by core-mm (i.e., not the buddy). In comparison, if the buddy and everybody else works on the memmap in "System RAM", it's much more significant if that resides on slow memory. Looking at commit 9b6e63cbf85b89b2dbffa4955dbf2df8250e5375 Author: Michal Hocko <mhocko@suse.com> Date: Tue Oct 3 16:16:19 2017 -0700 mm, page_alloc: add scheduling point to memmap_init_zone memmap_init_zone gets a pfn range to initialize and it can be really large resulting in a soft lockup on non-preemptible kernels NMI watchdog: BUG: soft lockup - CPU#31 stuck for 23s! [kworker/u642:5:1720] [...] task: ffff88ecd7e902c0 ti: ffff88eca4e50000 task.ti: ffff88eca4e50000 RIP: move_pfn_range_to_zone+0x185/0x1d0 [...] Call Trace: devm_memremap_pages+0x2c7/0x430 pmem_attach_disk+0x2fd/0x3f0 [nd_pmem] nvdimm_bus_probe+0x64/0x110 [libnvdimm] It's hard to tell if that was only required due to the memmap for these devices being that large, or also partially because the access to the memmap is slower that it makes a real difference. I recall that we're also often using ZONE_MOVABLE on such slow memory to not end up placing other kernel data structures on there: especially, user space page tables as I've been told. @Dan, any insight on the performance aspects when placing the memmap on (slow) memory and having that memory be consumed by the buddy where we frequently operate on the memmap?
David Hildenbrand <david@redhat.com> writes: > On 13.07.23 21:12, Jeff Moyer wrote: >> David Hildenbrand <david@redhat.com> writes: >> >>> On 16.06.23 00:00, Vishal Verma wrote: >>>> The dax/kmem driver can potentially hot-add large amounts of memory >>>> originating from CXL memory expanders, or NVDIMMs, or other 'device >>>> memories'. There is a chance there isn't enough regular system memory >>>> available to fit ythe memmap for this new memory. It's therefore >>>> desirable, if all other conditions are met, for the kmem managed memory >>>> to place its memmap on the newly added memory itself. >>>> >>>> Arrange for this by first allowing for a module parameter override for >>>> the mhp_supports_memmap_on_memory() test using a flag, adjusting the >>>> only other caller of this interface in dirvers/acpi/acpi_memoryhotplug.c, >>>> exporting the symbol so it can be called by kmem.c, and finally changing >>>> the kmem driver to add_memory() in chunks of memory_block_size_bytes(). >>> >>> 1) Why is the override a requirement here? Just let the admin >>> configure it then then add conditional support for kmem. >>> >>> 2) I recall that there are cases where we don't want the memmap to >>> land on slow memory (which online_movable would achieve). Just imagine >>> the slow PMEM case. So this might need another configuration knob on >>> the kmem side. >> >> From my memory, the case where you don't want the memmap to land on >> *persistent memory* is when the device is small (such as NVDIMM-N), and >> you want to reserve as much space as possible for the application data. >> This has nothing to do with the speed of access. > > Now that you mention it, I also do remember the origin of the altmap -- > to achieve exactly that: place the memmap on the device. > > commit 4b94ffdc4163bae1ec73b6e977ffb7a7da3d06d3 > Author: Dan Williams <dan.j.williams@intel.com> > Date: Fri Jan 15 16:56:22 2016 -0800 > > x86, mm: introduce vmem_altmap to augment vmemmap_populate() > In support of providing struct page for large persistent memory > capacities, use struct vmem_altmap to change the default policy for > allocating memory for the memmap array. The default vmemmap_populate() > allocates page table storage area from the page allocator. Given > persistent memory capacities relative to DRAM it may not be feasible to > store the memmap in 'System Memory'. Instead vmem_altmap represents > pre-allocated "device pages" to satisfy vmemmap_alloc_block_buf() > requests. > > In PFN_MODE_PMEM (and only then), we use the altmap (don't see a way to > configure it). Configuration is done at pmem namespace creation time. The metadata for the namespace indicates where the memmap resides. See the ndctl-create-namespace man page: -M, --map= A pmem namespace in "fsdax" or "devdax" mode requires allocation of per-page metadata. The allocation can be drawn from either: · "mem": typical system memory · "dev": persistent memory reserved from the namespace Given relative capacities of "Persistent Memory" to "System RAM" the allocation defaults to reserving space out of the namespace directly ("--map=dev"). The overhead is 64-bytes per 4K (16GB per 1TB) on x86. > BUT that case is completely different from the "System RAM" mode. The memmap > of an NVDIMM in pmem mode is barely used by core-mm (i.e., not the buddy). Right. (btw, I don't think system ram mode existed back then.) > In comparison, if the buddy and everybody else works on the memmap in > "System RAM", it's much more significant if that resides on slow memory. Agreed. > Looking at > > commit 9b6e63cbf85b89b2dbffa4955dbf2df8250e5375 > Author: Michal Hocko <mhocko@suse.com> > Date: Tue Oct 3 16:16:19 2017 -0700 > > mm, page_alloc: add scheduling point to memmap_init_zone > memmap_init_zone gets a pfn range to initialize and it can be > really > large resulting in a soft lockup on non-preemptible kernels > NMI watchdog: BUG: soft lockup - CPU#31 stuck for 23s! > [kworker/u642:5:1720] > [...] > task: ffff88ecd7e902c0 ti: ffff88eca4e50000 task.ti: ffff88eca4e50000 > RIP: move_pfn_range_to_zone+0x185/0x1d0 > [...] > Call Trace: > devm_memremap_pages+0x2c7/0x430 > pmem_attach_disk+0x2fd/0x3f0 [nd_pmem] > nvdimm_bus_probe+0x64/0x110 [libnvdimm] > > > It's hard to tell if that was only required due to the memmap for these devices > being that large, or also partially because the access to the memmap is slower > that it makes a real difference. I believe the main driver was the size. At the time, Intel was advertising 3TiB/socket for pmem. I can't remember the exact DRAM configuration sizes from the time. > I recall that we're also often using ZONE_MOVABLE on such slow memory > to not end up placing other kernel data structures on there: especially, > user space page tables as I've been told. Part of the issue was preserving the media. The page structure gets lots of updates, and that could cause premature wear. > @Dan, any insight on the performance aspects when placing the memmap on > (slow) memory and having that memory be consumed by the buddy where we frequently > operate on the memmap? I'm glad you're asking these questions. We definitely want to make sure we don't conflate requirements based on some particular technology/implementation. Also, I wouldn't make any assumptions about the performance of CXL devices. As I understand it, there could be a broad spectrum of performance profiles. And now Dan can correct anything I got wrong. ;-) Cheers, Jeff
The dax/kmem driver can potentially hot-add large amounts of memory originating from CXL memory expanders, or NVDIMMs, or other 'device memories'. There is a chance there isn't enough regular system memory available to fit ythe memmap for this new memory. It's therefore desirable, if all other conditions are met, for the kmem managed memory to place its memmap on the newly added memory itself. Arrange for this by first allowing for a module parameter override for the mhp_supports_memmap_on_memory() test using a flag, adjusting the only other caller of this interface in dirvers/acpi/acpi_memoryhotplug.c, exporting the symbol so it can be called by kmem.c, and finally changing the kmem driver to add_memory() in chunks of memory_block_size_bytes(). Signed-off-by: Vishal Verma <vishal.l.verma@intel.com> --- Vishal Verma (3): mm/memory_hotplug: Allow an override for the memmap_on_memory param mm/memory_hotplug: Export symbol mhp_supports_memmap_on_memory() dax/kmem: Always enroll hotplugged memory for memmap_on_memory include/linux/memory_hotplug.h | 2 +- drivers/acpi/acpi_memhotplug.c | 2 +- drivers/dax/kmem.c | 49 +++++++++++++++++++++++++++++++----------- mm/memory_hotplug.c | 25 ++++++++++++++------- 4 files changed, 55 insertions(+), 23 deletions(-) --- base-commit: f1fcbaa18b28dec10281551dfe6ed3a3ed80e3d6 change-id: 20230613-vv-kmem_memmap-5483c8d04279 Best regards,