lkml.org 
[lkml]   [2021]   [Mar]   [10]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH v18 1/9] mm: memory_hotplug: factor out bootmem core functions to bootmem_info.c
[I am sorry for a late review]

On Mon 08-03-21 18:27:59, Muchun Song wrote:
> Move bootmem info registration common API to individual bootmem_info.c.
> And we will use {get,put}_page_bootmem() to initialize the page for the
> vmemmap pages or free the vmemmap pages to buddy in the later patch.
> So move them out of CONFIG_MEMORY_HOTPLUG_SPARSE. This is just code
> movement without any functional change.
>
> Signed-off-by: Muchun Song <songmuchun@bytedance.com>
> Acked-by: Mike Kravetz <mike.kravetz@oracle.com>
> Reviewed-by: Oscar Salvador <osalvador@suse.de>
> Reviewed-by: David Hildenbrand <david@redhat.com>
> Reviewed-by: Miaohe Lin <linmiaohe@huawei.com>
> Tested-by: Chen Huang <chenhuang5@huawei.com>
> Tested-by: Bodeddula Balasubramaniam <bodeddub@amazon.com>

Separation from memory_hotplug.c is definitely a right step. I am
wondering about the config dependency though
[...]
> diff --git a/mm/Makefile b/mm/Makefile
> index 72227b24a616..daabf86d7da8 100644
> --- a/mm/Makefile
> +++ b/mm/Makefile
> @@ -83,6 +83,7 @@ obj-$(CONFIG_SLUB) += slub.o
> obj-$(CONFIG_KASAN) += kasan/
> obj-$(CONFIG_KFENCE) += kfence/
> obj-$(CONFIG_FAILSLAB) += failslab.o
> +obj-$(CONFIG_HAVE_BOOTMEM_INFO_NODE) += bootmem_info.o

I would have expected this would depend on CONFIG_SPARSE.
BOOTMEM_INFO_NODE is really an odd thing to depend on here. There is
some functionality which requires the node info but that can be gated
specifically. Or what is the thinking behind?

This doesn't matter right now because it seems that the *_page_bootmem
is only used by x86 outside of the memory hotplug.

Other than that looks good to me.
--
Michal Hocko
SUSE Labs

\
 
 \ /
  Last update: 2021-03-10 15:16    [W:0.179 / U:0.680 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site