On 06.07.23 11:36, Aneesh Kumar K V wrote:
On 7/6/23 2:48 PM, David Hildenbrand wrote:
On 06.07.23 10:50, Aneesh Kumar K.V wrote:
With memmap on memory, some architecture needs more details w.r.t altmap
such as base_pfn, end_pfn, etc to unmap vmemmap memory.
Can you elaborate why ppc64 needs that and x86-64 + aarch64 don't?
IOW, why can't ppc64 simply allocate the vmemmap from the start of the memblock (-> base_pfn) and use the stored number of vmemmap pages to calculate the end_pfn?
To rephrase: if the vmemmap is not at the beginning and doesn't cover full apgeblocks, memory onlining/offlining would be broken.
[...]
With ppc64 and 64K pagesize and different memory block sizes, we can end up allocating vmemmap backing memory from outside altmap because
a single page vmemmap can cover 1024 pages (64 *1024/sizeof(struct page)). and that can point to pages outside the dev_pagemap range.
So on free we check
So you end up with a mixture of altmap and ordinarily-allocated vmemmap
pages? That sound wrong (and is counter-intuitive to the feature in
general, where we *don't* want to allocate the vmemmap from outside the
altmap).
(64 * 1024) / sizeof(struct page) -> 1024 pages
1024 pages * 64k = 64 MiB.
What's the memory block size on these systems? If it's >= 64 MiB the
vmemmap of a single memory block fits into a single page and we should
be fine.
Smells like you want to disable the feature on a 64k system.
vmemmap_free() {
...
if (altmap) {
alt_start = altmap->base_pfn;
alt_end = altmap->base_pfn + altmap->reserve +
altmap->free + altmap->alloc + altmap->align;
}
...
if (base_pfn >= alt_start && base_pfn < alt_end) {
vmem_altmap_free(altmap, nr_pages);
to see whether we did use altmap for the vmemmap allocation.
+/**
+ * struct vmem_altmap - pre-allocated storage for vmemmap_populate
+ * @base_pfn: base of the entire dev_pagemap mapping
+ * @reserve: pages mapped, but reserved for driver use (relative to @base)
+ * @free: free pages set aside in the mapping for memmap storage
+ * @align: pages reserved to meet allocation alignments
+ * @alloc: track pages consumed, private to vmemmap_populate()
+ */
+struct vmem_altmap {
+ unsigned long base_pfn;
+ const unsigned long end_pfn;
+ const unsigned long reserve;
+ unsigned long free;
+ unsigned long align;
+ unsigned long alloc;
+};
Instead of embedding that, what about conditionally allocating it and store a pointer to it in the "struct memory_block"?
In the general case as of today, we don't have an altmap.
Sure but with memmap on memory option it is essentially adding that right?.
At least on x86_64 and aarch64 only for 128 MiB DIMMs (and especially,
not memory added by hv-balloon, virtio-mem, xen-balloon).
So in the general case it's not that frequently used. Maybe on ppc64
once wired up.
Is the concern related to the increase in the size of
struct memory_block ?
Partially. It looks cleaner to have !mem->altmap if there is no altmap.
--
Cheers,
David / dhildenb
