search for: try_remove_memory

...owledge" >> when onlining. > > I am not sure I follow. So you exclude those pages when onlining? Exactly, using the online_page callback. The pages will - again - be marked PG_offline with a refcount of 0. They will not be put to the buddy. > >>> Should we allow to try_remove_memory to succeed with these pages? >> >> I think we should first properly offline them (mark sections offline and >> memory blocks, fixup numbers, shrink zones ...). The we can cleanly remove >> the memory. (see [PATCH RFC v3 8/9] mm/memory_hotplug: Introduce >> offline_and_...

...a memory block that is partially > populated (e.g., HpyerV balloon right now). > > So it's effectively "re-initializing the memmap using the driver knowledge" > when onlining. I am not sure I follow. So you exclude those pages when onlining? > > Should we allow to try_remove_memory to succeed with these pages? > > I think we should first properly offline them (mark sections offline and > memory blocks, fixup numbers, shrink zones ...). The we can cleanly remove > the memory. (see [PATCH RFC v3 8/9] mm/memory_hotplug: Introduce > offline_and_remove_memory()) I...

...a memory block that is partially > populated (e.g., HpyerV balloon right now). > > So it's effectively "re-initializing the memmap using the driver knowledge" > when onlining. I am not sure I follow. So you exclude those pages when onlining? > > Should we allow to try_remove_memory to succeed with these pages? > > I think we should first properly offline them (mark sections offline and > memory blocks, fixup numbers, shrink zones ...). The we can cleanly remove > the memory. (see [PATCH RFC v3 8/9] mm/memory_hotplug: Introduce > offline_and_remove_memory()) I...

...fier. > > Alternative: Allow to offline with a reference count of 1 > and use some other sign in the struct page that offlining is permitted. Few questions. I do not see onlining code to take care of this special case. What should happen when offline && online? Should we allow to try_remove_memory to succeed with these pages? Do we really have hook into __put_page? Why do we even care about the reference count of those pages? Wouldn't it be just more consistent to elevate the reference count (I guess this is what you suggest in the last paragraph) and the virtio driver would return that...

...fier. > > Alternative: Allow to offline with a reference count of 1 > and use some other sign in the struct page that offlining is permitted. Few questions. I do not see onlining code to take care of this special case. What should happen when offline && online? Should we allow to try_remove_memory to succeed with these pages? Do we really have hook into __put_page? Why do we even care about the reference count of those pages? Wouldn't it be just more consistent to elevate the reference count (I guess this is what you suggest in the last paragraph) and the virtio driver would return that...

>> static int __ref try_remove_memory(int nid, u64 start, u64 size) >> { >> int rc = 0; >> @@ -1777,7 +1757,7 @@ static int __ref try_remove_memory(int nid, u64 start, u64 size) >> memblock_remove(start, size); >> } >> >> - __release_memory_resource(start, size); >> + release_mem_regi...

On 15.09.20 11:06, Wei Yang wrote: > On Tue, Sep 15, 2020 at 09:35:30AM +0200, David Hildenbrand wrote: >> >>>> static int __ref try_remove_memory(int nid, u64 start, u64 size) >>>> { >>>> int rc = 0; >>>> @@ -1777,7 +1757,7 @@ static int __ref try_remove_memory(int nid, u64 start, u64 size) >>>> memblock_remove(start, size); >>>> } >>>> >>>> - __release_m...

...of memory cannot fail. - */ - ret = release_mem_region_adjustable(&iomem_resource, start, size); - if (ret) { - resource_size_t endres = start + size - 1; - - pr_warn("Unable to release resource <%pa-%pa> (%d)\n", - &start, &endres, ret); - } -} - static int __ref try_remove_memory(int nid, u64 start, u64 size) { int rc = 0; @@ -1753,7 +1733,7 @@ static int __ref try_remove_memory(int nid, u64 start, u64 size) memblock_remove(start, size); } - __release_memory_resource(start, size); + release_mem_region_adjustable(&iomem_resource, start, size); try_offline_n...

...t callback where these pages will get excluded. This is exactly the same as when onling a memory block that is partially populated (e.g., HpyerV balloon right now). So it's effectively "re-initializing the memmap using the driver knowledge" when onlining. > Should we allow to try_remove_memory to succeed with these pages? I think we should first properly offline them (mark sections offline and memory blocks, fixup numbers, shrink zones ...). The we can cleanly remove the memory. (see [PATCH RFC v3 8/9] mm/memory_hotplug: Introduce offline_and_remove_memory()) Once offline, the memma...

...of memory cannot fail. - */ - ret = release_mem_region_adjustable(&iomem_resource, start, size); - if (ret) { - resource_size_t endres = start + size - 1; - - pr_warn("Unable to release resource <%pa-%pa> (%d)\n", - &start, &endres, ret); - } -} - static int __ref try_remove_memory(int nid, u64 start, u64 size) { int rc = 0; @@ -1777,7 +1757,7 @@ static int __ref try_remove_memory(int nid, u64 start, u64 size) memblock_remove(start, size); } - __release_memory_resource(start, size); + release_mem_region_adjustable(&iomem_resource, start, size); try_offline_n...

Some add_memory*() users add memory in small, contiguous memory blocks. Examples include virtio-mem, hyper-v balloon, and the XEN balloon. This can quickly result in a lot of memory resources, whereby the actual resource boundaries are not of interest (e.g., it might be relevant for DIMMs, exposed via /proc/iomem to user space). We really want to merge added resources in this scenario where

Some add_memory*() users add memory in small, contiguous memory blocks. Examples include virtio-mem, hyper-v balloon, and the XEN balloon. This can quickly result in a lot of memory resources, whereby the actual resource boundaries are not of interest (e.g., it might be relevant for DIMMs, exposed via /proc/iomem to user space). We really want to merge added resources in this scenario where

Some add_memory*() users add memory in small, contiguous memory blocks. Examples include virtio-mem, hyper-v balloon, and the XEN balloon. This can quickly result in a lot of memory resources, whereby the actual resource boundaries are not of interest (e.g., it might be relevant for DIMMs, exposed via /proc/iomem to user space). We really want to merge added resources in this scenario where

...h a reference count of 1 > > > and use some other sign in the struct page that offlining is permitted. > > > > Few questions. I do not see onlining code to take care of this special > > case. What should happen when offline && online? > > Should we allow to try_remove_memory to succeed with these pages? > > Do we really have hook into __put_page? Why do we even care about the > > reference count of those pages? > > Oh, I forgot to answer this questions. The __put_page() change is necessary > for the following race I identified: > > Page has...

...h a reference count of 1 > > > and use some other sign in the struct page that offlining is permitted. > > > > Few questions. I do not see onlining code to take care of this special > > case. What should happen when offline && online? > > Should we allow to try_remove_memory to succeed with these pages? > > Do we really have hook into __put_page? Why do we even care about the > > reference count of those pages? > > Oh, I forgot to answer this questions. The __put_page() change is necessary > for the following race I identified: > > Page has...

...again before it gets removed. > > > > Why does that matter? Is it really likely that the userspace would > > interfere? What would be the scenario? > > I guess it's not that relevant after all (I think this comment dates > back to the times where we didn't have try_remove_memory() and could > actually BUG_ON() in remove_memory() if there would have been a race). > Can drop that part. > > > > > Or is still mostly about not requiring callers to open code this general > > patter? > > From kernel module context, I cannot get access to the ac...

...tive: Allow to offline with a reference count of 1 >> and use some other sign in the struct page that offlining is permitted. > > Few questions. I do not see onlining code to take care of this special > case. What should happen when offline && online? > Should we allow to try_remove_memory to succeed with these pages? > Do we really have hook into __put_page? Why do we even care about the > reference count of those pages? Oh, I forgot to answer this questions. The __put_page() change is necessary for the following race I identified: Page has a refcount of 1 (e.g., allocated...

...000-ffffffff : Reserved 100000000-13fffffff : System RAM 140000000-33fffffff : virtio0 140000000-1bfffffff : System RAM (virtio_mem) 3280000000-32ffffffff : PCI Bus 0000:00 Of course, with more hotplugged memory, it gets worse. When unplugging memory blocks again, try_remove_memory() (via offline_and_remove_memory()) will properly split the resource up again. Signed-off-by: David Hildenbrand <david at redhat.com> --- drivers/virtio/virtio_mem.c | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/drivers/virtio/virtio_mem.c b/drivers/vir...

...t))); + if (mem) + rc = device_offline(&mem->dev); + /* Ignore if the device is already offline. */ + if (rc > 0) + rc = 0; + + /* + * In case we succeeded to offline the memory block, remove it. + * This cannot fail as it cannot get onlined in the meantime. + */ + if (!rc && try_remove_memory(nid, start, size)) + BUG(); + unlock_device_hotplug(); + + return rc; +} +EXPORT_SYMBOL_GPL(offline_and_remove_memory); #endif /* CONFIG_MEMORY_HOTREMOVE */ -- 2.21.0

...t))); + if (mem) + rc = device_offline(&mem->dev); + /* Ignore if the device is already offline. */ + if (rc > 0) + rc = 0; + + /* + * In case we succeeded to offline the memory block, remove it. + * This cannot fail as it cannot get onlined in the meantime. + */ + if (!rc) { + rc = try_remove_memory(nid, start, size); + WARN_ON_ONCE(rc); + } + unlock_device_hotplug(); + + return rc; +} +EXPORT_SYMBOL_GPL(offline_and_remove_memory); #endif /* CONFIG_MEMORY_HOTREMOVE */ -- 2.24.1