Re: [PATCH v7 06/10] mm/memory_hotplug: add offline_and_remove_memory_ranges()

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