Re: [PATCH v4 1/7] mm: page_alloc: avoid merging non-fallbackable pageblocks with others.
On Mon, Jan 24, 2022 at 11:12:07AM -0500, Zi Yan wrote:
> On 24 Jan 2022, at 9:02, Mel Gorman wrote:
>
> > On Wed, Jan 19, 2022 at 02:06:17PM -0500, Zi Yan wrote:
> >> From: Zi Yan
> >>
> >> This is done in addition to MIGRATE_ISOLATE pageblock merge avoidance.
> >> It prepares for the upcoming removal of the MAX_ORDER-1 alignment
> >> requirement for CMA and alloc_contig_range().
> >>
> >> MIGRARTE_HIGHATOMIC should not merge with other migratetypes like
> >> MIGRATE_ISOLATE and MIGRARTE_CMA[1], so this commit prevents that too.
> >> Also add MIGRARTE_HIGHATOMIC to fallbacks array for completeness.
> >>
> >> [1]
> >> https://lore.kernel.org/linux-mm/20211130100853.gp3...@techsingularity.net/
> >>
> >> Signed-off-by: Zi Yan
> >>
> >>
> >>
> >> @@ -2484,6 +2483,7 @@ static int fallbacks[MIGRATE_TYPES][3] = {
> >>[MIGRATE_UNMOVABLE] = { MIGRATE_RECLAIMABLE, MIGRATE_MOVABLE,
> >> MIGRATE_TYPES },
> >>[MIGRATE_MOVABLE] = { MIGRATE_RECLAIMABLE, MIGRATE_UNMOVABLE,
> >> MIGRATE_TYPES },
> >>[MIGRATE_RECLAIMABLE] = { MIGRATE_UNMOVABLE, MIGRATE_MOVABLE,
> >> MIGRATE_TYPES },
> >> + [MIGRATE_HIGHATOMIC] = { MIGRATE_TYPES }, /* Never used */
> >> #ifdef CONFIG_CMA
> >>[MIGRATE_CMA] = { MIGRATE_TYPES }, /* Never used */
> >> #endif
> >
> > If it's never used, why is it added?
>
> Just to make the fallbacks list complete, since MIGRATE_CMA and
> MIGRATE_ISOLATE are in the list. Instead, I can remove MIGRATE_CMA and
> MIGRATE_ISOLATE. WDYT?
>
It probably makes more sense to remove them or replace them with a comment
stating what migratetypes do not have a fallback list. Do it as a separate
patch that stands alone. It does not need to be part of this series.
--
Mel Gorman
SUSE Labs
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v4 1/7] mm: page_alloc: avoid merging non-fallbackable pageblocks with others.
On Wed, Jan 19, 2022 at 02:06:17PM -0500, Zi Yan wrote:
> From: Zi Yan
>
> This is done in addition to MIGRATE_ISOLATE pageblock merge avoidance.
> It prepares for the upcoming removal of the MAX_ORDER-1 alignment
> requirement for CMA and alloc_contig_range().
>
> MIGRARTE_HIGHATOMIC should not merge with other migratetypes like
> MIGRATE_ISOLATE and MIGRARTE_CMA[1], so this commit prevents that too.
> Also add MIGRARTE_HIGHATOMIC to fallbacks array for completeness.
>
> [1]
> https://lore.kernel.org/linux-mm/20211130100853.gp3...@techsingularity.net/
>
> Signed-off-by: Zi Yan
>
>
>
> @@ -2484,6 +2483,7 @@ static int fallbacks[MIGRATE_TYPES][3] = {
> [MIGRATE_UNMOVABLE] = { MIGRATE_RECLAIMABLE, MIGRATE_MOVABLE,
> MIGRATE_TYPES },
> [MIGRATE_MOVABLE] = { MIGRATE_RECLAIMABLE, MIGRATE_UNMOVABLE,
> MIGRATE_TYPES },
> [MIGRATE_RECLAIMABLE] = { MIGRATE_UNMOVABLE, MIGRATE_MOVABLE,
> MIGRATE_TYPES },
> + [MIGRATE_HIGHATOMIC] = { MIGRATE_TYPES }, /* Never used */
> #ifdef CONFIG_CMA
> [MIGRATE_CMA] = { MIGRATE_TYPES }, /* Never used */
> #endif
If it's never used, why is it added?
Otherwise looks fine so
Acked-by: Mel Gorman
--
Mel Gorman
SUSE Labs
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [RFC PATCH 0/3] Use pageblock_order for cma and alloc_contig_range alignment.
On Tue, Nov 30, 2021 at 10:11:36AM +0100, Vlastimil Babka wrote: > >>> I find that two pageblocks with different migratetypes, like > >>> MIGRATE_RECLAIMABLE > >>> and MIGRATE_MOVABLE can be merged into a single free page after I checked > >>> __free_one_page() in detail and printed pageblock information during > >>> buddy page > >>> merging. > >> > >> Yes, that can happen. > >> > >> I am not sure what consequence it will cause. Do you have any idea? > >> > >> For MIGRATE_RECLAIMABLE or MIGRATE_MOVABLE or even MIGRATE_UNMOVABLE it's > >> absolutely fine. As long as these pageblocks are fully free (and they are > >> if > >> it's a single free page spanning 2 pageblocks), they can be of any of these > >> type, as they can be reused as needed without causing fragmentation. > >> > >> But in case of MIGRATE_CMA and MIGRATE_ISOLATE, uncontrolled merging would > >> break the specifics of those types. That's why the code is careful for > >> MIGRATE_ISOLATE, and MIGRATE_CMA was until now done in MAX_ORDER > >> granularity. > > > > Thanks for the explanation. Basically migratetypes that can fall back to > > each > > other can be merged into a single free page, right? > > Yes. > > > How about MIGRATE_HIGHATOMIC? It should not be merged with other > > migratetypes > > from my understanding. > > Hmm it shouldn't minimally because it has an accounting that would become > broken. So it should prevent merging or make sure the reservations are with > MAX_ORDER granularity, but seems that neither is true? CCing Mel. > MIGRATE_HIGHATOMIC pageblocks can have pages allocated of different types, particularly UNMOVABLE and potentially RECLAIMABLE. The reserving or releasing MIGRATE_HIGHATOMIC pageblocks should be done with reserve_highatomic_pageblock and unreserve_highatomic_pageblock to get the accounting right. However, there does not appear to be any special protection against a page in a highatomic pageblock getting merged with a buddy of another pageblock type. The pageblock would still have the right setting but on allocation, the pages could split to the wrong free list and be lost until the pages belonging to MIGRATE_HIGHATOMIC were freed again. Not sure how much of a problem that is in practice, it's been a while since I've heard of high-order atomic allocation failures. -- Mel Gorman SUSE Labs ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 2/2] arm64: Use gfpflags_allow_blocking()
On Mon, Oct 19, 2015 at 01:43:13PM +0100, Robin Murphy wrote:
> Hi Andrew,
>
> On 16/10/15 21:59, Andrew Morton wrote:
> >On Fri, 16 Oct 2015 16:33:42 +0100 Robin Murphy <robin.mur...@arm.com> wrote:
> >
> >>__GFP_WAIT is going away to live its life under a new identity; convert
> >>__iommu_alloc_attrs() to the new helper function instead.
> >>
> >>...
> >>
> >>--- a/arch/arm64/mm/dma-mapping.c
> >>+++ b/arch/arm64/mm/dma-mapping.c
> >>@@ -566,7 +566,7 @@ static void *__iommu_alloc_attrs(struct device *dev,
> >>size_t size,
> >> */
> >>gfp |= __GFP_ZERO;
> >>
> >>- if (gfp & __GFP_WAIT) {
> >>+ if (gfpflags_allow_blocking(gfp)) {
> >>struct page **pages;
> >>pgprot_t prot = __get_dma_pgprot(attrs, PAGE_KERNEL, coherent);
> >
> >Seems unnecessarily elaborate. What's wrong with
> >
> >---
> >a/arch/arm64/mm/dma-mapping.c~mm-page_alloc-rename-__gfp_wait-to-__gfp_reclaim-arm-fix
> >+++ a/arch/arm64/mm/dma-mapping.c
> >@@ -562,7 +562,7 @@ static void *__iommu_alloc_attrs(struct
> > */
> > gfp |= __GFP_ZERO;
> >
> >-if (gfp & __GFP_WAIT) {
> >+if (gfp & __GFP_RECLAIM) {
> > struct page **pages;
> > pgprot_t prot = __get_dma_pgprot(attrs, PAGE_KERNEL, coherent);
> >
> >
> >?
>
> Well, in that case the charge of "unnecessarily elaborate" should have been
> directed at the original patch, and the 53 other locations where (flags &
> __GFP_WAIT) was changed as per the commit message:
>
> "Callers that are checking if they are non-blocking should use the
>helper gfpflags_allow_blocking() where possible."
>
The use of gfpflags_allows_blocking() like you originally had is actually
preferred by me. __GFP_RECLAIM can return true when the caller only allows
kswapd to wake which has nothing to do with blocking (currently). The
helper was added to avoid this type of confusion.
--
Mel Gorman
SUSE Labs
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
