lkml.org 
[lkml]   [2021]   [Mar]   [11]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
SubjectRe: [PATCH v4 1/5] mm,memory_hotplug: Allocate memmap from the added memory range
Date
This looks essentially good to me, except some parts in 
mhp_supports_memmap_on_memory()

>
> +bool mhp_supports_memmap_on_memory(unsigned long size)
> +{
> + unsigned long pageblock_size = PFN_PHYS(pageblock_nr_pages);
> + unsigned long remaining_mem = size - PMD_SIZE;

This looks weird. I think what you want to test is that


a) "nr_vmemmap_pages * sizeof(struct page)" spans complete PMDs (IOW, we
won't map too much via the altmap when populating a PMD in the vmemmap)

b) "remaining = size - nr_vmemmap_pages * sizeof(struct page)" spans
complete pageblock.

right?

> +
> + /*
> + * Besides having CONFIG_MHP_MEMMAP_ON_MEMORY, we need a few more
> + * assumptions to hold true:
> + * - we are working on a single memory block granularity
> + * - the size of struct page is multiple of PMD.

That's not what you are checking. (and the way it is phrase, I don;t
think it makes sense)

> + * - the remaining memory after having used part of the range
> + * for vmemmap pages is pageblock aligned.
> + *
> + * The reason for the struct page to be multiple of PMD is because
> + * otherwise two sections would intersect a PMD. This would go against
> + * memmap-on-memory premise, as memory would stay pinned until both
> + * sections were removed.
> + *
> + * And the reason for the remaining memory to be pageblock aligned is
> + * because mm core functions work on pageblock granularity in order to
> + * change page's migratetype.
> + */
> + return memmap_on_memory &&
> + size == memory_block_size_bytes() &&
> + IS_ENABLED(CONFIG_MHP_MEMMAP_ON_MEMORY) &&
> + !(PMD_SIZE % sizeof(struct page)) &&
> + remaining_mem &&
> + IS_ALIGNED(remaining_mem, pageblock_size);
> +}
> +

I suggest a restructuring, compressing the information like:

"
Besides having arch support and the feature enabled at runtime, we need
a few more assumptions to hold true:

a) We span a single memory block: memory onlining/offlining happens in
memory block granularity. We don't want the vmemmap of online memory
blocks to reside on offline memory blocks. In the future, we might want
to support variable-sized memory blocks to make the feature more versatile.

b) The vmemmap pages span complete PMDs: We don't want vmemmap code to
populate memory from the altmap for unrelated parts (i.e., other memory
blocks).

c) The vmemmap pages (and thereby the pages that will be exposed to the
buddy) have to cover full pageblocks: memory onlining/offlining code
requires applicable ranges to be page-aligned, for example, to set the
migratetypes properly.
"

Do we have to special case / protect against the vmemmap optimization
for hugetlb pages? Or is that already blocked somehow and I missed it?

--
Thanks,

David / dhildenb

\
 
 \ /
  Last update: 2021-03-11 20:09    [W:0.086 / U:1.128 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site