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: [RFC PATCH 0/3] Use pageblock_order for cma and alloc_contig_range alignment.
On 11/29/21 23:08, Zi Yan wrote: > On 23 Nov 2021, at 12:32, Vlastimil Babka wrote: > >> On 11/23/21 17:35, Zi Yan wrote: >>> On 19 Nov 2021, at 10:15, Zi Yan wrote: >> From what my understanding, cma required alignment of >> max(MAX_ORDER - 1, pageblock_order), because when MIGRATE_CMA was >> introduced, >> __free_one_page() does not prevent merging two different pageblocks, when >> MAX_ORDER - 1 > pageblock_order. But current __free_one_page() >> implementation >> does prevent that. > > But it does prevent that only for isolated pageblock, not CMA, and yout > patchset doesn't seem to expand that to CMA? Or am I missing something. Yeah, you are right. Originally, I thought preventing merging isolated pageblock with other types of pageblocks is sufficient, since MIGRATE_CMA is always converted from MIGRATE_ISOLATE. But that is not true. I will rework the code. Thanks for pointing this out. >>> >>> 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. > -- > Best Regards, > Yan, Zi > ___ 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 23 Nov 2021, at 12:32, Vlastimil Babka wrote: > On 11/23/21 17:35, Zi Yan wrote: >> On 19 Nov 2021, at 10:15, Zi Yan wrote: > From what my understanding, cma required alignment of > max(MAX_ORDER - 1, pageblock_order), because when MIGRATE_CMA was > introduced, > __free_one_page() does not prevent merging two different pageblocks, when > MAX_ORDER - 1 > pageblock_order. But current __free_one_page() > implementation > does prevent that. But it does prevent that only for isolated pageblock, not CMA, and yout patchset doesn't seem to expand that to CMA? Or am I missing something. >>> >>> Yeah, you are right. Originally, I thought preventing merging isolated >>> pageblock >>> with other types of pageblocks is sufficient, since MIGRATE_CMA is always >>> converted from MIGRATE_ISOLATE. But that is not true. I will rework the >>> code. >>> Thanks for pointing this out. >>> >> >> 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? How about MIGRATE_HIGHATOMIC? It should not be merged with other migratetypes from my understanding. -- Best Regards, Yan, Zi signature.asc Description: OpenPGP digital signature ___ 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 11/23/21 17:35, Zi Yan wrote: > On 19 Nov 2021, at 10:15, Zi Yan wrote: From what my understanding, cma required alignment of max(MAX_ORDER - 1, pageblock_order), because when MIGRATE_CMA was introduced, __free_one_page() does not prevent merging two different pageblocks, when MAX_ORDER - 1 > pageblock_order. But current __free_one_page() implementation does prevent that. >>> >>> But it does prevent that only for isolated pageblock, not CMA, and yout >>> patchset doesn't seem to expand that to CMA? Or am I missing something. >> >> Yeah, you are right. Originally, I thought preventing merging isolated >> pageblock >> with other types of pageblocks is sufficient, since MIGRATE_CMA is always >> converted from MIGRATE_ISOLATE. But that is not true. I will rework the code. >> Thanks for pointing this out. >> > > 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. ___ 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 17.11.21 04:04, Zi Yan wrote: > On 16 Nov 2021, at 3:58, David Hildenbrand wrote: > >> On 15.11.21 20:37, Zi Yan wrote: >>> From: Zi Yan >>> >>> Hi David, >> >> Hi, >> >> thanks for looking into this. >> Hi, sorry for the delay, I wasn't "actually working" last week, so now I'm back from holiday :) >>> >>> You suggested to make alloc_contig_range() deal with pageblock_order >>> instead of >>> MAX_ORDER - 1 and get rid of MAX_ORDER - 1 dependency in virtio_mem[1]. This >>> patchset is my attempt to achieve that. Please take a look and let me know >>> if >>> I am doing it correctly or not. >>> >>> From what my understanding, cma required alignment of >>> max(MAX_ORDER - 1, pageblock_order), because when MIGRATE_CMA was >>> introduced, >>> __free_one_page() does not prevent merging two different pageblocks, when >>> MAX_ORDER - 1 > pageblock_order. But current __free_one_page() >>> implementation >>> does prevent that. It should be OK to just align cma to pageblock_order. >>> alloc_contig_range() relies on MIGRATE_CMA to get free pages, so it can use >>> pageblock_order as alignment too. >> >> I wonder if that's sufficient. Especially the outer_start logic in >> alloc_contig_range() might be problematic. There are some ugly corner >> cases with free pages/allocations spanning multiple pageblocks and we >> only isolated a single pageblock. > > Thank you a lot for writing the list of these corner cases. They are > very helpful! > >> >> >> Regarding CMA, we have to keep the following cases working: >> >> a) Different pageblock types (MIGRATE_CMA and !MIGRATE_CMA) in MAX_ORDER >>- 1 page: >>[ MAX_ ORDER - 1 ] >>[ pageblock 0 | pageblock 1] >> >> Assume either pageblock 0 is MIGRATE_CMA or pageblock 1 is MIGRATE_CMA, >> but not both. We have to make sure alloc_contig_range() keeps working >> correctly. This should be the case even with your change, as we won't >> merging pages accross differing migratetypes. > > Yes. > >> >> b) Migrating/freeing a MAX_ ORDER - 1 page while partially isolated: >>[ MAX_ ORDER - 1 ] >>[ pageblock 0 | pageblock 1] >> >> Assume both are MIGRATE_CMA. Assume we want to either allocate from >> pageblock 0 or pageblock 1. Especially, assume we want to allocate from >> pageblock 1. While we would isolate pageblock 1, we wouldn't isolate >> pageblock 0. >> >> What happens if we either have a free page spanning the MAX_ORDER - 1 >> range already OR if we have to migrate a MAX_ORDER - 1 page, resulting >> in a free MAX_ORDER - 1 page of which only the second pageblock is >> isolated? We would end up essentially freeing a page that has mixed >> pageblocks, essentially placing it in !MIGRATE_ISOLATE free lists ... I >> might be wrong but I have the feeling that this would be problematic. >> > > This could happen when start_isolate_page_range() stumbles upon a compound > page with order >= pageblock_order or a free page with order >= > pageblock_order, but should not. start_isolate_page_range() should check > the actual page size, either compound page size or free page size, and set > the migratetype across pageblocks if the page is bigger than pageblock size. > More precisely set_migratetype_isolate() should do that. Right -- but then we have to extend the isolation range from within set_migratetype_isolate() :/ E.g., how should we know what we have to unisolate later .. > > >> c) Concurrent allocations: >> [ MAX_ ORDER - 1 ] >> [ pageblock 0 | pageblock 1] >> >> Assume b) but we have two concurrent CMA allocations to pageblock 0 and >> pageblock 1, which would now be possible as start_isolate_page_range() >> isolate would succeed on both. > > Two isolations will be serialized by the zone lock taken by > set_migratetype_isolate(), so the concurrent allocation would not be a > problem. > If it is a MAX_ORDER-1 free page, the first comer should split it and only > isolate one of the pageblock then second one can isolate the other pageblock. Right, the issue is essentially b) above. > If it is a MAX_ORDER-1 compound page, the first comer should isolate both > pageblocks, then the second one would fail. WDYT? Possibly we could even have two independent CMA areas "colliding" within a MAX_ ORDER - 1 page. I guess the surprise would be getting an "-EAGAIN" from isolation, but the caller should properly handle that. Maybe it's really easier to do something along the lines I proposed below and always isolate the complete MAX_ORDER-1 range in alloc_contig_range(). We just have to "fix" isolation as I drafted. > > > In sum, it seems to me that the issue is page isolation code only sees > pageblock without check the actual page. When there are multiple pageblocks > belonging to one page, the problem appears. This should be fixed. > >> >> >> Regarding virtio-mem, we care about the following cases: >> >> a) Allocating parts from completely movable MAX_ ORDER - 1 page: >>[ MAX_ ORDER - 1 ] >>[ pageblock
Re: [RFC PATCH 0/3] Use pageblock_order for cma and alloc_contig_range alignment.
On 19 Nov 2021, at 10:15, Zi Yan wrote: > On 19 Nov 2021, at 7:33, Vlastimil Babka wrote: > >> On 11/15/21 20:37, Zi Yan wrote: >>> From: Zi Yan >>> >>> Hi David, >>> >>> You suggested to make alloc_contig_range() deal with pageblock_order >>> instead of >>> MAX_ORDER - 1 and get rid of MAX_ORDER - 1 dependency in virtio_mem[1]. This >>> patchset is my attempt to achieve that. Please take a look and let me know >>> if >>> I am doing it correctly or not. >>> >>> From what my understanding, cma required alignment of >>> max(MAX_ORDER - 1, pageblock_order), because when MIGRATE_CMA was >>> introduced, >>> __free_one_page() does not prevent merging two different pageblocks, when >>> MAX_ORDER - 1 > pageblock_order. But current __free_one_page() >>> implementation >>> does prevent that. >> >> But it does prevent that only for isolated pageblock, not CMA, and yout >> patchset doesn't seem to expand that to CMA? Or am I missing something. > > Yeah, you are right. Originally, I thought preventing merging isolated > pageblock > with other types of pageblocks is sufficient, since MIGRATE_CMA is always > converted from MIGRATE_ISOLATE. But that is not true. I will rework the code. > Thanks for pointing this out. > 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. I am not sure what consequence it will cause. Do you have any idea? I will fix it in the next version of this patchset. >> >> >>> It should be OK to just align cma to pageblock_order. >>> alloc_contig_range() relies on MIGRATE_CMA to get free pages, so it can use >>> pageblock_order as alignment too. >>> >>> In terms of virtio_mem, if I understand correctly, it relies on >>> alloc_contig_range() to obtain contiguous free pages and offlines them to >>> reduce >>> guest memory size. As the result of alloc_contig_range() alignment change, >>> virtio_mem should be able to just align PFNs to pageblock_order. >>> >>> Thanks. >>> >>> >>> [1] >>> https://lore.kernel.org/linux-mm/28b57903-fae6-47ac-7e1b-a1dd41421...@redhat.com/ >>> >>> Zi Yan (3): >>> mm: cma: alloc_contig_range: use pageblock_order as the single >>> alignment. >>> drivers: virtio_mem: use pageblock size as the minimum virtio_mem >>> size. >>> arch: powerpc: adjust fadump alignment to be pageblock aligned. >>> >>> arch/powerpc/include/asm/fadump-internal.h | 4 +--- >>> drivers/virtio/virtio_mem.c| 6 ++ >>> include/linux/mmzone.h | 5 + >>> kernel/dma/contiguous.c| 2 +- >>> mm/cma.c | 6 ++ >>> mm/page_alloc.c| 12 +--- >>> 6 files changed, 12 insertions(+), 23 deletions(-) >>> > > -- > Best Regards, > Yan, Zi -- Best Regards, Yan, Zi signature.asc Description: OpenPGP digital signature ___ 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 19 Nov 2021, at 7:33, Vlastimil Babka wrote: > On 11/15/21 20:37, Zi Yan wrote: >> From: Zi Yan >> >> Hi David, >> >> You suggested to make alloc_contig_range() deal with pageblock_order instead >> of >> MAX_ORDER - 1 and get rid of MAX_ORDER - 1 dependency in virtio_mem[1]. This >> patchset is my attempt to achieve that. Please take a look and let me know if >> I am doing it correctly or not. >> >> From what my understanding, cma required alignment of >> max(MAX_ORDER - 1, pageblock_order), because when MIGRATE_CMA was introduced, >> __free_one_page() does not prevent merging two different pageblocks, when >> MAX_ORDER - 1 > pageblock_order. But current __free_one_page() implementation >> does prevent that. > > But it does prevent that only for isolated pageblock, not CMA, and yout > patchset doesn't seem to expand that to CMA? Or am I missing something. Yeah, you are right. Originally, I thought preventing merging isolated pageblock with other types of pageblocks is sufficient, since MIGRATE_CMA is always converted from MIGRATE_ISOLATE. But that is not true. I will rework the code. Thanks for pointing this out. > > >> It should be OK to just align cma to pageblock_order. >> alloc_contig_range() relies on MIGRATE_CMA to get free pages, so it can use >> pageblock_order as alignment too. >> >> In terms of virtio_mem, if I understand correctly, it relies on >> alloc_contig_range() to obtain contiguous free pages and offlines them to >> reduce >> guest memory size. As the result of alloc_contig_range() alignment change, >> virtio_mem should be able to just align PFNs to pageblock_order. >> >> Thanks. >> >> >> [1] >> https://lore.kernel.org/linux-mm/28b57903-fae6-47ac-7e1b-a1dd41421...@redhat.com/ >> >> Zi Yan (3): >> mm: cma: alloc_contig_range: use pageblock_order as the single >> alignment. >> drivers: virtio_mem: use pageblock size as the minimum virtio_mem >> size. >> arch: powerpc: adjust fadump alignment to be pageblock aligned. >> >> arch/powerpc/include/asm/fadump-internal.h | 4 +--- >> drivers/virtio/virtio_mem.c| 6 ++ >> include/linux/mmzone.h | 5 + >> kernel/dma/contiguous.c| 2 +- >> mm/cma.c | 6 ++ >> mm/page_alloc.c| 12 +--- >> 6 files changed, 12 insertions(+), 23 deletions(-) >> -- Best Regards, Yan, Zi signature.asc Description: OpenPGP digital signature ___ 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 11/15/21 20:37, Zi Yan wrote: > From: Zi Yan > > Hi David, > > You suggested to make alloc_contig_range() deal with pageblock_order instead > of > MAX_ORDER - 1 and get rid of MAX_ORDER - 1 dependency in virtio_mem[1]. This > patchset is my attempt to achieve that. Please take a look and let me know if > I am doing it correctly or not. > > From what my understanding, cma required alignment of > max(MAX_ORDER - 1, pageblock_order), because when MIGRATE_CMA was introduced, > __free_one_page() does not prevent merging two different pageblocks, when > MAX_ORDER - 1 > pageblock_order. But current __free_one_page() implementation > does prevent that. But it does prevent that only for isolated pageblock, not CMA, and yout patchset doesn't seem to expand that to CMA? Or am I missing something. > It should be OK to just align cma to pageblock_order. > alloc_contig_range() relies on MIGRATE_CMA to get free pages, so it can use > pageblock_order as alignment too. > > In terms of virtio_mem, if I understand correctly, it relies on > alloc_contig_range() to obtain contiguous free pages and offlines them to > reduce > guest memory size. As the result of alloc_contig_range() alignment change, > virtio_mem should be able to just align PFNs to pageblock_order. > > Thanks. > > > [1] > https://lore.kernel.org/linux-mm/28b57903-fae6-47ac-7e1b-a1dd41421...@redhat.com/ > > Zi Yan (3): > mm: cma: alloc_contig_range: use pageblock_order as the single > alignment. > drivers: virtio_mem: use pageblock size as the minimum virtio_mem > size. > arch: powerpc: adjust fadump alignment to be pageblock aligned. > > arch/powerpc/include/asm/fadump-internal.h | 4 +--- > drivers/virtio/virtio_mem.c| 6 ++ > include/linux/mmzone.h | 5 + > kernel/dma/contiguous.c| 2 +- > mm/cma.c | 6 ++ > mm/page_alloc.c| 12 +--- > 6 files changed, 12 insertions(+), 23 deletions(-) > ___ 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 16 Nov 2021, at 3:58, David Hildenbrand wrote: > On 15.11.21 20:37, Zi Yan wrote: >> From: Zi Yan >> >> Hi David, > > Hi, > > thanks for looking into this. > >> >> You suggested to make alloc_contig_range() deal with pageblock_order instead >> of >> MAX_ORDER - 1 and get rid of MAX_ORDER - 1 dependency in virtio_mem[1]. This >> patchset is my attempt to achieve that. Please take a look and let me know if >> I am doing it correctly or not. >> >> From what my understanding, cma required alignment of >> max(MAX_ORDER - 1, pageblock_order), because when MIGRATE_CMA was introduced, >> __free_one_page() does not prevent merging two different pageblocks, when >> MAX_ORDER - 1 > pageblock_order. But current __free_one_page() implementation >> does prevent that. It should be OK to just align cma to pageblock_order. >> alloc_contig_range() relies on MIGRATE_CMA to get free pages, so it can use >> pageblock_order as alignment too. > > I wonder if that's sufficient. Especially the outer_start logic in > alloc_contig_range() might be problematic. There are some ugly corner > cases with free pages/allocations spanning multiple pageblocks and we > only isolated a single pageblock. Thank you a lot for writing the list of these corner cases. They are very helpful! > > > Regarding CMA, we have to keep the following cases working: > > a) Different pageblock types (MIGRATE_CMA and !MIGRATE_CMA) in MAX_ORDER >- 1 page: >[ MAX_ ORDER - 1 ] >[ pageblock 0 | pageblock 1] > > Assume either pageblock 0 is MIGRATE_CMA or pageblock 1 is MIGRATE_CMA, > but not both. We have to make sure alloc_contig_range() keeps working > correctly. This should be the case even with your change, as we won't > merging pages accross differing migratetypes. Yes. > > b) Migrating/freeing a MAX_ ORDER - 1 page while partially isolated: >[ MAX_ ORDER - 1 ] >[ pageblock 0 | pageblock 1] > > Assume both are MIGRATE_CMA. Assume we want to either allocate from > pageblock 0 or pageblock 1. Especially, assume we want to allocate from > pageblock 1. While we would isolate pageblock 1, we wouldn't isolate > pageblock 0. > > What happens if we either have a free page spanning the MAX_ORDER - 1 > range already OR if we have to migrate a MAX_ORDER - 1 page, resulting > in a free MAX_ORDER - 1 page of which only the second pageblock is > isolated? We would end up essentially freeing a page that has mixed > pageblocks, essentially placing it in !MIGRATE_ISOLATE free lists ... I > might be wrong but I have the feeling that this would be problematic. > This could happen when start_isolate_page_range() stumbles upon a compound page with order >= pageblock_order or a free page with order >= pageblock_order, but should not. start_isolate_page_range() should check the actual page size, either compound page size or free page size, and set the migratetype across pageblocks if the page is bigger than pageblock size. More precisely set_migratetype_isolate() should do that. > c) Concurrent allocations: > [ MAX_ ORDER - 1 ] > [ pageblock 0 | pageblock 1] > > Assume b) but we have two concurrent CMA allocations to pageblock 0 and > pageblock 1, which would now be possible as start_isolate_page_range() > isolate would succeed on both. Two isolations will be serialized by the zone lock taken by set_migratetype_isolate(), so the concurrent allocation would not be a problem. If it is a MAX_ORDER-1 free page, the first comer should split it and only isolate one of the pageblock then second one can isolate the other pageblock. If it is a MAX_ORDER-1 compound page, the first comer should isolate both pageblocks, then the second one would fail. WDYT? In sum, it seems to me that the issue is page isolation code only sees pageblock without check the actual page. When there are multiple pageblocks belonging to one page, the problem appears. This should be fixed. > > > Regarding virtio-mem, we care about the following cases: > > a) Allocating parts from completely movable MAX_ ORDER - 1 page: >[ MAX_ ORDER - 1 ] >[ pageblock 0 | pageblock 1] > > Assume pageblock 0 and pageblock 1 are either free or contain only > movable pages. Assume we allocated pageblock 0. We have to make sure we > can allocate pageblock 1. The other way around, assume we allocated > pageblock 1, we have to make sure we can allocate pageblock 0. > > Free pages spanning both pageblocks might be problematic. Can you elaborate a bit? If either of pageblock 0 and 1 is used by virtio-mem, why do we care the other? If pageblock 0 and 1 belong to the same page (either free or compound), they should have the same migratetype. If we want to just allocate one of them, we can split the free page or migrate the compound page then split the remaining free page. > > b) Allocate parts of partially movable MAX_ ORDER - 1 page: >[ MAX_ ORDER - 1 ] >[ pageblock 0 | pageblock 1] > > Assume pageblock 0 contains unmovable data but
Re: [RFC PATCH 0/3] Use pageblock_order for cma and alloc_contig_range alignment.
On 15.11.21 20:37, Zi Yan wrote: > From: Zi Yan > > Hi David, Hi, thanks for looking into this. > > You suggested to make alloc_contig_range() deal with pageblock_order instead > of > MAX_ORDER - 1 and get rid of MAX_ORDER - 1 dependency in virtio_mem[1]. This > patchset is my attempt to achieve that. Please take a look and let me know if > I am doing it correctly or not. > > From what my understanding, cma required alignment of > max(MAX_ORDER - 1, pageblock_order), because when MIGRATE_CMA was introduced, > __free_one_page() does not prevent merging two different pageblocks, when > MAX_ORDER - 1 > pageblock_order. But current __free_one_page() implementation > does prevent that. It should be OK to just align cma to pageblock_order. > alloc_contig_range() relies on MIGRATE_CMA to get free pages, so it can use > pageblock_order as alignment too. I wonder if that's sufficient. Especially the outer_start logic in alloc_contig_range() might be problematic. There are some ugly corner cases with free pages/allocations spanning multiple pageblocks and we only isolated a single pageblock. Regarding CMA, we have to keep the following cases working: a) Different pageblock types (MIGRATE_CMA and !MIGRATE_CMA) in MAX_ORDER - 1 page: [ MAX_ ORDER - 1 ] [ pageblock 0 | pageblock 1] Assume either pageblock 0 is MIGRATE_CMA or pageblock 1 is MIGRATE_CMA, but not both. We have to make sure alloc_contig_range() keeps working correctly. This should be the case even with your change, as we won't merging pages accross differing migratetypes. b) Migrating/freeing a MAX_ ORDER - 1 page while partially isolated: [ MAX_ ORDER - 1 ] [ pageblock 0 | pageblock 1] Assume both are MIGRATE_CMA. Assume we want to either allocate from pageblock 0 or pageblock 1. Especially, assume we want to allocate from pageblock 1. While we would isolate pageblock 1, we wouldn't isolate pageblock 0. What happens if we either have a free page spanning the MAX_ORDER - 1 range already OR if we have to migrate a MAX_ORDER - 1 page, resulting in a free MAX_ORDER - 1 page of which only the second pageblock is isolated? We would end up essentially freeing a page that has mixed pageblocks, essentially placing it in !MIGRATE_ISOLATE free lists ... I might be wrong but I have the feeling that this would be problematic. c) Concurrent allocations: [ MAX_ ORDER - 1 ] [ pageblock 0 | pageblock 1] Assume b) but we have two concurrent CMA allocations to pageblock 0 and pageblock 1, which would now be possible as start_isolate_page_range() isolate would succeed on both. Regarding virtio-mem, we care about the following cases: a) Allocating parts from completely movable MAX_ ORDER - 1 page: [ MAX_ ORDER - 1 ] [ pageblock 0 | pageblock 1] Assume pageblock 0 and pageblock 1 are either free or contain only movable pages. Assume we allocated pageblock 0. We have to make sure we can allocate pageblock 1. The other way around, assume we allocated pageblock 1, we have to make sure we can allocate pageblock 0. Free pages spanning both pageblocks might be problematic. b) Allocate parts of partially movable MAX_ ORDER - 1 page: [ MAX_ ORDER - 1 ] [ pageblock 0 | pageblock 1] Assume pageblock 0 contains unmovable data but pageblock 1 not: we have to make sure we can allocate pageblock 1. Similarly, assume pageblock 1 contains unmovable data but pageblock 0 no: we have to make sure we can allocate pageblock 1. has_unmovable_pages() might allow for that. But, we want to fail early in case we want to allocate a single pageblock but it contains unmovable data. This could be either directly or indirectly. If we have an unmovable (compound) MAX_ ORDER - 1 and we'd try isolating pageblock 1, has_unmovable_pages() would always return "false" because we'd simply be skiping over any tail pages, and not detect the un-movability. c) Migrating/freeing a MAX_ ORDER - 1 page while partially isolated: Same concern as for CMA b) So the biggest concern I have is dealing with migrating/freeing > pageblock_order pages while only having isolated a single pageblock. > > In terms of virtio_mem, if I understand correctly, it relies on > alloc_contig_range() to obtain contiguous free pages and offlines them to > reduce > guest memory size. As the result of alloc_contig_range() alignment change, > virtio_mem should be able to just align PFNs to pageblock_order. For virtio-mem it will most probably be desirable to first try allocating the MAX_ORDER -1 range if possible and then fallback to pageblock_order. But that's an additional change on top in virtio-mem code. My take to teach alloc_contig_range() to properly handle would be the following: a) Convert MIGRATE_ISOLATE into a separate pageblock flag We would want to convert MIGRATE_ISOLATE into a separate pageblock flags, such that when we isolate a page block we preserve the original migratetype. While start_isolate_page_range()
[RFC PATCH 0/3] Use pageblock_order for cma and alloc_contig_range alignment.
From: Zi Yan Hi David, You suggested to make alloc_contig_range() deal with pageblock_order instead of MAX_ORDER - 1 and get rid of MAX_ORDER - 1 dependency in virtio_mem[1]. This patchset is my attempt to achieve that. Please take a look and let me know if I am doing it correctly or not. >From what my understanding, cma required alignment of max(MAX_ORDER - 1, pageblock_order), because when MIGRATE_CMA was introduced, __free_one_page() does not prevent merging two different pageblocks, when MAX_ORDER - 1 > pageblock_order. But current __free_one_page() implementation does prevent that. It should be OK to just align cma to pageblock_order. alloc_contig_range() relies on MIGRATE_CMA to get free pages, so it can use pageblock_order as alignment too. In terms of virtio_mem, if I understand correctly, it relies on alloc_contig_range() to obtain contiguous free pages and offlines them to reduce guest memory size. As the result of alloc_contig_range() alignment change, virtio_mem should be able to just align PFNs to pageblock_order. Thanks. [1] https://lore.kernel.org/linux-mm/28b57903-fae6-47ac-7e1b-a1dd41421...@redhat.com/ Zi Yan (3): mm: cma: alloc_contig_range: use pageblock_order as the single alignment. drivers: virtio_mem: use pageblock size as the minimum virtio_mem size. arch: powerpc: adjust fadump alignment to be pageblock aligned. arch/powerpc/include/asm/fadump-internal.h | 4 +--- drivers/virtio/virtio_mem.c| 6 ++ include/linux/mmzone.h | 5 + kernel/dma/contiguous.c| 2 +- mm/cma.c | 6 ++ mm/page_alloc.c| 12 +--- 6 files changed, 12 insertions(+), 23 deletions(-) -- 2.33.0 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
