Re: [PATCH v17 1/9] mm: memory_hotplug: factor out bootmem core functions to bootmem_info.c

Re: [PATCH v17 1/9] mm: memory_hotplug: factor out bootmem core functions to bootmem_info.c

[Bodeddula, Balasubramaniam]

[-- Attachment #1: Type: text/plain, Size: 981 bytes --]

Tested-by: bodeddub@amazon.com

We are interested in this patch and have tested an earlier version of this patch on 5.11-rc4 kernel version. I did a functional validation of the changes and saw that the total memory listed by free command increasing and more memory was made available when memory was allocated in hugepages. 1G hugepages gave higher improvements compared to 2M, as expected. Is there a formal way to publish the results? I can do the same as required (I am new to Linux Kernel patching process).

I have a few follow-up questions on this patch:
1. What is the overall status of this patch? What is the ballpark timeline we are looking for this patch to be accepted.
2. Why is this patch not working when memory is allocated as hugepages by THP (transparent hugepages). THP uses AnonHugePages, doesn’t this patch generalize for all ‘type’ of hugepages?

Please let me know if there are any additional tasks that I can help. Happy to help.

Thanks.

[-- Attachment #2: Type: text/html, Size: 4041 bytes --]

Re: [External] Re: [PATCH v17 1/9] mm: memory_hotplug: factor out bootmem core functions to bootmem_info.c

[Muchun Song]

On Sat, Feb 27, 2021 at 2:06 AM Bodeddula, Balasubramaniam
<bodeddub@amazon.com> wrote:
>
> Tested-by: bodeddub@amazon.com

Very thanks for your test.

>
>
>
> We are interested in this patch and have tested an earlier version of this patch on 5.11-rc4 kernel version. I did a functional validation of the changes and saw that the total memory listed by free command increasing and more memory was made available when memory was allocated in hugepages. 1G hugepages gave higher improvements compared to 2M, as expected. Is there a formal way to publish the results? I can do the same as required (I am new to Linux Kernel patching process).

I don’t know if there is a formal way. But maybe you can share
the test result directly through this thread. If someone knows
this, please let me know. Thanks.

>
>
>
> I have a few follow-up questions on this patch:
>
> 1. What is the overall status of this patch? What is the ballpark timeline we are looking for this patch to be accepted.

There is only one patch of this patchset that has no reviewed-by tag.
I think it might be 5.13 in the best case But I don't have the right to
decide.

>
> 2. Why is this patch not working when memory is allocated as hugepages by THP (transparent hugepages). THP uses AnonHugePages, doesn’t this patch generalize for all ‘type’ of hugepages?

Now it only supports HugeTLB pages. THP is a little different
and complex compared to HugeTLB. I need to investigate THP
in depth to determine the possible problems.

>
>
>
> Please let me know if there are any additional tasks that I can help. Happy to help.
>
>
>
> Thanks.

Re: [External] Re: [PATCH v17 1/9] mm: memory_hotplug: factor out bootmem core functions to bootmem_info.c

[Muchun Song]

On Thu, Mar 4, 2021 at 12:26 PM Balbir Singh <bsingharora@gmail.com> wrote:
>
> On Wed, Mar 03, 2021 at 01:45:00PM +1100, Singh, Balbir wrote:
> > On 26/2/21 12:21 am, 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>
> > ...
> >
> > > diff --git a/mm/bootmem_info.c b/mm/bootmem_info.c
> > > new file mode 100644
> > > index 000000000000..fcab5a3f8cc0
> > > --- /dev/null
> > > +++ b/mm/bootmem_info.c
> > > @@ -0,0 +1,124 @@
> > > +// SPDX-License-Identifier: GPL-2.0
> > > +/*
> > > + *  linux/mm/bootmem_info.c
> > > + *
> > > + *  Copyright (C)
> >
> > Looks like incomplete
> >
> Not that my comment was, I should have said
>
> The copyright looks very incomplete

Yes. Just copied from mm/memory_hotplug.c.
I can improve it in the next version. Thanks.

>
> Balbir Singh.