lkml.org 
[lkml]   [2019]   [Apr]   [9]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH v1 1/4] mm/memory_hotplug: Release memory resource after arch_remove_memory()
On Tue,  9 Apr 2019 12:01:45 +0200 David Hildenbrand <david@redhat.com> 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);

\
 
 \ /
  Last update: 2019-04-10 00:42    [W:0.141 / U:9.644 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site