Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [High] offline_and_remove_memory_ranges() silently discards errors from try_remove_memory(), falsely returning success and breaking all-or-nothing rollback semantics. --
--- Patch [6]: [PATCH v7 06/10] mm/memory_hotplug: add offline_and_remove_memory_ranges() --- commit 5dfae7d4a23ca17c8437f8cb92a4cc3f409e0c37 Author: Gregory Price <[email protected]> mm/memory_hotplug: add offline_and_remove_memory_ranges() This commit introduces offline_and_remove_memory_ranges() to handle the offlining and removal of multiple memory ranges as a single atomic operation under the device hotplug lock. It offlines all blocks first, reverting on failure, before attempting to remove them. > diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c > index a66346def504b..e33e0f4ae5da1 100644 > --- a/mm/memory_hotplug.c > +++ b/mm/memory_hotplug.c [ ... ] > +int offline_and_remove_memory_ranges(const struct range *ranges, > + unsigned int nr_ranges) > +{ [ ... ] > + /* Phase 2: Remove. This should never fail holding the hotplug lock */ > + for (i = 0; i < nr_ranges; i++) > + WARN_ON_ONCE(try_remove_memory(ranges[i].start, > + range_len(&ranges[i]))); [Severity: High] If try_remove_memory() does happen to fail during Phase 2, does wrapping it in WARN_ON_ONCE() silently discard the error code? Since rc is not updated here, offline_and_remove_memory_ranges() would return 0 (success) to the caller even if the removal failed. Could this mislead the calling driver into freeing its device structures, leaving the kernel's memory subsystem with stale references to unremoved memory blocks? > + > +out_unlock: > unlock_device_hotplug(); > > kfree(online_types); > return rc; > } > +EXPORT_SYMBOL_GPL(offline_and_remove_memory_ranges); -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=6
