Message ID | 1592442930-9380-3-git-send-email-anshuman.khandual@arm.com (mailing list archive) |
---|---|
State | Not Applicable |
Headers | show |
Series | arm64: Enable vmemmap mapping from device memory | expand |
On Thu, Jun 18, 2020 at 06:45:29AM +0530, Anshuman Khandual wrote: > There are many instances where vmemap allocation is often switched between > regular memory and device memory just based on whether altmap is available > or not. vmemmap_alloc_block_buf() is used in various platforms to allocate > vmemmap mappings. Lets also enable it to handle altmap based device memory > allocation along with existing regular memory allocations. This will help > in avoiding the altmap based allocation switch in many places. > > While here also implement a regular memory allocation fallback mechanism > when the first preferred device memory allocation fails. This will ensure > preserving the existing semantics on powerpc platform. To summarize there > are three different methods to call vmemmap_alloc_block_buf(). > > (., NULL, false) /* Allocate from system RAM */ > (., altmap, false) /* Allocate from altmap without any fallback */ > (., altmap, true) /* Allocate from altmap with fallback (system RAM) */ [...] > diff --git a/arch/powerpc/mm/init_64.c b/arch/powerpc/mm/init_64.c > index bc73abf0bc25..01e25b56eccb 100644 > --- a/arch/powerpc/mm/init_64.c > +++ b/arch/powerpc/mm/init_64.c > @@ -225,12 +225,12 @@ int __meminit vmemmap_populate(unsigned long start, unsigned long end, int node, > * fall back to system memory if the altmap allocation fail. > */ > if (altmap && !altmap_cross_boundary(altmap, start, page_size)) { > - p = altmap_alloc_block_buf(page_size, altmap); > - if (!p) > - pr_debug("altmap block allocation failed, falling back to system memory"); > + p = vmemmap_alloc_block_buf(page_size, node, > + altmap, true); > + } else { > + p = vmemmap_alloc_block_buf(page_size, node, > + NULL, false); > } > - if (!p) > - p = vmemmap_alloc_block_buf(page_size, node); > if (!p) > return -ENOMEM; Is the fallback argument actually necessary. It may be cleaner to just leave the code as is with the choice between altmap and NULL. If an arch needs a fallback (only powerpc), they have the fallback in place already. I don't see the powerpc code any better after this change. I'm fine with the altmap argument though.
On 07/02/2020 07:37 PM, Catalin Marinas wrote: > On Thu, Jun 18, 2020 at 06:45:29AM +0530, Anshuman Khandual wrote: >> There are many instances where vmemap allocation is often switched between >> regular memory and device memory just based on whether altmap is available >> or not. vmemmap_alloc_block_buf() is used in various platforms to allocate >> vmemmap mappings. Lets also enable it to handle altmap based device memory >> allocation along with existing regular memory allocations. This will help >> in avoiding the altmap based allocation switch in many places. >> >> While here also implement a regular memory allocation fallback mechanism >> when the first preferred device memory allocation fails. This will ensure >> preserving the existing semantics on powerpc platform. To summarize there >> are three different methods to call vmemmap_alloc_block_buf(). >> >> (., NULL, false) /* Allocate from system RAM */ >> (., altmap, false) /* Allocate from altmap without any fallback */ >> (., altmap, true) /* Allocate from altmap with fallback (system RAM) */ > [...] >> diff --git a/arch/powerpc/mm/init_64.c b/arch/powerpc/mm/init_64.c >> index bc73abf0bc25..01e25b56eccb 100644 >> --- a/arch/powerpc/mm/init_64.c >> +++ b/arch/powerpc/mm/init_64.c >> @@ -225,12 +225,12 @@ int __meminit vmemmap_populate(unsigned long start, unsigned long end, int node, >> * fall back to system memory if the altmap allocation fail. >> */ >> if (altmap && !altmap_cross_boundary(altmap, start, page_size)) { >> - p = altmap_alloc_block_buf(page_size, altmap); >> - if (!p) >> - pr_debug("altmap block allocation failed, falling back to system memory"); >> + p = vmemmap_alloc_block_buf(page_size, node, >> + altmap, true); >> + } else { >> + p = vmemmap_alloc_block_buf(page_size, node, >> + NULL, false); >> } >> - if (!p) >> - p = vmemmap_alloc_block_buf(page_size, node); >> if (!p) >> return -ENOMEM; > > Is the fallback argument actually necessary. It may be cleaner to just > leave the code as is with the choice between altmap and NULL. If an arch > needs a fallback (only powerpc), they have the fallback in place > already. I don't see the powerpc code any better after this change. > > I'm fine with the altmap argument though. Okay. Will drop 'fallback' from vmemmap_alloc_block_buf() and update the callers. There will also be a single change in the subsequent patch i.e vmemmap_alloc_block_buf(PMD_SIZE, node, altmap).
Catalin Marinas <catalin.marinas@arm.com> writes: > On Thu, Jun 18, 2020 at 06:45:29AM +0530, Anshuman Khandual wrote: >> There are many instances where vmemap allocation is often switched between >> regular memory and device memory just based on whether altmap is available >> or not. vmemmap_alloc_block_buf() is used in various platforms to allocate >> vmemmap mappings. Lets also enable it to handle altmap based device memory >> allocation along with existing regular memory allocations. This will help >> in avoiding the altmap based allocation switch in many places. >> >> While here also implement a regular memory allocation fallback mechanism >> when the first preferred device memory allocation fails. This will ensure >> preserving the existing semantics on powerpc platform. To summarize there >> are three different methods to call vmemmap_alloc_block_buf(). >> >> (., NULL, false) /* Allocate from system RAM */ >> (., altmap, false) /* Allocate from altmap without any fallback */ >> (., altmap, true) /* Allocate from altmap with fallback (system RAM) */ > [...] >> diff --git a/arch/powerpc/mm/init_64.c b/arch/powerpc/mm/init_64.c >> index bc73abf0bc25..01e25b56eccb 100644 >> --- a/arch/powerpc/mm/init_64.c >> +++ b/arch/powerpc/mm/init_64.c >> @@ -225,12 +225,12 @@ int __meminit vmemmap_populate(unsigned long start, unsigned long end, int node, >> * fall back to system memory if the altmap allocation fail. >> */ >> if (altmap && !altmap_cross_boundary(altmap, start, page_size)) { >> - p = altmap_alloc_block_buf(page_size, altmap); >> - if (!p) >> - pr_debug("altmap block allocation failed, falling back to system memory"); >> + p = vmemmap_alloc_block_buf(page_size, node, >> + altmap, true); >> + } else { >> + p = vmemmap_alloc_block_buf(page_size, node, >> + NULL, false); >> } >> - if (!p) >> - p = vmemmap_alloc_block_buf(page_size, node); >> if (!p) >> return -ENOMEM; > > Is the fallback argument actually necessary. It may be cleaner to just > leave the code as is with the choice between altmap and NULL. If an arch > needs a fallback (only powerpc), they have the fallback in place > already. I don't see the powerpc code any better after this change. Yeah I agree. cheers
diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c index 0adad8859393..7ca21adb4412 100644 --- a/arch/arm64/mm/mmu.c +++ b/arch/arm64/mm/mmu.c @@ -1100,7 +1100,8 @@ int __meminit vmemmap_populate(unsigned long start, unsigned long end, int node, if (pmd_none(READ_ONCE(*pmdp))) { void *p = NULL; - p = vmemmap_alloc_block_buf(PMD_SIZE, node); + p = vmemmap_alloc_block_buf(PMD_SIZE, node, + NULL, false); if (!p) return -ENOMEM; diff --git a/arch/powerpc/mm/init_64.c b/arch/powerpc/mm/init_64.c index bc73abf0bc25..01e25b56eccb 100644 --- a/arch/powerpc/mm/init_64.c +++ b/arch/powerpc/mm/init_64.c @@ -225,12 +225,12 @@ int __meminit vmemmap_populate(unsigned long start, unsigned long end, int node, * fall back to system memory if the altmap allocation fail. */ if (altmap && !altmap_cross_boundary(altmap, start, page_size)) { - p = altmap_alloc_block_buf(page_size, altmap); - if (!p) - pr_debug("altmap block allocation failed, falling back to system memory"); + p = vmemmap_alloc_block_buf(page_size, node, + altmap, true); + } else { + p = vmemmap_alloc_block_buf(page_size, node, + NULL, false); } - if (!p) - p = vmemmap_alloc_block_buf(page_size, node); if (!p) return -ENOMEM; diff --git a/arch/x86/mm/init_64.c b/arch/x86/mm/init_64.c index 19c0ed3271a3..4ae4f767c004 100644 --- a/arch/x86/mm/init_64.c +++ b/arch/x86/mm/init_64.c @@ -1463,10 +1463,8 @@ static int __meminit vmemmap_populate_hugepages(unsigned long start, if (pmd_none(*pmd)) { void *p; - if (altmap) - p = altmap_alloc_block_buf(PMD_SIZE, altmap); - else - p = vmemmap_alloc_block_buf(PMD_SIZE, node); + p = vmemmap_alloc_block_buf(PMD_SIZE, node, + altmap, false); if (p) { pte_t entry; diff --git a/include/linux/mm.h b/include/linux/mm.h index e40ac543d248..dade7c3f634d 100644 --- a/include/linux/mm.h +++ b/include/linux/mm.h @@ -3015,7 +3015,8 @@ pte_t *vmemmap_pte_populate(pmd_t *pmd, unsigned long addr, int node, struct vmem_altmap *altmap); void *vmemmap_alloc_block(unsigned long size, int node); struct vmem_altmap; -void *vmemmap_alloc_block_buf(unsigned long size, int node); +void *vmemmap_alloc_block_buf(unsigned long size, int node, + struct vmem_altmap *altmap, bool sysram_fallback); void *altmap_alloc_block_buf(unsigned long size, struct vmem_altmap *altmap); void vmemmap_verify(pte_t *, int, unsigned long, unsigned long); int vmemmap_populate_basepages(unsigned long start, unsigned long end, diff --git a/mm/sparse-vmemmap.c b/mm/sparse-vmemmap.c index ceed10dec31e..388121c5bbcb 100644 --- a/mm/sparse-vmemmap.c +++ b/mm/sparse-vmemmap.c @@ -70,10 +70,31 @@ void * __meminit vmemmap_alloc_block(unsigned long size, int node) } /* need to make sure size is all the same during early stage */ -void * __meminit vmemmap_alloc_block_buf(unsigned long size, int node) +void * __meminit vmemmap_alloc_block_buf(unsigned long size, int node, + struct vmem_altmap *altmap, + bool sysram_fallback) { - void *ptr = sparse_buffer_alloc(size); + void *ptr; + /* + * There is no point in asking for sysram fallback + * without an altmap request to begin with. So just + * warn here to catch potential call sites that may + * be violating this. + */ + WARN_ON(!altmap && sysram_fallback); + + if (altmap) { + ptr = altmap_alloc_block_buf(size, altmap); + if (ptr) + return ptr; + pr_debug("altmap block allocation failed\n"); + if (!sysram_fallback) + return NULL; + pr_debug("falling back to system memory\n"); + } + + ptr = sparse_buffer_alloc(size); if (!ptr) ptr = vmemmap_alloc_block(size, node); return ptr; @@ -147,10 +168,7 @@ pte_t * __meminit vmemmap_pte_populate(pmd_t *pmd, unsigned long addr, int node, pte_t entry; void *p; - if (altmap) - p = altmap_alloc_block_buf(PAGE_SIZE, altmap); - else - p = vmemmap_alloc_block_buf(PAGE_SIZE, node); + p = vmemmap_alloc_block_buf(PAGE_SIZE, node, altmap, false); if (!p) return NULL; entry = pfn_pte(__pa(p) >> PAGE_SHIFT, PAGE_KERNEL);