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:4.647 / U:2.560 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site