On Wed, Nov 24, 2021 at 02:45:59PM -0800, John Hubbard wrote:
> @@ -2201,13 +2201,12 @@ static int __ref try_remove_memory(u64 start, u64 size)
> */
> void __remove_memory(u64 start, u64 size)
> {
> -
> + int ret = try_remove_memory(start, size);
> /*
> * trigger BUG() if some memory is not offlined prior to calling this
> * function
> */
> - if (try_remove_memory(start, size))
> - BUG();
> + BUG_ON(ret);
> }
I'd rather leave it the way it is. I don't see why the version you
propose is better.
> ...and by the way, while going to type that, I immediately stumbled upon
> another pre-existing case of this sort of thing, in try_remove_memory(),
> which does this:
>
> static int __ref try_remove_memory(u64 start, u64 size)
> {
> struct vmem_altmap mhp_altmap = {};
> struct vmem_altmap *altmap = NULL;
> unsigned long nr_vmemmap_pages;
> int rc = 0, nid = NUMA_NO_NODE;
>
> BUG_ON(check_hotplug_memory_range(start, size));
That needs to be fixed.
