On Tue, 9 Apr 2019 12:01:45 +0200 David Hildenbrand <david@xxxxxxxxxx> wrote:
> __add_pages() doesn't add the memory resource, so __remove_pages()
> shouldn't remove it. Let's factor it out. Especially as it is a special
> case for memory used as system memory, added via add_memory() and
> friends.
>
> We now remove the resource after removing the sections instead of doing
> it the other way around. I don't think this change is problematic.
>
> add_memory()
> register memory resource
> arch_add_memory()
>
> remove_memory
> arch_remove_memory()
> release memory resource
>
> While at it, explain why we ignore errors and that it only happeny if
> we remove memory in a different granularity as we added it.
Seems sane.
> --- a/mm/memory_hotplug.c
> +++ b/mm/memory_hotplug.c
> @@ -1820,6 +1806,25 @@ void try_offline_node(int nid)
> }
> EXPORT_SYMBOL(try_offline_node);
>
> +static void __release_memory_resource(u64 start, u64 size)
> +{
> + int ret;
> +
> + /*
> + * When removing memory in the same granularity as it was added,
> + * this function never fails. It might only fail if resources
> + * have to be adjusted or split. We'll ignore the error, as
> + * removing 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);
> + }
> +}
The types seem confused here. Should `start' and `size' be
resource_size_t? Or maybe phys_addr_t.
release_mem_region_adjustable() takes resource_size_t's.
Is %pa the way to print a resource_size_t? I guess it happens to work
because resource_size_t happens to map onto phys_addr_t, which isn't
ideal.
Wanna have a headscratch over that?
> /**
> * remove_memory
> * @nid: the node ID
> @@ -1854,6 +1859,7 @@ void __ref __remove_memory(int nid, u64 start, u64 size)
> memblock_remove(start, size);
>
> arch_remove_memory(nid, start, size, NULL);
> + __release_memory_resource(start, size);
>
> try_offline_node(nid);
