Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1620863 > unrolled thread
| Started by | js1304@gmail.com |
|---|---|
| First post | 2017-04-11 05:20 +0200 |
| Last post | 2017-04-24 06:10 +0200 |
| Articles | 20 on this page of 27 — 6 participants |
Back to article view | Back to linux.kernel
[PATCH v7 0/7] Introduce ZONE_CMA js1304@gmail.com - 2017-04-11 05:20 +0200
[PATCH v7 7/7] ARM: CMA: avoid re-mapping CMA region if CONFIG_HIGHMEM js1304@gmail.com - 2017-04-11 05:20 +0200
[PATCH v7 5/7] mm/cma: remove MIGRATE_CMA js1304@gmail.com - 2017-04-11 05:20 +0200
Re: [PATCH v7 0/7] Introduce ZONE_CMA Michal Hocko <mhocko@kernel.org> - 2017-04-11 20:20 +0200
Re: [PATCH v7 0/7] Introduce ZONE_CMA Joonsoo Kim <js1304@gmail.com> - 2017-04-12 03:40 +0200
Re: [PATCH v7 0/7] Introduce ZONE_CMA Michal Hocko <mhocko@kernel.org> - 2017-04-13 14:00 +0200
Re: [PATCH v7 0/7] Introduce ZONE_CMA Joonsoo Kim <js1304@gmail.com> - 2017-04-17 04:10 +0200
Re: [PATCH v7 0/7] Introduce ZONE_CMA Joonsoo Kim <js1304@gmail.com> - 2017-04-21 03:40 +0200
Re: [PATCH v7 0/7] Introduce ZONE_CMA Michal Hocko <mhocko@kernel.org> - 2017-04-21 09:00 +0200
Re: [PATCH v7 0/7] Introduce ZONE_CMA Michal Hocko <mhocko@kernel.org> - 2017-04-24 15:20 +0200
Re: [PATCH v7 0/7] Introduce ZONE_CMA Joonsoo Kim <js1304@gmail.com> - 2017-04-25 05:50 +0200
Re: [PATCH v7 0/7] Introduce ZONE_CMA Michal Hocko <mhocko@kernel.org> - 2017-04-27 17:10 +0200
Generic approach to customizable zones - was: Re: [PATCH v7 0/7] Introduce ZONE_CMA Igor Stoppa <igor.stoppa@huawei.com> - 2017-04-28 10:10 +0200
Re: Generic approach to customizable zones - was: Re: [PATCH v7 0/7] Introduce ZONE_CMA Michal Hocko <mhocko@kernel.org> - 2017-04-28 10:40 +0200
Re: Generic approach to customizable zones - was: Re: [PATCH v7 0/7] Introduce ZONE_CMA Igor Stoppa <igor.stoppa@huawei.com> - 2017-04-28 11:10 +0200
Re: [PATCH v7 0/7] Introduce ZONE_CMA Joonsoo Kim <js1304@gmail.com> - 2017-05-02 06:10 +0200
Re: [PATCH v7 0/7] Introduce ZONE_CMA Michal Hocko <mhocko@kernel.org> - 2017-05-02 15:40 +0200
Re: [PATCH v7 0/7] Introduce ZONE_CMA Joonsoo Kim <js1304@gmail.com> - 2017-05-11 04:20 +0200
Re: [PATCH v7 0/7] Introduce ZONE_CMA Michal Hocko <mhocko@kernel.org> - 2017-05-11 11:20 +0200
Re: [PATCH v7 0/7] Introduce ZONE_CMA Vlastimil Babka <vbabka@suse.cz> - 2017-05-02 10:10 +0200
Re: [PATCH v7 0/7] Introduce ZONE_CMA Michal Hocko <mhocko@kernel.org> - 2017-05-02 15:10 +0200
Re: [PATCH v7 0/7] Introduce ZONE_CMA Igor Stoppa <igor.stoppa@huawei.com> - 2017-05-02 15:50 +0200
Re: [PATCH v7 0/7] Introduce ZONE_CMA Vlastimil Babka <vbabka@suse.cz> - 2017-05-04 14:40 +0200
Re: [PATCH v7 0/7] Introduce ZONE_CMA Michal Hocko <mhocko@kernel.org> - 2017-05-04 14:50 +0200
Re: [PATCH v7 0/7] Introduce ZONE_CMA Vlastimil Babka <vbabka@suse.cz> - 2017-05-11 11:00 +0200
Re: [PATCH v7 0/7] Introduce ZONE_CMA Joonsoo Kim <js1304@gmail.com> - 2017-04-12 03:40 +0200
Re: [PATCH v7 0/7] Introduce ZONE_CMA Bob Liu <liubo95@huawei.com> - 2017-04-24 06:10 +0200
Page 1 of 2 [1] 2 Next page →
| From | js1304@gmail.com |
|---|---|
| Date | 2017-04-11 05:20 +0200 |
| Subject | [PATCH v7 0/7] Introduce ZONE_CMA |
| Message-ID | <tuUvw-1sy-3@gated-at.bofh.it> |
From: Joonsoo Kim <iamjoonsoo.kim@lge.com> Changed from v6 o Rebase on next-20170405 o Add a fix for lowmem mapping on ARM (last patch) o Re-organize the cover letter Changes from v5 o Rebase on next-20161013 o Cosmetic change on patch 1 o Optimize span of ZONE_CMA on multiple node system Changes from v4 o Rebase on next-20160825 o Add general fix patch for lowmem reserve o Fix lowmem reserve ratio o Fix zone span optimizaion per Vlastimil o Fix pageset initialization o Change invocation timing on cma_init_reserved_areas() Changes from v3 o Rebase on next-20160805 o Split first patch per Vlastimil o Remove useless function parameter per Vlastimil o Add code comment per Vlastimil o Add following description on cover-letter Changes from v2 o Rebase on next-20160525 o No other changes except following description Changes from v1 o Separate some patches which deserve to submit independently o Modify description to reflect current kernel state (e.g. high-order watermark problem disappeared by Mel's work) o Don't increase SECTION_SIZE_BITS to make a room in page flags (detailed reason is on the patch that adds ZONE_CMA) o Adjust ZONE_CMA population code Hello, This is the 7th version of ZONE_CMA patchset. One patch is added to fix potential problem on ARM. Other changes are just due to rebase. This patchset has long history and got some reviews before. This cover-letter has the summary and my opinion on those reviews. Content order is so confusing so I make a simple index. If anyone want to understand the history properly, please read them by reverse order. PART 1. Strong points of the zone approach PART 2. Summary in LSF/MM 2016 discussion PART 3. Original motivation of this patchset ***** PART 1 ***** CMA has many problems and I mentioned them on the bottom of the cover letter. These problems comes from limitation of CMA memory that should be always migratable for device usage. I think that introducing a new zone is the best approach to solve them. Here are the reasons. Zone is introduced to solve some issues due to H/W addressing limitation. MM subsystem is implemented to work efficiently with these zones. Allocation/reclaim logic in MM consider this limitation very much. What I did in this patchset is introducing a new zone and extending zone's concept slightly. New concept is that zone can have not only H/W addressing limitation but also S/W limitation to guarantee page migration. This concept is originated from ZONE_MOVABLE and it works well for a long time. So, ZONE_CMA should not be special at this moment. There is a major concern from Mel that ZONE_MOVABLE which has S/W limitation causes highmem/lowmem problem. Highmem/lowmem problem is that some of memory cannot be usable for kernel memory due to limitation of the zone. It causes to break LRU ordering and makes hard to find kernel usable memory when memory pressure. However, important point is that this problem doesn't come from implementation detail (ZONE_MOVABLE/MIGRATETYPE). Even if we implement it by MIGRATETYPE instead of by ZONE_MOVABLE, we cannot use that type of memory for kernel allocation because it isn't migratable. So, it will cause to break LRU ordering, too. We cannot avoid the problem in any case. Therefore, we should focus on which solution is better for maintenance and not intrusive for MM subsystem. In this viewpoint, I think that zone approach is better. As mentioned earlier, MM subsystem already have many infrastructures to deal with zone's H/W addressing limitation. Adding S/W limitation on zone concept and adding a new zone doesn't change anything. It will work by itself. My patchset can remove many hooks related to CMA area management in MM while solving the problems. More hooks are required to solve the problems if we choose MIGRATETYPE approach. Although Mel withdrew the review, Vlastimil expressed an agreement on this new zone approach [6]. "I realize I differ here from much more experienced mm guys, and will probably deservingly regret it later on, but I think that the ZONE_CMA approach could work indeed better than current MIGRATE_CMA pageblocks." If anyone has a different opinion, please let me know. Thanks. ***** PART 2 ***** There was a discussion with Mel [5] after LSF/MM 2016. I could summarise it to help merge decision but it's better to read by yourself since if I summarise it, it would be biased for me. But, if anyone hope the summary, I will do it. :) Anyway, Mel's position on this patchset seems to be neutral. He saids: "I'm not going to outright NAK your series but I won't ACK it either" We can fix the problems with any approach but I hope to go a new zone approach because it is less error-prone. It reduces some corner case handling for now and remove need for potential corner case handling to fix problems. Note that our company is already using ZONE_CMA and there is no problem. If anyone has a different opinion, please let me know and let's discuss together. Andrew, if there is something to do for merge, please let me know. ***** PART 3 ***** This series try to solve problems of current CMA implementation. CMA is introduced to provide physically contiguous pages at runtime without exclusive reserved memory area. But, current implementation works like as previous reserved memory approach, because freepages on CMA region are used only if there is no movable freepage. In other words, freepages on CMA region are only used as fallback. In that situation where freepages on CMA region are used as fallback, kswapd would be woken up easily since there is no unmovable and reclaimable freepage, too. If kswapd starts to reclaim memory, fallback allocation to MIGRATE_CMA doesn't occur any more since movable freepages are already refilled by kswapd and then most of freepage on CMA are left to be in free. This situation looks like exclusive reserved memory case. In my experiment, I found that if system memory has 1024 MB memory and 512 MB is reserved for CMA, kswapd is mostly woken up when roughly 512 MB free memory is left. Detailed reason is that for keeping enough free memory for unmovable and reclaimable allocation, kswapd uses below equation when calculating free memory and it easily go under the watermark. Free memory for unmovable and reclaimable = Free total - Free CMA pages This is derivated from the property of CMA freepage that CMA freepage can't be used for unmovable and reclaimable allocation. Anyway, in this case, kswapd are woken up when (FreeTotal - FreeCMA) is lower than low watermark and tries to make free memory until (FreeTotal - FreeCMA) is higher than high watermark. That results in that FreeTotal is moving around 512MB boundary consistently. It then means that we can't utilize full memory capacity. To fix this problem, I submitted some patches [1] about 10 months ago, but, found some more problems to be fixed before solving this problem. It requires many hooks in allocator hotpath so some developers doesn't like it. Instead, some of them suggest different approach [2] to fix all the problems related to CMA, that is, introducing a new zone to deal with free CMA pages. I agree that it is the best way to go so implement here. Although properties of ZONE_MOVABLE and ZONE_CMA is similar, I decide to add a new zone rather than piggyback on ZONE_MOVABLE since they have some differences. First, reserved CMA pages should not be offlined. If freepage for CMA is managed by ZONE_MOVABLE, we need to keep MIGRATE_CMA migratetype and insert many hooks on memory hotplug code to distiguish hotpluggable memory and reserved memory for CMA in the same zone. It would make memory hotplug code which is already complicated more complicated. Second, cma_alloc() can be called more frequently than memory hotplug operation and possibly we need to control allocation rate of ZONE_CMA to optimize latency in the future. In this case, separate zone approach is easy to modify. Third, I'd like to see statistics for CMA, separately. Sometimes, we need to debug why cma_alloc() is failed and separate statistics would be more helpful in this situtaion. Anyway, this patchset solves four problems related to CMA implementation. 1) Utilization problem As mentioned above, we can't utilize full memory capacity due to the limitation of CMA freepage and fallback policy. This patchset implements a new zone for CMA and uses it for GFP_HIGHUSER_MOVABLE request. This typed allocation is used for page cache and anonymous pages which occupies most of memory usage in normal case so we can utilize full memory capacity. Below is the experiment result about this problem. 8 CPUs, 1024 MB, VIRTUAL MACHINE make -j16 <Before this series> CMA reserve: 0 MB 512 MB Elapsed-time: 92.4 186.5 pswpin: 82 18647 pswpout: 160 69839 <After this series> CMA reserve: 0 MB 512 MB Elapsed-time: 93.1 93.4 pswpin: 84 46 pswpout: 183 92 FYI, there is another attempt [3] trying to solve this problem in lkml. And, as far as I know, Qualcomm also has out-of-tree solution for this problem. 2) Reclaim problem Currently, there is no logic to distinguish CMA pages in reclaim path. If reclaim is initiated for unmovable and reclaimable allocation, reclaiming CMA pages doesn't help to satisfy the request and reclaiming CMA page is just waste. By managing CMA pages in the new zone, we can skip to reclaim ZONE_CMA completely if it is unnecessary. 3) Atomic allocation failure problem Kswapd isn't started to reclaim pages when allocation request is movable type and there is enough free page in the CMA region. After bunch of consecutive movable allocation requests, free pages in ordinary region (not CMA region) would be exhausted without waking up kswapd. At that time, if atomic unmovable allocation comes, it can't be successful since there is not enough page in ordinary region. This problem is reported by Aneesh [4] and can be solved by this patchset. 4) Inefficiently work of compaction Usual high-order allocation request is unmovable type and it cannot be serviced from CMA area. In compaction, migration scanner doesn't distinguish migratable pages on the CMA area and do migration. In this case, even if we make high-order page on that region, it cannot be used due to type mismatch. This patch will solve this problem by separating CMA pages from ordinary zones. I passed boot test on x86_64, x86_32, arm and arm64. I did some stress tests on x86_64 and x86_32 and there is no problem. Feel free to enjoy and please give me a feedback. :) Thanks. [1] https://lkml.org/lkml/2014/5/28/64 [2] https://lkml.org/lkml/2014/11/4/55 [3] https://lkml.org/lkml/2014/10/15/623 [4] http://www.spinics.net/lists/linux-mm/msg100562.html [5] https://lkml.kernel.org/r/20160425053653.GA25662@js1304-P5Q-DELUXE [6] https://lkml.kernel.org/r/1919a85d-6e1e-374f-b8c3-1236c36b0393@suse.cz Joonsoo Kim (7): mm/page_alloc: don't reserve ZONE_HIGHMEM for ZONE_MOVABLE request mm/cma: introduce new zone, ZONE_CMA mm/cma: populate ZONE_CMA mm/cma: remove ALLOC_CMA mm/cma: remove MIGRATE_CMA mm/cma: remove per zone CMA stat ARM: CMA: avoid re-mapping CMA region if CONFIG_HIGHMEM arch/arm/mm/dma-mapping.c | 7 +- arch/powerpc/mm/mmu_context_iommu.c | 2 +- arch/x86/mm/highmem_32.c | 8 ++ fs/proc/meminfo.c | 2 +- include/linux/cma.h | 7 ++ include/linux/gfp.h | 32 +++--- include/linux/memory_hotplug.h | 3 - include/linux/mempolicy.h | 2 +- include/linux/mm.h | 1 + include/linux/mmzone.h | 60 +++++----- include/linux/page-isolation.h | 5 +- include/linux/vm_event_item.h | 10 +- include/linux/vmstat.h | 8 -- include/trace/events/mmflags.h | 10 +- kernel/power/snapshot.c | 8 ++ mm/cma.c | 78 +++++++++++-- mm/compaction.c | 12 +- mm/hugetlb.c | 3 +- mm/internal.h | 4 +- mm/memory_hotplug.c | 7 +- mm/page_alloc.c | 220 ++++++++++++++++++------------------ mm/page_isolation.c | 15 +-- mm/page_owner.c | 6 +- mm/usercopy.c | 4 +- mm/vmstat.c | 10 +- 25 files changed, 310 insertions(+), 214 deletions(-) -- 2.7.4
[toc] | [next] | [standalone]
| From | js1304@gmail.com |
|---|---|
| Date | 2017-04-11 05:20 +0200 |
| Subject | [PATCH v7 7/7] ARM: CMA: avoid re-mapping CMA region if CONFIG_HIGHMEM |
| Message-ID | <tuUvw-1sy-17@gated-at.bofh.it> |
| In reply to | #1620863 |
From: Joonsoo Kim <iamjoonsoo.kim@lge.com> CMA region is now managed by the separate zone, ZONE_CMA, to fix many MM related problems. In this implementation, it is possible that ZONE_CMA contains two CMA regions that are on the both, lowmem and highmem, respectively. To handle this case properly, ZONE_CMA is considered as highmem. In dma_contiguous_remap(), mapping for CMA region on lowmem is cleared and remapped for DMA, but, in the new CMA implementation, remap isn't needed since the region is considered as highmem. And, remap should not be allowed since it would cause cache problems. So, this patch disables it. Signed-off-by: Joonsoo Kim <iamjoonsoo.kim@lge.com> --- arch/arm/mm/dma-mapping.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/arch/arm/mm/dma-mapping.c b/arch/arm/mm/dma-mapping.c index 475811f..377053a 100644 --- a/arch/arm/mm/dma-mapping.c +++ b/arch/arm/mm/dma-mapping.c @@ -506,7 +506,12 @@ void __init dma_contiguous_remap(void) flush_tlb_kernel_range(__phys_to_virt(start), __phys_to_virt(end)); - iotable_init(&map, 1); + /* + * For highmem system, all the memory in CMA region will be + * considered as highmem, therefore, re-mapping isn't required. + */ + if (!IS_ENABLED(CONFIG_HIGHMEM)) + iotable_init(&map, 1); } } -- 2.7.4
[toc] | [prev] | [next] | [standalone]
| From | js1304@gmail.com |
|---|---|
| Date | 2017-04-11 05:20 +0200 |
| Subject | [PATCH v7 5/7] mm/cma: remove MIGRATE_CMA |
| Message-ID | <tuUvx-1sy-19@gated-at.bofh.it> |
| In reply to | #1620863 |
From: Joonsoo Kim <iamjoonsoo.kim@lge.com>
Now, all reserved pages for CMA region are belong to the ZONE_CMA
and there is no other type of pages. Therefore, we don't need to
use MIGRATE_CMA to distinguish and handle differently for CMA pages
and ordinary pages. Remove MIGRATE_CMA.
Unfortunately, this patch make free CMA counter incorrect because
we count it when pages are on the MIGRATE_CMA. It will be fixed
by next patch. I can squash next patch here but it makes changes
complicated and hard to review so I separate that.
Reviewed-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
Acked-by: Vlastimil Babka <vbabka@suse.cz>
Signed-off-by: Joonsoo Kim <iamjoonsoo.kim@lge.com>
---
arch/powerpc/mm/mmu_context_iommu.c | 2 +-
include/linux/gfp.h | 2 +-
include/linux/mmzone.h | 26 +----------
include/linux/page-isolation.h | 5 +--
include/linux/vmstat.h | 8 ----
mm/cma.c | 3 +-
mm/compaction.c | 8 +---
mm/hugetlb.c | 3 +-
mm/memory_hotplug.c | 7 ++-
mm/page_alloc.c | 86 ++++++++++---------------------------
mm/page_isolation.c | 15 +++----
mm/page_owner.c | 6 +--
mm/usercopy.c | 4 +-
13 files changed, 43 insertions(+), 132 deletions(-)
diff --git a/arch/powerpc/mm/mmu_context_iommu.c b/arch/powerpc/mm/mmu_context_iommu.c
index fc67bd7..330c495 100644
--- a/arch/powerpc/mm/mmu_context_iommu.c
+++ b/arch/powerpc/mm/mmu_context_iommu.c
@@ -184,7 +184,7 @@ long mm_iommu_get(struct mm_struct *mm, unsigned long ua, unsigned long entries,
* of the CMA zone if possible. NOTE: faulting in + migration
* can be expensive. Batching can be considered later
*/
- if (is_migrate_cma_page(page)) {
+ if (is_zone_cma(page_zone(page))) {
if (mm_iommu_move_page_from_cma(page))
goto populate;
if (1 != get_user_pages_fast(ua + (i << PAGE_SHIFT),
diff --git a/include/linux/gfp.h b/include/linux/gfp.h
index c2ed2eb..15987cc 100644
--- a/include/linux/gfp.h
+++ b/include/linux/gfp.h
@@ -563,7 +563,7 @@ static inline bool pm_suspended_storage(void)
#if (defined(CONFIG_MEMORY_ISOLATION) && defined(CONFIG_COMPACTION)) || defined(CONFIG_CMA)
/* The below functions must be run on a range from a single zone. */
extern int alloc_contig_range(unsigned long start, unsigned long end,
- unsigned migratetype, gfp_t gfp_mask);
+ gfp_t gfp_mask);
extern void free_contig_range(unsigned long pfn, unsigned nr_pages);
#endif
diff --git a/include/linux/mmzone.h b/include/linux/mmzone.h
index 74eda07..efb69b1 100644
--- a/include/linux/mmzone.h
+++ b/include/linux/mmzone.h
@@ -41,22 +41,6 @@ enum migratetype {
MIGRATE_RECLAIMABLE,
MIGRATE_PCPTYPES, /* the number of types on the pcp lists */
MIGRATE_HIGHATOMIC = MIGRATE_PCPTYPES,
-#ifdef CONFIG_CMA
- /*
- * MIGRATE_CMA migration type is designed to mimic the way
- * ZONE_MOVABLE works. Only movable pages can be allocated
- * from MIGRATE_CMA pageblocks and page allocator never
- * implicitly change migration type of MIGRATE_CMA pageblock.
- *
- * The way to use it is to change migratetype of a range of
- * pageblocks to MIGRATE_CMA which can be done by
- * __free_pageblock_cma() function. What is important though
- * is that a range of pageblocks must be aligned to
- * MAX_ORDER_NR_PAGES should biggest page be bigger then
- * a single pageblock.
- */
- MIGRATE_CMA,
-#endif
#ifdef CONFIG_MEMORY_ISOLATION
MIGRATE_ISOLATE, /* can't allocate from here */
#endif
@@ -66,17 +50,9 @@ enum migratetype {
/* In mm/page_alloc.c; keep in sync also with show_migration_types() there */
extern char * const migratetype_names[MIGRATE_TYPES];
-#ifdef CONFIG_CMA
-# define is_migrate_cma(migratetype) unlikely((migratetype) == MIGRATE_CMA)
-# define is_migrate_cma_page(_page) (get_pageblock_migratetype(_page) == MIGRATE_CMA)
-#else
-# define is_migrate_cma(migratetype) false
-# define is_migrate_cma_page(_page) false
-#endif
-
static inline bool is_migrate_movable(int mt)
{
- return is_migrate_cma(mt) || mt == MIGRATE_MOVABLE;
+ return mt == MIGRATE_MOVABLE;
}
#define for_each_migratetype_order(order, type) \
diff --git a/include/linux/page-isolation.h b/include/linux/page-isolation.h
index d4cd201..67735f2 100644
--- a/include/linux/page-isolation.h
+++ b/include/linux/page-isolation.h
@@ -46,15 +46,14 @@ int move_freepages_block(struct zone *zone, struct page *page,
*/
int
start_isolate_page_range(unsigned long start_pfn, unsigned long end_pfn,
- unsigned migratetype, bool skip_hwpoisoned_pages);
+ bool skip_hwpoisoned_pages);
/*
* Changes MIGRATE_ISOLATE to MIGRATE_MOVABLE.
* target range is [start_pfn, end_pfn)
*/
int
-undo_isolate_page_range(unsigned long start_pfn, unsigned long end_pfn,
- unsigned migratetype);
+undo_isolate_page_range(unsigned long start_pfn, unsigned long end_pfn);
/*
* Test all pages in [start_pfn, end_pfn) are isolated or not.
diff --git a/include/linux/vmstat.h b/include/linux/vmstat.h
index 6137719..ac6db88 100644
--- a/include/linux/vmstat.h
+++ b/include/linux/vmstat.h
@@ -341,14 +341,6 @@ static inline void drain_zonestat(struct zone *zone,
struct per_cpu_pageset *pset) { }
#endif /* CONFIG_SMP */
-static inline void __mod_zone_freepage_state(struct zone *zone, int nr_pages,
- int migratetype)
-{
- __mod_zone_page_state(zone, NR_FREE_PAGES, nr_pages);
- if (is_migrate_cma(migratetype))
- __mod_zone_page_state(zone, NR_FREE_CMA_PAGES, nr_pages);
-}
-
extern const char * const vmstat_text[];
#endif /* _LINUX_VMSTAT_H */
diff --git a/mm/cma.c b/mm/cma.c
index 6d8bd300..91dd85a 100644
--- a/mm/cma.c
+++ b/mm/cma.c
@@ -479,8 +479,7 @@ struct page *cma_alloc(struct cma *cma, size_t count, unsigned int align,
pfn = cma->base_pfn + (bitmap_no << cma->order_per_bit);
mutex_lock(&cma_mutex);
- ret = alloc_contig_range(pfn, pfn + count, MIGRATE_CMA,
- gfp_mask);
+ ret = alloc_contig_range(pfn, pfn + count, gfp_mask);
mutex_unlock(&cma_mutex);
if (ret == 0) {
page = pfn_to_page(pfn);
diff --git a/mm/compaction.c b/mm/compaction.c
index 80b1424..f6ae10f 100644
--- a/mm/compaction.c
+++ b/mm/compaction.c
@@ -1017,7 +1017,7 @@ static bool suitable_migration_target(struct compact_control *cc,
if (cc->ignore_block_suitable)
return true;
- /* If the block is MIGRATE_MOVABLE or MIGRATE_CMA, allow migration */
+ /* If the block is MIGRATE_MOVABLE, allow migration */
if (is_migrate_movable(get_pageblock_migratetype(page)))
return true;
@@ -1338,12 +1338,6 @@ static enum compact_result __compact_finished(struct zone *zone,
if (!list_empty(&area->free_list[migratetype]))
return COMPACT_SUCCESS;
-#ifdef CONFIG_CMA
- /* MIGRATE_MOVABLE can fallback on MIGRATE_CMA */
- if (migratetype == MIGRATE_MOVABLE &&
- !list_empty(&area->free_list[MIGRATE_CMA]))
- return COMPACT_SUCCESS;
-#endif
/*
* Job done if allocation would steal freepages from
* other migratetype buddy lists.
diff --git a/mm/hugetlb.c b/mm/hugetlb.c
index e582887..d26c837 100644
--- a/mm/hugetlb.c
+++ b/mm/hugetlb.c
@@ -1053,8 +1053,7 @@ static int __alloc_gigantic_page(unsigned long start_pfn,
unsigned long nr_pages)
{
unsigned long end_pfn = start_pfn + nr_pages;
- return alloc_contig_range(start_pfn, end_pfn, MIGRATE_MOVABLE,
- GFP_KERNEL);
+ return alloc_contig_range(start_pfn, end_pfn, GFP_KERNEL);
}
static bool pfn_range_valid_gigantic(struct zone *z,
diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
index 76d4745..c48c36f 100644
--- a/mm/memory_hotplug.c
+++ b/mm/memory_hotplug.c
@@ -1897,8 +1897,7 @@ static int __ref __offline_pages(unsigned long start_pfn,
return -EINVAL;
/* set above range as isolated */
- ret = start_isolate_page_range(start_pfn, end_pfn,
- MIGRATE_MOVABLE, true);
+ ret = start_isolate_page_range(start_pfn, end_pfn, true);
if (ret)
return ret;
@@ -1968,7 +1967,7 @@ static int __ref __offline_pages(unsigned long start_pfn,
We cannot do rollback at this point. */
offline_isolated_pages(start_pfn, end_pfn);
/* reset pagetype flags and makes migrate type to be MOVABLE */
- undo_isolate_page_range(start_pfn, end_pfn, MIGRATE_MOVABLE);
+ undo_isolate_page_range(start_pfn, end_pfn);
/* removal success */
adjust_managed_page_count(pfn_to_page(start_pfn), -offlined_pages);
zone->present_pages -= offlined_pages;
@@ -2005,7 +2004,7 @@ static int __ref __offline_pages(unsigned long start_pfn,
((unsigned long long) end_pfn << PAGE_SHIFT) - 1);
memory_notify(MEM_CANCEL_OFFLINE, &arg);
/* pushback to free area */
- undo_isolate_page_range(start_pfn, end_pfn, MIGRATE_MOVABLE);
+ undo_isolate_page_range(start_pfn, end_pfn);
return ret;
}
diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index 18f16bf..33a1b69 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -136,8 +136,8 @@ gfp_t gfp_allowed_mask __read_mostly = GFP_BOOT_MASK;
* put on a pcplist. Used to avoid the pageblock migratetype lookup when
* freeing from pcplists in most cases, at the cost of possibly becoming stale.
* Also the migratetype set in the page does not necessarily match the pcplist
- * index, e.g. page might have MIGRATE_CMA set but be on a pcplist with any
- * other index - this ensures that it will be put on the correct CMA freelist.
+ * index, e.g. page might have MIGRATE_MOVABLE set but be on a pcplist with any
+ * other index - this ensures that it will be put on the correct freelist.
*/
static inline int get_pcppage_migratetype(struct page *page)
{
@@ -247,9 +247,6 @@ char * const migratetype_names[MIGRATE_TYPES] = {
"Movable",
"Reclaimable",
"HighAtomic",
-#ifdef CONFIG_CMA
- "CMA",
-#endif
#ifdef CONFIG_MEMORY_ISOLATION
"Isolate",
#endif
@@ -681,7 +678,7 @@ static inline bool set_page_guard(struct zone *zone, struct page *page,
INIT_LIST_HEAD(&page->lru);
set_page_private(page, order);
/* Guard pages are not available for any usage */
- __mod_zone_freepage_state(zone, -(1 << order), migratetype);
+ __mod_zone_page_state(zone, NR_FREE_PAGES, -(1 << order));
return true;
}
@@ -702,7 +699,7 @@ static inline void clear_page_guard(struct zone *zone, struct page *page,
set_page_private(page, 0);
if (!is_migrate_isolate(migratetype))
- __mod_zone_freepage_state(zone, (1 << order), migratetype);
+ __mod_zone_page_state(zone, NR_FREE_PAGES, (1 << order));
}
#else
struct page_ext_operations debug_guardpage_ops;
@@ -809,7 +806,7 @@ static inline void __free_one_page(struct page *page,
VM_BUG_ON(migratetype == -1);
if (likely(!is_migrate_isolate(migratetype)))
- __mod_zone_freepage_state(zone, 1 << order, migratetype);
+ __mod_zone_page_state(zone, NR_FREE_PAGES, 1 << order);
VM_BUG_ON_PAGE(pfn & ((1 << order) - 1), page);
VM_BUG_ON_PAGE(bad_range(zone, page), page);
@@ -1591,7 +1588,7 @@ static void __init adjust_present_page_count(struct page *page, long count)
zone->present_pages += count;
}
-/* Free whole pageblock and set its migration type to MIGRATE_CMA. */
+/* Free whole pageblock and set its migration type to MIGRATE_MOVABLE. */
void __init init_cma_reserved_pageblock(struct page *page)
{
unsigned i = pageblock_nr_pages;
@@ -1616,7 +1613,7 @@ void __init init_cma_reserved_pageblock(struct page *page)
adjust_present_page_count(page, pageblock_nr_pages);
- set_pageblock_migratetype(page, MIGRATE_CMA);
+ set_pageblock_migratetype(page, MIGRATE_MOVABLE);
if (pageblock_order >= MAX_ORDER) {
i = pageblock_nr_pages;
@@ -1836,25 +1833,11 @@ static int fallbacks[MIGRATE_TYPES][4] = {
[MIGRATE_UNMOVABLE] = { MIGRATE_RECLAIMABLE, MIGRATE_MOVABLE, MIGRATE_TYPES },
[MIGRATE_RECLAIMABLE] = { MIGRATE_UNMOVABLE, MIGRATE_MOVABLE, MIGRATE_TYPES },
[MIGRATE_MOVABLE] = { MIGRATE_RECLAIMABLE, MIGRATE_UNMOVABLE, MIGRATE_TYPES },
-#ifdef CONFIG_CMA
- [MIGRATE_CMA] = { MIGRATE_TYPES }, /* Never used */
-#endif
#ifdef CONFIG_MEMORY_ISOLATION
[MIGRATE_ISOLATE] = { MIGRATE_TYPES }, /* Never used */
#endif
};
-#ifdef CONFIG_CMA
-static struct page *__rmqueue_cma_fallback(struct zone *zone,
- unsigned int order)
-{
- return __rmqueue_smallest(zone, order, MIGRATE_CMA);
-}
-#else
-static inline struct page *__rmqueue_cma_fallback(struct zone *zone,
- unsigned int order) { return NULL; }
-#endif
-
/*
* Move the free pages in a range to the free lists of the requested type.
* Note that start_page and end_pages are not aligned on a pageblock
@@ -2122,8 +2105,7 @@ static void reserve_highatomic_pageblock(struct page *page, struct zone *zone,
/* Yoink! */
mt = get_pageblock_migratetype(page);
- if (!is_migrate_highatomic(mt) && !is_migrate_isolate(mt)
- && !is_migrate_cma(mt)) {
+ if (!is_migrate_highatomic(mt) && !is_migrate_isolate(mt)) {
zone->nr_reserved_highatomic += pageblock_nr_pages;
set_pageblock_migratetype(page, MIGRATE_HIGHATOMIC);
move_freepages_block(zone, page, MIGRATE_HIGHATOMIC, NULL);
@@ -2267,13 +2249,8 @@ static struct page *__rmqueue(struct zone *zone, unsigned int order,
retry:
page = __rmqueue_smallest(zone, order, migratetype);
- if (unlikely(!page)) {
- if (migratetype == MIGRATE_MOVABLE)
- page = __rmqueue_cma_fallback(zone, order);
-
- if (!page && __rmqueue_fallback(zone, order, migratetype))
- goto retry;
- }
+ if (unlikely(!page) && __rmqueue_fallback(zone, order, migratetype))
+ goto retry;
trace_mm_page_alloc_zone_locked(page, order, migratetype);
return page;
@@ -2315,9 +2292,6 @@ static int rmqueue_bulk(struct zone *zone, unsigned int order,
list_add_tail(&page->lru, list);
list = &page->lru;
alloced++;
- if (is_migrate_cma(get_pcppage_migratetype(page)))
- __mod_zone_page_state(zone, NR_FREE_CMA_PAGES,
- -(1 << order));
}
/*
@@ -2667,7 +2641,7 @@ int __isolate_free_page(struct page *page, unsigned int order)
if (!zone_watermark_ok(zone, 0, watermark, 0, 0))
return 0;
- __mod_zone_freepage_state(zone, -(1UL << order), mt);
+ __mod_zone_page_state(zone, NR_FREE_PAGES, -(1UL << order));
}
/* Remove page from free list */
@@ -2683,8 +2657,8 @@ int __isolate_free_page(struct page *page, unsigned int order)
struct page *endpage = page + (1 << order) - 1;
for (; page < endpage; page += pageblock_nr_pages) {
int mt = get_pageblock_migratetype(page);
- if (!is_migrate_isolate(mt) && !is_migrate_cma(mt)
- && !is_migrate_highatomic(mt))
+ if (!is_migrate_isolate(mt) &&
+ !is_migrate_highatomic(mt))
set_pageblock_migratetype(page,
MIGRATE_MOVABLE);
}
@@ -2807,8 +2781,7 @@ struct page *rmqueue(struct zone *preferred_zone,
spin_unlock(&zone->lock);
if (!page)
goto failed;
- __mod_zone_freepage_state(zone, -(1 << order),
- get_pcppage_migratetype(page));
+ __mod_zone_page_state(zone, NR_FREE_PAGES, -(1 << order));
__count_zid_vm_events(PGALLOC, page_zonenum(page), 1 << order);
zone_statistics(preferred_zone, zone);
@@ -2958,11 +2931,6 @@ bool __zone_watermark_ok(struct zone *z, unsigned int order, unsigned long mark,
if (!list_empty(&area->free_list[mt]))
return true;
}
-
-#ifdef CONFIG_CMA
- if (!list_empty(&area->free_list[MIGRATE_CMA]))
- return true;
-#endif
}
return false;
}
@@ -4469,9 +4437,6 @@ static void show_migration_types(unsigned char type)
[MIGRATE_MOVABLE] = 'M',
[MIGRATE_RECLAIMABLE] = 'E',
[MIGRATE_HIGHATOMIC] = 'H',
-#ifdef CONFIG_CMA
- [MIGRATE_CMA] = 'C',
-#endif
#ifdef CONFIG_MEMORY_ISOLATION
[MIGRATE_ISOLATE] = 'I',
#endif
@@ -7361,7 +7326,7 @@ bool has_unmovable_pages(struct zone *zone, struct page *page, int count,
if (zone_idx(zone) == ZONE_MOVABLE || is_zone_cma(zone))
return false;
mt = get_pageblock_migratetype(page);
- if (mt == MIGRATE_MOVABLE || is_migrate_cma(mt))
+ if (mt == MIGRATE_MOVABLE)
return false;
pfn = page_to_pfn(page);
@@ -7512,16 +7477,12 @@ static int __alloc_contig_migrate_range(struct compact_control *cc,
* alloc_contig_range() -- tries to allocate given range of pages
* @start: start PFN to allocate
* @end: one-past-the-last PFN to allocate
- * @migratetype: migratetype of the underlaying pageblocks (either
- * #MIGRATE_MOVABLE or #MIGRATE_CMA). All pageblocks
- * in range must have the same migratetype and it must
- * be either of the two.
* @gfp_mask: GFP mask to use during compaction
*
* The PFN range does not have to be pageblock or MAX_ORDER_NR_PAGES
* aligned, however it's the caller's responsibility to guarantee that
* we are the only thread that changes migrate type of pageblocks the
- * pages fall in.
+ * pages fall in and it should be MIGRATE_MOVABLE.
*
* The PFN range must belong to a single zone.
*
@@ -7530,7 +7491,7 @@ static int __alloc_contig_migrate_range(struct compact_control *cc,
* need to be freed with free_contig_range().
*/
int alloc_contig_range(unsigned long start, unsigned long end,
- unsigned migratetype, gfp_t gfp_mask)
+ gfp_t gfp_mask)
{
unsigned long outer_start, outer_end;
unsigned int order;
@@ -7564,15 +7525,14 @@ int alloc_contig_range(unsigned long start, unsigned long end,
* allocator removing them from the buddy system. This way
* page allocator will never consider using them.
*
- * This lets us mark the pageblocks back as
- * MIGRATE_CMA/MIGRATE_MOVABLE so that free pages in the
- * aligned range but not in the unaligned, original range are
- * put back to page allocator so that buddy can use them.
+ * This lets us mark the pageblocks back as MIGRATE_MOVABLE
+ * so that free pages in the aligned range but not in the
+ * unaligned, original range are put back to page allocator
+ * so that buddy can use them.
*/
ret = start_isolate_page_range(pfn_max_align_down(start),
- pfn_max_align_up(end), migratetype,
- false);
+ pfn_max_align_up(end), false);
if (ret)
return ret;
@@ -7650,7 +7610,7 @@ int alloc_contig_range(unsigned long start, unsigned long end,
done:
undo_isolate_page_range(pfn_max_align_down(start),
- pfn_max_align_up(end), migratetype);
+ pfn_max_align_up(end));
return ret;
}
diff --git a/mm/page_isolation.c b/mm/page_isolation.c
index 5092e4e..312f2f6 100644
--- a/mm/page_isolation.c
+++ b/mm/page_isolation.c
@@ -62,14 +62,13 @@ static int set_migratetype_isolate(struct page *page,
out:
if (!ret) {
unsigned long nr_pages;
- int migratetype = get_pageblock_migratetype(page);
set_pageblock_migratetype(page, MIGRATE_ISOLATE);
zone->nr_isolate_pageblock++;
nr_pages = move_freepages_block(zone, page, MIGRATE_ISOLATE,
NULL);
- __mod_zone_freepage_state(zone, -nr_pages, migratetype);
+ __mod_zone_page_state(zone, NR_FREE_PAGES, -nr_pages);
}
spin_unlock_irqrestore(&zone->lock, flags);
@@ -122,7 +121,7 @@ static void unset_migratetype_isolate(struct page *page, unsigned migratetype)
*/
if (!isolated_page) {
nr_pages = move_freepages_block(zone, page, migratetype, NULL);
- __mod_zone_freepage_state(zone, nr_pages, migratetype);
+ __mod_zone_page_state(zone, NR_FREE_PAGES, nr_pages);
}
set_pageblock_migratetype(page, migratetype);
zone->nr_isolate_pageblock--;
@@ -151,7 +150,6 @@ __first_valid_page(unsigned long pfn, unsigned long nr_pages)
* to be MIGRATE_ISOLATE.
* @start_pfn: The lower PFN of the range to be isolated.
* @end_pfn: The upper PFN of the range to be isolated.
- * @migratetype: migrate type to set in error recovery.
*
* Making page-allocation-type to be MIGRATE_ISOLATE means free pages in
* the range will never be allocated. Any free pages and pages freed in the
@@ -161,7 +159,7 @@ __first_valid_page(unsigned long pfn, unsigned long nr_pages)
* Returns 0 on success and -EBUSY if any part of range cannot be isolated.
*/
int start_isolate_page_range(unsigned long start_pfn, unsigned long end_pfn,
- unsigned migratetype, bool skip_hwpoisoned_pages)
+ bool skip_hwpoisoned_pages)
{
unsigned long pfn;
unsigned long undo_pfn;
@@ -185,7 +183,7 @@ int start_isolate_page_range(unsigned long start_pfn, unsigned long end_pfn,
for (pfn = start_pfn;
pfn < undo_pfn;
pfn += pageblock_nr_pages)
- unset_migratetype_isolate(pfn_to_page(pfn), migratetype);
+ unset_migratetype_isolate(pfn_to_page(pfn), MIGRATE_MOVABLE);
return -EBUSY;
}
@@ -193,8 +191,7 @@ int start_isolate_page_range(unsigned long start_pfn, unsigned long end_pfn,
/*
* Make isolated pages available again.
*/
-int undo_isolate_page_range(unsigned long start_pfn, unsigned long end_pfn,
- unsigned migratetype)
+int undo_isolate_page_range(unsigned long start_pfn, unsigned long end_pfn)
{
unsigned long pfn;
struct page *page;
@@ -208,7 +205,7 @@ int undo_isolate_page_range(unsigned long start_pfn, unsigned long end_pfn,
page = __first_valid_page(pfn, pageblock_nr_pages);
if (!page || !is_migrate_isolate_page(page))
continue;
- unset_migratetype_isolate(page, migratetype);
+ unset_migratetype_isolate(page, MIGRATE_MOVABLE);
}
return 0;
}
diff --git a/mm/page_owner.c b/mm/page_owner.c
index c3cee24..4016815 100644
--- a/mm/page_owner.c
+++ b/mm/page_owner.c
@@ -299,11 +299,7 @@ void pagetypeinfo_showmixedcount_print(struct seq_file *m,
page_mt = gfpflags_to_migratetype(
page_owner->gfp_mask);
if (pageblock_mt != page_mt) {
- if (is_migrate_cma(pageblock_mt))
- count[MIGRATE_MOVABLE]++;
- else
- count[pageblock_mt]++;
-
+ count[pageblock_mt]++;
pfn = block_end_pfn;
break;
}
diff --git a/mm/usercopy.c b/mm/usercopy.c
index a9852b2..f0a2c8f 100644
--- a/mm/usercopy.c
+++ b/mm/usercopy.c
@@ -179,7 +179,7 @@ static inline const char *check_page_span(const void *ptr, unsigned long n,
* several independently allocated pages.
*/
is_reserved = PageReserved(page);
- is_cma = is_migrate_cma_page(page);
+ is_cma = is_zone_cma(page_zone(page));
if (!is_reserved && !is_cma)
return "<spans multiple pages>";
@@ -187,7 +187,7 @@ static inline const char *check_page_span(const void *ptr, unsigned long n,
page = virt_to_head_page(ptr);
if (is_reserved && !PageReserved(page))
return "<spans Reserved and non-Reserved pages>";
- if (is_cma && !is_migrate_cma_page(page))
+ if (is_cma && !is_zone_cma(page_zone(page)))
return "<spans CMA and non-CMA pages>";
}
#endif
--
2.7.4
[toc] | [prev] | [next] | [standalone]
| From | Michal Hocko <mhocko@kernel.org> |
|---|---|
| Date | 2017-04-11 20:20 +0200 |
| Message-ID | <tv8yu-27K-15@gated-at.bofh.it> |
| In reply to | #1620863 |
Hi, I didn't get to read though patches yet but the cover letter didn't really help me to understand the basic concepts to have a good starting point before diving into implementation details. It contains a lot of history remarks which is not bad but IMHO too excessive here. I would appreciate the following information (some of that is already provided in the cover but could benefit from some rewording/text reorganization). - what is ZONE_CMA and how it is configured (from admin POV) - how does ZONE_CMA compare to other zones - who is allowed to allocate from this zone and what are the guarantees/requirements for successful allocation - how does the zone compare to a preallocate allocation pool - how is ZONE_CMA balanced/reclaimed due to internal memory pressure (from CMA users) - is this zone reclaimable for the global memory reclaim - why this was/is controversial -- Michal Hocko SUSE Labs
[toc] | [prev] | [next] | [standalone]
| From | Joonsoo Kim <js1304@gmail.com> |
|---|---|
| Date | 2017-04-12 03:40 +0200 |
| Message-ID | <tvfqh-6rL-1@gated-at.bofh.it> |
| In reply to | #1621525 |
On Tue, Apr 11, 2017 at 08:15:20PM +0200, Michal Hocko wrote: > Hi, > I didn't get to read though patches yet but the cover letter didn't > really help me to understand the basic concepts to have a good starting > point before diving into implementation details. It contains a lot of > history remarks which is not bad but IMHO too excessive here. I would > appreciate the following information (some of that is already provided > in the cover but could benefit from some rewording/text reorganization). > > - what is ZONE_CMA and how it is configured (from admin POV) > - how does ZONE_CMA compare to other zones > - who is allowed to allocate from this zone and what are the > guarantees/requirements for successful allocation > - how does the zone compare to a preallocate allocation pool > - how is ZONE_CMA balanced/reclaimed due to internal memory pressure > (from CMA users) > - is this zone reclaimable for the global memory reclaim > - why this was/is controversial Hello, I hope that following summary helps you to understand this patchset. I skip some basic things about CMA. I will attach this description to the cover-letter if re-spin is needed. 1. What is ZONE_CMA ZONE_CMA is a newly introduced zone that manages freepages in CMA areas. Previously, freepages in CMA areas are in the ordinary zone and managed/distinguished by the special migratetype, MIGRATE_CMA. However, it causes too many subtle problems and fixing all the problems due to it seems to be impossible and too intrusive to MM subsystem. Therefore, different solution is requested and this is the outcome of this request. Problem details are described in PART 3. There is no change in admin POV. It is just implementation detail. If the kernel is congifured to use CMA, it is managed by MM like as before except pages are now belong to the separate zone, ZONE_CMA. 2. How does ZONE_CMA compare to other zones ZONE_CMA is conceptually the same with ZONE_MOVABLE. There is a software constraint to guarantee the success of future allocation request from the device. If the device requests the specific range of the memory in CMA area at the runtime, page that allocated by MM will be migrated to the other page and it will be returned to the device. To guarantee it, ZONE_CMA only takes the allocation request with GFP_MOVABLE. The other important point about ZONE_CMA is that span of ZONE_CMA would be overlapped with the other zone. This is not new to MM subsystem and MM subsystem has enough logic to handle such situation so there would be no problem. Other things are completely the same with other zones. For MM POV, there is no difference in allocation process except that it only takes GFP_MOVABLE request. In reclaim, pages that are allocated by MM will be reclaimed by the same policy of the MM. So, no difference. This 'no difference' is a strong point of this approach. ZONE_CMA is naturally handled by MM subsystem unlike as before (special handling is required for MIGRATE_CMA). 3. Controversial Point Major concern from Mel is that zone concept is abused. ZONE is originally introduced to solve some issues due to H/W addressing limitation. However, from the age of ZONE_MOVABLE, ZONE is used to solve the issues due to S/W limitation. This S/W limitation causes highmem/lowmem problem that is some of memory cannot be usable for kernel memory and LRU ordering would be broken easily. My major objection to this point is that this problem isn't related to implementation detail like as ZONE. Problems just comes from S/W limitation that we cannot use this memory for kernel memory to guarantee offlining the memory (ZONE_MOVABLE) or allocation from the device (ZONE_CMA) in the future. See PART 1 for more information. Thanks.
[toc] | [prev] | [next] | [standalone]
| From | Michal Hocko <mhocko@kernel.org> |
|---|---|
| Date | 2017-04-13 14:00 +0200 |
| Message-ID | <tvLzP-2vl-1@gated-at.bofh.it> |
| In reply to | #1621737 |
On Wed 12-04-17 10:35:06, Joonsoo Kim wrote: > On Tue, Apr 11, 2017 at 08:15:20PM +0200, Michal Hocko wrote: > > Hi, > > I didn't get to read though patches yet but the cover letter didn't > > really help me to understand the basic concepts to have a good starting > > point before diving into implementation details. It contains a lot of > > history remarks which is not bad but IMHO too excessive here. I would > > appreciate the following information (some of that is already provided > > in the cover but could benefit from some rewording/text reorganization). > > > > - what is ZONE_CMA and how it is configured (from admin POV) > > - how does ZONE_CMA compare to other zones > > - who is allowed to allocate from this zone and what are the > > guarantees/requirements for successful allocation > > - how does the zone compare to a preallocate allocation pool > > - how is ZONE_CMA balanced/reclaimed due to internal memory pressure > > (from CMA users) > > - is this zone reclaimable for the global memory reclaim > > - why this was/is controversial > > Hello, > > I hope that following summary helps you to understand this patchset. > I skip some basic things about CMA. I will attach this description to > the cover-letter if re-spin is needed. I believe that sorting out these questions is more important than what you have in the current cover letter. Andrew tends to fold the cover into the first patch so I think you should update. > 2. How does ZONE_CMA compare to other zones > > ZONE_CMA is conceptually the same with ZONE_MOVABLE. There is a software > constraint to guarantee the success of future allocation request from > the device. If the device requests the specific range of the memory in CMA > area at the runtime, page that allocated by MM will be migrated to > the other page and it will be returned to the device. To guarantee it, > ZONE_CMA only takes the allocation request with GFP_MOVABLE. The immediate follow up question is. Why cannot we reuse ZONE_MOVABLE for that purpose? > The other important point about ZONE_CMA is that span of ZONE_CMA would be > overlapped with the other zone. This is not new to MM subsystem and > MM subsystem has enough logic to handle such situation > so there would be no problem. I am not really sure this is actually true. Zones are disjoint from the early beginning. I remember that we had something like numa nodes interleaving but that is such a rare configuration that I wouldn't be surprised if it wasn't very well tested and actually broken in some subtle ways. There are many page_zone(page) != zone checks sprinkled in the code but I do not see anything consistent there. Similarly pageblock_pfn_to_page is only used by compaction but there are other pfn walkers which do ad-hoc checking. I was staring into that code these days due to my hotplug patches. That being said, I think that interleaving zones are an interesting concept but I would be rather nervous to consider this as working currently without a deeper review. > Other things are completely the same with other zones. For MM POV, there is > no difference in allocation process except that it only takes > GFP_MOVABLE request. In reclaim, pages that are allocated by MM will > be reclaimed by the same policy of the MM. So, no difference. OK, so essentially this is yet another "highmem" zone. We already know that only GFP_MOVABLE are allowed to fallback to ZONE_CMA but do CMA allocations fallback to other zones and punch new holes? In which zone order? > This 'no difference' is a strong point of this approach. ZONE_CMA is > naturally handled by MM subsystem unlike as before (special handling is > required for MIGRATE_CMA). > > 3. Controversial Point > > Major concern from Mel is that zone concept is abused. ZONE is originally > introduced to solve some issues due to H/W addressing limitation. Yes, very much agreed on that. You basically want to punch holes into other zones to guarantee an allocation progress. Marking those wholes with special migrate type sounds quite natural but I will have to study the current code some more to see whether issues you mention are inherently unfixable. This might very well turn out to be the case. > However, from the age of ZONE_MOVABLE, ZONE is used to solve the issues > due to S/W limitation. copying ZONE_MOVABLE pattern doesn't sound all that great to me to be honest. > This S/W limitation causes highmem/lowmem problem > that is some of memory cannot be usable for kernel memory and LRU ordering > would be broken easily. My major objection to this point is that > this problem isn't related to implementation detail like as ZONE. yes, agreement on that. > Problems just comes from S/W limitation that we cannot use this memory > for kernel memory to guarantee offlining the memory (ZONE_MOVABLE) or > allocation from the device (ZONE_CMA) in the future. See PART 1 for > more information. -- Michal Hocko SUSE Labs
[toc] | [prev] | [next] | [standalone]
| From | Joonsoo Kim <js1304@gmail.com> |
|---|---|
| Date | 2017-04-17 04:10 +0200 |
| Message-ID | <tx4h4-31r-1@gated-at.bofh.it> |
| In reply to | #1622951 |
On Thu, Apr 13, 2017 at 01:56:15PM +0200, Michal Hocko wrote: > On Wed 12-04-17 10:35:06, Joonsoo Kim wrote: > > On Tue, Apr 11, 2017 at 08:15:20PM +0200, Michal Hocko wrote: > > > Hi, > > > I didn't get to read though patches yet but the cover letter didn't > > > really help me to understand the basic concepts to have a good starting > > > point before diving into implementation details. It contains a lot of > > > history remarks which is not bad but IMHO too excessive here. I would > > > appreciate the following information (some of that is already provided > > > in the cover but could benefit from some rewording/text reorganization). > > > > > > - what is ZONE_CMA and how it is configured (from admin POV) > > > - how does ZONE_CMA compare to other zones > > > - who is allowed to allocate from this zone and what are the > > > guarantees/requirements for successful allocation > > > - how does the zone compare to a preallocate allocation pool > > > - how is ZONE_CMA balanced/reclaimed due to internal memory pressure > > > (from CMA users) > > > - is this zone reclaimable for the global memory reclaim > > > - why this was/is controversial > > > > Hello, > > > > I hope that following summary helps you to understand this patchset. > > I skip some basic things about CMA. I will attach this description to > > the cover-letter if re-spin is needed. > > I believe that sorting out these questions is more important than what > you have in the current cover letter. Andrew tends to fold the cover > into the first patch so I think you should update. Okay. > > 2. How does ZONE_CMA compare to other zones > > > > ZONE_CMA is conceptually the same with ZONE_MOVABLE. There is a software > > constraint to guarantee the success of future allocation request from > > the device. If the device requests the specific range of the memory in CMA > > area at the runtime, page that allocated by MM will be migrated to > > the other page and it will be returned to the device. To guarantee it, > > ZONE_CMA only takes the allocation request with GFP_MOVABLE. > > The immediate follow up question is. Why cannot we reuse ZONE_MOVABLE > for that purpose? I can make CMA reuses the ZONE_MOVABLE but I don't want it. Reasons are that 1. If ZONE_MOVABLE has two different types of memory, hotpluggable and CMA, it may need special handling for each type. This would lead to a new migratetype again (to distinguish them) and easy to be error-prone. I don't want that case. 2. CMA users want to see usage stat separately since CMA often causes the problems and separate stat would helps to debug it. > > The other important point about ZONE_CMA is that span of ZONE_CMA would be > > overlapped with the other zone. This is not new to MM subsystem and > > MM subsystem has enough logic to handle such situation > > so there would be no problem. > > I am not really sure this is actually true. Zones are disjoint from the > early beginning. I remember that we had something like numa nodes > interleaving but that is such a rare configuration that I wouldn't be > surprised if it wasn't very well tested and actually broken in some > subtle ways. I agree with your concern however if something is broken for them, it just shows that we need to fix it. MM should handle this situation since we already know that such architecture exists. > > There are many page_zone(page) != zone checks sprinkled in the code but > I do not see anything consistent there. Similarly pageblock_pfn_to_page > is only used by compaction but there are other pfn walkers which do > ad-hoc checking. I was staring into that code these days due to my > hotplug patches. > > That being said, I think that interleaving zones are an interesting > concept but I would be rather nervous to consider this as working > currently without a deeper review. I have tried to audit all the pfn walkers before and have added above mentioned check. Perhaps, I missed something however I believe not that much. Our production already have used ZONE_CMA and I haven't get the report about such problem. > > > Other things are completely the same with other zones. For MM POV, there is > > no difference in allocation process except that it only takes > > GFP_MOVABLE request. In reclaim, pages that are allocated by MM will > > be reclaimed by the same policy of the MM. So, no difference. > > OK, so essentially this is yet another "highmem" zone. We already know > that only GFP_MOVABLE are allowed to fallback to ZONE_CMA but do CMA > allocations fallback to other zones and punch new holes? In which zone > order? Hmm... I don't understand your question. Could you elaborate it more? > > This 'no difference' is a strong point of this approach. ZONE_CMA is > > naturally handled by MM subsystem unlike as before (special handling is > > required for MIGRATE_CMA). > > > > 3. Controversial Point > > > > Major concern from Mel is that zone concept is abused. ZONE is originally > > introduced to solve some issues due to H/W addressing limitation. > > Yes, very much agreed on that. You basically want to punch holes into > other zones to guarantee an allocation progress. Marking those wholes > with special migrate type sounds quite natural but I will have to study > the current code some more to see whether issues you mention are > inherently unfixable. This might very well turn out to be the case. At a glance, special migratetype sound natural. I also did. However, it's not natural in implementation POV. Zone consists of the same type of memory (by definition ?) and MM subsystem is implemented with that assumption. If difference type of memory shares the same zone, it easily causes the problem and CMA problems are the such case. Thanks.
[toc] | [prev] | [next] | [standalone]
| From | Joonsoo Kim <js1304@gmail.com> |
|---|---|
| Date | 2017-04-21 03:40 +0200 |
| Message-ID | <tyvId-7Y8-1@gated-at.bofh.it> |
| In reply to | #1624509 |
On Mon, Apr 17, 2017 at 11:02:12AM +0900, Joonsoo Kim wrote: > On Thu, Apr 13, 2017 at 01:56:15PM +0200, Michal Hocko wrote: > > On Wed 12-04-17 10:35:06, Joonsoo Kim wrote: > > > On Tue, Apr 11, 2017 at 08:15:20PM +0200, Michal Hocko wrote: > > > > Hi, > > > > I didn't get to read though patches yet but the cover letter didn't > > > > really help me to understand the basic concepts to have a good starting > > > > point before diving into implementation details. It contains a lot of > > > > history remarks which is not bad but IMHO too excessive here. I would > > > > appreciate the following information (some of that is already provided > > > > in the cover but could benefit from some rewording/text reorganization). > > > > > > > > - what is ZONE_CMA and how it is configured (from admin POV) > > > > - how does ZONE_CMA compare to other zones > > > > - who is allowed to allocate from this zone and what are the > > > > guarantees/requirements for successful allocation > > > > - how does the zone compare to a preallocate allocation pool > > > > - how is ZONE_CMA balanced/reclaimed due to internal memory pressure > > > > (from CMA users) > > > > - is this zone reclaimable for the global memory reclaim > > > > - why this was/is controversial > > > > > > Hello, > > > > > > I hope that following summary helps you to understand this patchset. > > > I skip some basic things about CMA. I will attach this description to > > > the cover-letter if re-spin is needed. > > > > I believe that sorting out these questions is more important than what > > you have in the current cover letter. Andrew tends to fold the cover > > into the first patch so I think you should update. > > Okay. > > > > 2. How does ZONE_CMA compare to other zones > > > > > > ZONE_CMA is conceptually the same with ZONE_MOVABLE. There is a software > > > constraint to guarantee the success of future allocation request from > > > the device. If the device requests the specific range of the memory in CMA > > > area at the runtime, page that allocated by MM will be migrated to > > > the other page and it will be returned to the device. To guarantee it, > > > ZONE_CMA only takes the allocation request with GFP_MOVABLE. > > > > The immediate follow up question is. Why cannot we reuse ZONE_MOVABLE > > for that purpose? > > I can make CMA reuses the ZONE_MOVABLE but I don't want it. Reasons > are that > > 1. If ZONE_MOVABLE has two different types of memory, hotpluggable and > CMA, it may need special handling for each type. This would lead to a new > migratetype again (to distinguish them) and easy to be error-prone. I > don't want that case. > > 2. CMA users want to see usage stat separately since CMA often causes > the problems and separate stat would helps to debug it. > > > > The other important point about ZONE_CMA is that span of ZONE_CMA would be > > > overlapped with the other zone. This is not new to MM subsystem and > > > MM subsystem has enough logic to handle such situation > > > so there would be no problem. > > > > I am not really sure this is actually true. Zones are disjoint from the > > early beginning. I remember that we had something like numa nodes > > interleaving but that is such a rare configuration that I wouldn't be > > surprised if it wasn't very well tested and actually broken in some > > subtle ways. > > I agree with your concern however if something is broken for them, it > just shows that we need to fix it. MM should handle this situation > since we already know that such architecture exists. > > > > > There are many page_zone(page) != zone checks sprinkled in the code but > > I do not see anything consistent there. Similarly pageblock_pfn_to_page > > is only used by compaction but there are other pfn walkers which do > > ad-hoc checking. I was staring into that code these days due to my > > hotplug patches. > > > > That being said, I think that interleaving zones are an interesting > > concept but I would be rather nervous to consider this as working > > currently without a deeper review. > > I have tried to audit all the pfn walkers before and have added above > mentioned check. Perhaps, I missed something however I believe not > that much. Our production already have used ZONE_CMA and I haven't get > the report about such problem. > > > > > > Other things are completely the same with other zones. For MM POV, there is > > > no difference in allocation process except that it only takes > > > GFP_MOVABLE request. In reclaim, pages that are allocated by MM will > > > be reclaimed by the same policy of the MM. So, no difference. > > > > OK, so essentially this is yet another "highmem" zone. We already know > > that only GFP_MOVABLE are allowed to fallback to ZONE_CMA but do CMA > > allocations fallback to other zones and punch new holes? In which zone > > order? > > Hmm... I don't understand your question. Could you elaborate it more? > > > > This 'no difference' is a strong point of this approach. ZONE_CMA is > > > naturally handled by MM subsystem unlike as before (special handling is > > > required for MIGRATE_CMA). > > > > > > 3. Controversial Point > > > > > > Major concern from Mel is that zone concept is abused. ZONE is originally > > > introduced to solve some issues due to H/W addressing limitation. > > > > Yes, very much agreed on that. You basically want to punch holes into > > other zones to guarantee an allocation progress. Marking those wholes > > with special migrate type sounds quite natural but I will have to study > > the current code some more to see whether issues you mention are > > inherently unfixable. This might very well turn out to be the case. > > At a glance, special migratetype sound natural. I also did. However, > it's not natural in implementation POV. Zone consists of the same type > of memory (by definition ?) and MM subsystem is implemented with that > assumption. If difference type of memory shares the same zone, it easily > causes the problem and CMA problems are the such case. Hello, Michal. If you don't have any more question, I will send next version with updated cover-letter. Thanks.
[toc] | [prev] | [next] | [standalone]
| From | Michal Hocko <mhocko@kernel.org> |
|---|---|
| Date | 2017-04-21 09:00 +0200 |
| Message-ID | <tyAHT-2AK-3@gated-at.bofh.it> |
| In reply to | #1627903 |
On Fri 21-04-17 10:35:03, Joonsoo Kim wrote: [...] > Hello, Michal. > > If you don't have any more question, I will send next version with > updated cover-letter. I am sorry but I am bussy as hell this week and didn't get to your email yet. I will try as soon as possible. -- Michal Hocko SUSE Labs
[toc] | [prev] | [next] | [standalone]
| From | Michal Hocko <mhocko@kernel.org> |
|---|---|
| Date | 2017-04-24 15:20 +0200 |
| Message-ID | <tzM4h-7tv-1@gated-at.bofh.it> |
| In reply to | #1624509 |
On Mon 17-04-17 11:02:12, Joonsoo Kim wrote: > On Thu, Apr 13, 2017 at 01:56:15PM +0200, Michal Hocko wrote: > > On Wed 12-04-17 10:35:06, Joonsoo Kim wrote: [...] > > > ZONE_CMA is conceptually the same with ZONE_MOVABLE. There is a software > > > constraint to guarantee the success of future allocation request from > > > the device. If the device requests the specific range of the memory in CMA > > > area at the runtime, page that allocated by MM will be migrated to > > > the other page and it will be returned to the device. To guarantee it, > > > ZONE_CMA only takes the allocation request with GFP_MOVABLE. > > > > The immediate follow up question is. Why cannot we reuse ZONE_MOVABLE > > for that purpose? > > I can make CMA reuses the ZONE_MOVABLE but I don't want it. Reasons > are that > > 1. If ZONE_MOVABLE has two different types of memory, hotpluggable and > CMA, it may need special handling for each type. This would lead to a new > migratetype again (to distinguish them) and easy to be error-prone. I > don't want that case. Hmm, I see your motivation. I believe that we could find a way around this. Anyway, movable zones are quite special and configuring overlapping CMA and hotplug movable regions could be refused. So I am not even sure this is a real problem in practice. > 2. CMA users want to see usage stat separately since CMA often causes > the problems and separate stat would helps to debug it. That could be solved by a per-zone/node counter. Anyway, these reasons should be mentioned as well. Adding a new zone is not for free. For most common configurations where we have ZONE_DMA, ZONE_DMA32, ZONE_NORMAL and ZONE_MOVABLE all the 3 bits are already consumed so a new zone will need a new one AFAICS. [...] > > > Other things are completely the same with other zones. For MM POV, there is > > > no difference in allocation process except that it only takes > > > GFP_MOVABLE request. In reclaim, pages that are allocated by MM will > > > be reclaimed by the same policy of the MM. So, no difference. > > > > OK, so essentially this is yet another "highmem" zone. We already know > > that only GFP_MOVABLE are allowed to fallback to ZONE_CMA but do CMA > > allocations fallback to other zones and punch new holes? In which zone > > order? > > Hmm... I don't understand your question. Could you elaborate it more? Well, my question was about the zone fallback chain. MOVABLE allocation can fallback to lower zones and also to the ZONE_CMA with your patch. If there is a CMA allocation it doesn't fall back to any other zone - in other words no new holes are punched to other zones. Is this correct? > > > This 'no difference' is a strong point of this approach. ZONE_CMA is > > > naturally handled by MM subsystem unlike as before (special handling is > > > required for MIGRATE_CMA). > > > > > > 3. Controversial Point > > > > > > Major concern from Mel is that zone concept is abused. ZONE is originally > > > introduced to solve some issues due to H/W addressing limitation. > > > > Yes, very much agreed on that. You basically want to punch holes into > > other zones to guarantee an allocation progress. Marking those wholes > > with special migrate type sounds quite natural but I will have to study > > the current code some more to see whether issues you mention are > > inherently unfixable. This might very well turn out to be the case. > > At a glance, special migratetype sound natural. I also did. However, > it's not natural in implementation POV. Zone consists of the same type > of memory (by definition ?) and MM subsystem is implemented with that > assumption. If difference type of memory shares the same zone, it easily > causes the problem and CMA problems are the such case. But this is not any different from the highmem vs. lowmem problems we already have, no? I have looked at your example in the cover where you mention utilization and the reclaim problems. With the node reclaim we will have pages from all zones on the same LRU(s). isolate_lru_pages will skip those from ZONE_CMA because their zone_idx is higher than gfp_idx(GFP_KERNEL). The same could be achieved by an explicit check for the pageblock migrate type. So the zone doesn't really help much. Or is there some aspect that I am missing? Another worry I would have with the zone approach is that there is a risk to reintroduce issues we used to have with small zones in the past. Just consider that the CMA will get depleted by CMA users almost completely. Now that zone will not get balanced with only few pages. wakeup_kswapd/pgdat_balanced already has measures to prevent from wake ups but I cannot say I would be sure everything will work smoothly. I have glanced through the cumulative diff and to be honest I am not really sure the result is a great simplification in the end. There is still quite a lot of special casing. It is true that the page allocator path is cleaned up and some CMA specific checks are moved away. This is definitely good to see but I am not convinced that the new zone is really justified. Only very little from the zone infrastructure is used in the end AFAICS. Is there any specific usecase which cannot be solved with the pageblock while it could be with the zone approach? That would be a strong argument to chose one over another. Please do _not_ take this as a NAK from me. At least not at this time. I am still trying to understand all the consequences but my intuition tells me that building on top of highmem like approach will turn out to be problematic in future (as we have already seen with the highmem and movable zones) so this needs a very prudent consideration. -- Michal Hocko SUSE Labs
[toc] | [prev] | [next] | [standalone]
| From | Joonsoo Kim <js1304@gmail.com> |
|---|---|
| Date | 2017-04-25 05:50 +0200 |
| Message-ID | <tzZEe-7Hj-9@gated-at.bofh.it> |
| In reply to | #1629531 |
On Mon, Apr 24, 2017 at 03:09:36PM +0200, Michal Hocko wrote: > On Mon 17-04-17 11:02:12, Joonsoo Kim wrote: > > On Thu, Apr 13, 2017 at 01:56:15PM +0200, Michal Hocko wrote: > > > On Wed 12-04-17 10:35:06, Joonsoo Kim wrote: > [...] > > > > ZONE_CMA is conceptually the same with ZONE_MOVABLE. There is a software > > > > constraint to guarantee the success of future allocation request from > > > > the device. If the device requests the specific range of the memory in CMA > > > > area at the runtime, page that allocated by MM will be migrated to > > > > the other page and it will be returned to the device. To guarantee it, > > > > ZONE_CMA only takes the allocation request with GFP_MOVABLE. > > > > > > The immediate follow up question is. Why cannot we reuse ZONE_MOVABLE > > > for that purpose? > > > > I can make CMA reuses the ZONE_MOVABLE but I don't want it. Reasons > > are that > > > > 1. If ZONE_MOVABLE has two different types of memory, hotpluggable and > > CMA, it may need special handling for each type. This would lead to a new > > migratetype again (to distinguish them) and easy to be error-prone. I > > don't want that case. > > Hmm, I see your motivation. I believe that we could find a way > around this. Anyway, movable zones are quite special and configuring > overlapping CMA and hotplug movable regions could be refused. So I am > not even sure this is a real problem in practice. > > > 2. CMA users want to see usage stat separately since CMA often causes > > the problems and separate stat would helps to debug it. > > That could be solved by a per-zone/node counter. > > Anyway, these reasons should be mentioned as well. Adding a new zone is Okay. > not for free. For most common configurations where we have ZONE_DMA, > ZONE_DMA32, ZONE_NORMAL and ZONE_MOVABLE all the 3 bits are already > consumed so a new zone will need a new one AFAICS. Yes, it requires one more bit for a new zone and it's handled by the patch. > > [...] > > > > Other things are completely the same with other zones. For MM POV, there is > > > > no difference in allocation process except that it only takes > > > > GFP_MOVABLE request. In reclaim, pages that are allocated by MM will > > > > be reclaimed by the same policy of the MM. So, no difference. > > > > > > OK, so essentially this is yet another "highmem" zone. We already know > > > that only GFP_MOVABLE are allowed to fallback to ZONE_CMA but do CMA > > > allocations fallback to other zones and punch new holes? In which zone > > > order? > > > > Hmm... I don't understand your question. Could you elaborate it more? > > Well, my question was about the zone fallback chain. MOVABLE allocation > can fallback to lower zones and also to the ZONE_CMA with your patch. If > there is a CMA allocation it doesn't fall back to any other zone - in > other words no new holes are punched to other zones. Is this correct? Hmm... I still don't get the meaning of "no new holes are punched to other zones". I try to answer with my current understanding about your question. MOVABLE allocation will fallback as following sequence. ZONE_CMA -> ZONE_MOVABLE -> ZONE_HIGHMEM -> ZONE_NORMAL -> ... I don't understand what you mean CMA allocation. In MM's context, there is no CMA allocation. That is just MOVABLE allocation. For device's context, there is CMA allocation. It is range specific allocation so it should be succeed for requested range. No fallback is allowed in this case. > > > > This 'no difference' is a strong point of this approach. ZONE_CMA is > > > > naturally handled by MM subsystem unlike as before (special handling is > > > > required for MIGRATE_CMA). > > > > > > > > 3. Controversial Point > > > > > > > > Major concern from Mel is that zone concept is abused. ZONE is originally > > > > introduced to solve some issues due to H/W addressing limitation. > > > > > > Yes, very much agreed on that. You basically want to punch holes into > > > other zones to guarantee an allocation progress. Marking those wholes > > > with special migrate type sounds quite natural but I will have to study > > > the current code some more to see whether issues you mention are > > > inherently unfixable. This might very well turn out to be the case. > > > > At a glance, special migratetype sound natural. I also did. However, > > it's not natural in implementation POV. Zone consists of the same type > > of memory (by definition ?) and MM subsystem is implemented with that > > assumption. If difference type of memory shares the same zone, it easily > > causes the problem and CMA problems are the such case. > > But this is not any different from the highmem vs. lowmem problems we > already have, no? I have looked at your example in the cover where you > mention utilization and the reclaim problems. With the node reclaim we > will have pages from all zones on the same LRU(s). isolate_lru_pages > will skip those from ZONE_CMA because their zone_idx is higher than > gfp_idx(GFP_KERNEL). The same could be achieved by an explicit check for > the pageblock migrate type. So the zone doesn't really help much. Or is > there some aspect that I am missing? Your understanding is correct. It can archieved by an explict check for migratetype. And, this is the main reason that we should avoid such approach. With ZONE approach, all these things are done naturally. We don't need any explicit check to anywhere. We already have a code to skip to reclaim such pages by checking zone_idx. However, with MIGRATETYPE approach, all these things *cannot* be done naturally. We need extra checks to all the places (allocator fast path, reclaim path, compaction, etc...). It is really error-prone and it already causes many problems due to this aspect. For the performance wise, this approach is also bad since it requires to check migratetype for each pages. Moreover, even if we adds extra checks, things cannot be easily perfect. See 3) Atomic allocation failure problem. It's inherent problem if we have different types of memory in a single zone. We possibly can make things perfect even with MIGRATETYPE approach, however, it requires additional checks in hotpath than current. It's expensive and undesirable. It will make future maintenance of MM code much difficult. This is why I prefer the ZONE approach. > > Another worry I would have with the zone approach is that there is a > risk to reintroduce issues we used to have with small zones in the > past. Just consider that the CMA will get depleted by CMA users almost > completely. Now that zone will not get balanced with only few pages. > wakeup_kswapd/pgdat_balanced already has measures to prevent from wake > ups but I cannot say I would be sure everything will work smoothly. If there is a small zone problem, it should be fixed in any case. There are many workloads that allocates memory almost completely and doesn't return them back to the page allocator. > I have glanced through the cumulative diff and to be honest I am not > really sure the result is a great simplification in the end. There is > still quite a lot of special casing. It is true that the page allocator Special casing is mostly for initialization. We cannot avoid such things since CMA isn't normal memory. We have just two choices. 1) ZONE: special casing in intialization phase 2) MIGRATETYPE: special casing in runtime phase And, I choose 1). > path is cleaned up and some CMA specific checks are moved away. This is > definitely good to see but I am not convinced that the new zone is > really justified. Only very little from the zone infrastructure is used > in the end AFAICS. Is there any specific usecase which cannot be solved > with the pageblock while it could be with the zone approach? That would > be a strong argument to chose one over another. As I mentioned above, atomic allocation failure problem is somewhat inherent problem. Another one is described in Vlastimil's reply in other thread. lkml.kernel.org/r/ae9c5714-ff45-05ea-6a10-976c311b5742@suse.cz > > Please do _not_ take this as a NAK from me. At least not at this time. I > am still trying to understand all the consequences but my intuition > tells me that building on top of highmem like approach will turn out to > be problematic in future (as we have already seen with the highmem and > movable zones) so this needs a very prudent consideration. I can understand that you are prudent to this issue. However, it takes more than two years and many people already expressed that ZONE approach is the way to go. As I said before, some problems are due to S/W limitation of the CMA memory, not due to the implementation. Even if it chooses MIGRATETYPE approach, some problem that you have seen in highmem and movable zones will still exist. So, it should not be a criteria for implementation decision. Thanks.
[toc] | [prev] | [next] | [standalone]
| From | Michal Hocko <mhocko@kernel.org> |
|---|---|
| Date | 2017-04-27 17:10 +0200 |
| Message-ID | <tATdn-2kj-15@gated-at.bofh.it> |
| In reply to | #1630170 |
On Tue 25-04-17 12:42:57, Joonsoo Kim wrote:
> On Mon, Apr 24, 2017 at 03:09:36PM +0200, Michal Hocko wrote:
> > On Mon 17-04-17 11:02:12, Joonsoo Kim wrote:
> > > On Thu, Apr 13, 2017 at 01:56:15PM +0200, Michal Hocko wrote:
> > > > On Wed 12-04-17 10:35:06, Joonsoo Kim wrote:
[...]
> > not for free. For most common configurations where we have ZONE_DMA,
> > ZONE_DMA32, ZONE_NORMAL and ZONE_MOVABLE all the 3 bits are already
> > consumed so a new zone will need a new one AFAICS.
>
> Yes, it requires one more bit for a new zone and it's handled by the patch.
I am pretty sure that you are aware that consuming new page flag bits
is usually a no-go and something we try to avoid as much as possible
because we are in a great shortage there. So there really have to be a
_strong_ reason if we go that way. My current understanding that the
whole zone concept is more about a more convenient implementation rather
than a fundamental change which will solve unsolvable problems with the
current approach. More on that below.
[...]
> MOVABLE allocation will fallback as following sequence.
>
> ZONE_CMA -> ZONE_MOVABLE -> ZONE_HIGHMEM -> ZONE_NORMAL -> ...
>
> I don't understand what you mean CMA allocation. In MM's context,
> there is no CMA allocation. That is just MOVABLE allocation.
>
> For device's context, there is CMA allocation. It is range specific
> allocation so it should be succeed for requested range. No fallback is
> allowed in this case.
OK. that answers my question. I guess... My main confusion comes from
__alloc_gigantic_page which shares alloc_contig_range with the cma
allocation. But from what you wrote above and my quick glance over the
code __alloc_gigantic_page simply changes the migrate type of the pfn
range and it doesn't move it to the zone CMA. Right?
[...]
> > > At a glance, special migratetype sound natural. I also did. However,
> > > it's not natural in implementation POV. Zone consists of the same type
> > > of memory (by definition ?) and MM subsystem is implemented with that
> > > assumption. If difference type of memory shares the same zone, it easily
> > > causes the problem and CMA problems are the such case.
> >
> > But this is not any different from the highmem vs. lowmem problems we
> > already have, no? I have looked at your example in the cover where you
> > mention utilization and the reclaim problems. With the node reclaim we
> > will have pages from all zones on the same LRU(s). isolate_lru_pages
> > will skip those from ZONE_CMA because their zone_idx is higher than
> > gfp_idx(GFP_KERNEL). The same could be achieved by an explicit check for
> > the pageblock migrate type. So the zone doesn't really help much. Or is
> > there some aspect that I am missing?
>
> Your understanding is correct. It can archieved by an explict check
> for migratetype. And, this is the main reason that we should avoid
> such approach.
>
> With ZONE approach, all these things are done naturally. We don't need
> any explicit check to anywhere. We already have a code to skip to
> reclaim such pages by checking zone_idx.
Yes, and as we have to filter pages anyway doing so for cma blocks
doesn't sound overly burdensome from the maintenance point of view.
> However, with MIGRATETYPE approach, all these things *cannot* be done
> naturally. We need extra checks to all the places (allocator fast
> path, reclaim path, compaction, etc...). It is really error-prone and
> it already causes many problems due to this aspect. For the
> performance wise, this approach is also bad since it requires to check
> migratetype for each pages.
>
> Moreover, even if we adds extra checks, things cannot be easily
> perfect.
I see this point and I agree that using a specific zone might be a
_nicer_ solution in the end but you have to consider another aspects as
well. The main one I am worried about is a long term maintainability.
We are really out of page flags and consuming one for a rather specific
usecase is not good. Look at ZONE_DMA. I am pretty sure that almost
no sane HW needs 16MB zone anymore, yet we have hard time to get rid
of it and so we have that memory laying around unused all the time
and blocking one page flag bit. CMA falls into a similar category
AFAIU. I wouldn't be all that surprised if a future HW will not need CMA
allocations in few years, yet we will have to fight to get rid of it
like we do with ZONE_DMA. And not only that. We will also have to fight
finding page flags for other more general usecases in the meantime.
> See 3) Atomic allocation failure problem. It's inherent
> problem if we have different types of memory in a single zone.
> We possibly can make things perfect even with MIGRATETYPE approach,
> however, it requires additional checks in hotpath than current. It's
> expensive and undesirable. It will make future maintenance of MM code
> much difficult.
I believe that the overhead in the hot path is not such a big deal. We
have means to make it 0 when CMA is not used by jumplabels. I assume
that the vast majority of systems will not use CMA. Those systems which
use CMA should be able to cope with some slight overhead IMHO.
I agree that the code maintenance cost is not free. And that is a valid
concern. CMA maintenance will not be for free in either case, though (if
for nothing else the page flags space mentioned above). Let's see what
what this means for mm/page_alloc.c
mm/page_alloc.c | 220 ++++++++++++++++++++++++++++----------------------------
1 file changed, 109 insertions(+), 111 deletions(-)
Not very convincing at first glance but this can be quite misleading as
you have already mentioned because you have moved a lot of code to to
init path. So let's just focus on the allocator hot paths
@@ -800,7 +805,7 @@ static inline void __free_one_page(struct page *page,
VM_BUG_ON(migratetype == -1);
if (likely(!is_migrate_isolate(migratetype)))
- __mod_zone_freepage_state(zone, 1 << order, migratetype);
+ __mod_zone_page_state(zone, NR_FREE_PAGES, 1 << order);
VM_BUG_ON_PAGE(pfn & ((1 << order) - 1), page);
VM_BUG_ON_PAGE(bad_range(zone, page), page);
@@ -1804,25 +1831,11 @@ static int fallbacks[MIGRATE_TYPES][4] = {
[MIGRATE_UNMOVABLE] = { MIGRATE_RECLAIMABLE, MIGRATE_MOVABLE, MIGRATE_TYPES },
[MIGRATE_RECLAIMABLE] = { MIGRATE_UNMOVABLE, MIGRATE_MOVABLE, MIGRATE_TYPES },
[MIGRATE_MOVABLE] = { MIGRATE_RECLAIMABLE, MIGRATE_UNMOVABLE, MIGRATE_TYPES },
-#ifdef CONFIG_CMA
- [MIGRATE_CMA] = { MIGRATE_TYPES }, /* Never used */
-#endif
#ifdef CONFIG_MEMORY_ISOLATION
[MIGRATE_ISOLATE] = { MIGRATE_TYPES }, /* Never used */
#endif
};
-#ifdef CONFIG_CMA
-static struct page *__rmqueue_cma_fallback(struct zone *zone,
- unsigned int order)
-{
- return __rmqueue_smallest(zone, order, MIGRATE_CMA);
-}
-#else
-static inline struct page *__rmqueue_cma_fallback(struct zone *zone,
- unsigned int order) { return NULL; }
-#endif
-
/*
* Move the free pages in a range to the free lists of the requested type.
* Note that start_page and end_pages are not aligned on a pageblock
@@ -2090,8 +2103,7 @@ static void reserve_highatomic_pageblock(struct page *page, struct zone *zone,
/* Yoink! */
mt = get_pageblock_migratetype(page);
- if (!is_migrate_highatomic(mt) && !is_migrate_isolate(mt)
- && !is_migrate_cma(mt)) {
+ if (!is_migrate_highatomic(mt) && !is_migrate_isolate(mt)) {
zone->nr_reserved_highatomic += pageblock_nr_pages;
set_pageblock_migratetype(page, MIGRATE_HIGHATOMIC);
move_freepages_block(zone, page, MIGRATE_HIGHATOMIC, NULL);
@@ -2235,13 +2247,8 @@ static struct page *__rmqueue(struct zone *zone, unsigned int order,
retry:
page = __rmqueue_smallest(zone, order, migratetype);
- if (unlikely(!page)) {
- if (migratetype == MIGRATE_MOVABLE)
- page = __rmqueue_cma_fallback(zone, order);
-
- if (!page && __rmqueue_fallback(zone, order, migratetype))
- goto retry;
- }
+ if (unlikely(!page) && __rmqueue_fallback(zone, order, migratetype))
+ goto retry;
trace_mm_page_alloc_zone_locked(page, order, migratetype);
return page;
@@ -2283,9 +2290,6 @@ static int rmqueue_bulk(struct zone *zone, unsigned int order,
list_add_tail(&page->lru, list);
list = &page->lru;
alloced++;
- if (is_migrate_cma(get_pcppage_migratetype(page)))
- __mod_zone_page_state(zone, NR_FREE_CMA_PAGES,
- -(1 << order));
}
/*
@@ -2636,10 +2640,10 @@ int __isolate_free_page(struct page *page, unsigned int order)
* exists.
*/
watermark = min_wmark_pages(zone) + (1UL << order);
- if (!zone_watermark_ok(zone, 0, watermark, 0, ALLOC_CMA))
+ if (!zone_watermark_ok(zone, 0, watermark, 0, 0))
return 0;
- __mod_zone_freepage_state(zone, -(1UL << order), mt);
+ __mod_zone_page_state(zone, NR_FREE_PAGES, -(1UL << order));
}
/* Remove page from free list */
@@ -2655,8 +2659,8 @@ int __isolate_free_page(struct page *page, unsigned int order)
struct page *endpage = page + (1 << order) - 1;
for (; page < endpage; page += pageblock_nr_pages) {
int mt = get_pageblock_migratetype(page);
- if (!is_migrate_isolate(mt) && !is_migrate_cma(mt)
- && !is_migrate_highatomic(mt))
+ if (!is_migrate_isolate(mt) &&
+ !is_migrate_highatomic(mt))
set_pageblock_migratetype(page,
MIGRATE_MOVABLE);
}
@@ -2783,8 +2787,7 @@ struct page *rmqueue(struct zone *preferred_zone,
spin_unlock(&zone->lock);
if (!page)
goto failed;
- __mod_zone_freepage_state(zone, -(1 << order),
- get_pcppage_migratetype(page));
+ __mod_zone_page_state(zone, NR_FREE_PAGES, -(1 << order));
__count_zid_vm_events(PGALLOC, page_zonenum(page), 1 << order);
zone_statistics(preferred_zone, zone);
@@ -2907,12 +2910,6 @@ bool __zone_watermark_ok(struct zone *z, unsigned int order, unsigned long mark,
else
min -= min / 4;
-#ifdef CONFIG_CMA
- /* If allocation can't use CMA areas don't use free CMA pages */
- if (!(alloc_flags & ALLOC_CMA))
- free_pages -= zone_page_state(z, NR_FREE_CMA_PAGES);
-#endif
-
/*
* Check watermarks for an order-0 allocation request. If these
* are not met, then a high-order request also cannot go ahead
@@ -2940,13 +2937,6 @@ bool __zone_watermark_ok(struct zone *z, unsigned int order, unsigned long mark,
if (!list_empty(&area->free_list[mt]))
return true;
}
-
-#ifdef CONFIG_CMA
- if ((alloc_flags & ALLOC_CMA) &&
- !list_empty(&area->free_list[MIGRATE_CMA])) {
- return true;
- }
-#endif
}
return false;
}
@@ -2962,13 +2952,6 @@ static inline bool zone_watermark_fast(struct zone *z, unsigned int order,
unsigned long mark, int classzone_idx, unsigned int alloc_flags)
{
long free_pages = zone_page_state(z, NR_FREE_PAGES);
- long cma_pages = 0;
-
-#ifdef CONFIG_CMA
- /* If allocation can't use CMA areas don't use free CMA pages */
- if (!(alloc_flags & ALLOC_CMA))
- cma_pages = zone_page_state(z, NR_FREE_CMA_PAGES);
-#endif
/*
* Fast check for order-0 only. If this fails then the reserves
@@ -2977,7 +2960,7 @@ static inline bool zone_watermark_fast(struct zone *z, unsigned int order,
* the caller is !atomic then it'll uselessly search the free
* list. That corner case is then slower but it is harmless.
*/
- if (!order && (free_pages - cma_pages) > mark + z->lowmem_reserve[classzone_idx])
+ if (!order && free_pages > mark + z->lowmem_reserve[classzone_idx])
return true;
return __zone_watermark_ok(z, order, mark, classzone_idx, alloc_flags,
@@ -3547,10 +3530,6 @@ gfp_to_alloc_flags(gfp_t gfp_mask)
} else if (unlikely(rt_task(current)) && !in_interrupt())
alloc_flags |= ALLOC_HARDER;
-#ifdef CONFIG_CMA
- if (gfpflags_to_migratetype(gfp_mask) == MIGRATE_MOVABLE)
- alloc_flags |= ALLOC_CMA;
-#endif
return alloc_flags;
}
@@ -3972,9 +3951,6 @@ static inline bool prepare_alloc_pages(gfp_t gfp_mask, unsigned int order,
if (should_fail_alloc_page(gfp_mask, order))
return false;
- if (IS_ENABLED(CONFIG_CMA) && ac->migratetype == MIGRATE_MOVABLE)
- *alloc_flags |= ALLOC_CMA;
-
return true;
}
This looks like a nice clean up. Those ifdefs are ugly as hell. One
could argue that some of that could be cleaned up by simply adding some
helpers (with a jump label to reduce the overhead), though. But is this
really strong enough reason to bring the whole zone in? I am not really
convinced to be honest.
[...]
> > Please do _not_ take this as a NAK from me. At least not at this time. I
> > am still trying to understand all the consequences but my intuition
> > tells me that building on top of highmem like approach will turn out to
> > be problematic in future (as we have already seen with the highmem and
> > movable zones) so this needs a very prudent consideration.
>
> I can understand that you are prudent to this issue. However, it takes more
> than two years and many people already expressed that ZONE approach is the
> way to go.
I can see a single Acked-by and one Reviewed-by. It would be much more
convincing to see much larger support. Do not take me wrong I am not
trying to undermine the feedback so far but we should be clear about one
thing. CMA is mostly motivated by the industry which tries to overcome
HW limitations which can change in future very easily. I would rather
see good enough solution for something like that than a nicer solution
which is pushing additional burden on more general usecases.
That being said, I would like to see a much larger consensus in the MM
community before a new zone is merged. I am staying very skeptical this
is the right direction though.
--
Michal Hocko
SUSE Labs
[toc] | [prev] | [next] | [standalone]
| From | Igor Stoppa <igor.stoppa@huawei.com> |
|---|---|
| Date | 2017-04-28 10:10 +0200 |
| Subject | Generic approach to customizable zones - was: Re: [PATCH v7 0/7] Introduce ZONE_CMA |
| Message-ID | <tB98u-4Ii-13@gated-at.bofh.it> |
| In reply to | #1632163 |
On 27/04/17 18:06, Michal Hocko wrote:
> On Tue 25-04-17 12:42:57, Joonsoo Kim wrote:
[...]
>> Yes, it requires one more bit for a new zone and it's handled by the patch.
>
> I am pretty sure that you are aware that consuming new page flag bits
> is usually a no-go and something we try to avoid as much as possible
> because we are in a great shortage there. So there really have to be a
> _strong_ reason if we go that way. My current understanding that the
> whole zone concept is more about a more convenient implementation rather
> than a fundamental change which will solve unsolvable problems with the
> current approach. More on that below.
Since I am in a similar situation, I think it's better if I join this
conversation instead of going through the same in a separate thread.
In this regard, I have a few observations (are they correct?):
* not everyone seems to be interested in having all the current
zones active simultaneously
* some zones are even not so meaningful on certain architectures or
platforms
* some architectures/platforms that are 64 bits would have no penalty
in dealing with a larger data type.
So I wonder, would anybody be against this:
* within the 32bits constraint, define some optional zones
* decouple the specific position of a bit from the zone it represents;
iow: if the zone is enabled, ensure that it gets a bit in the mask,
but do not make promises about which one it is, provided that the
corresponding macros work properly
* ensure that if one selects more optional zones than there are bits
available (in the case of a 32bits mask), an error is produced at
compile time
* if one is happy to have a 64bits type, allow for as many zones as
it's possible to fit, or anyway more than what is possible with
the 32 bit mask.
I think I can re-factor the code so that there is no runtime performance
degradation, if there is no immediate objection to what I described. Or
maybe I failed to notice some obvious pitfall?
From what I see, there seems to be a lot of interest in using functions
like Kmalloc / vmalloc, with the ability of specifying pseudo-custom
areas from where they should tap into.
Why not, as long as those who do not need it are not negatively impacted?
I understand that if the association between bits and zones is fixed,
then suddenly bits become very precious stuff, but if they could be used
in a more efficient way, then maybe they could be used more liberally.
The alternative is to keep getting requests about new zones and turning
them away because they do not pass the bar of being extremely critical,
even if indeed they would simplify people's life.
The change shouldn't be too ugly, if I do something along these lines of
the pseudo code below.
Note: the #ifdefs would be mainly concentrated in the declaration part.
enum gfp_zone_shift {
#if IS_ENABLED(CONFIG_ZONE_DMA)
/*I haven't checked if this is the correct name, but it gives the idea*/
ZONE_DMA_SHIFT = 0,
#endif
#if IS_ENABLED(CONFIG_ZONE_HIGHMEM)
ZONE_HIGHMEM_SHIFT,
#endif
#if IS_ENABLED(CONFIG_ZONE_DMA32)
ZONE_DMA32_SHIFT,
#endif
#if IS_ENABLED(CONFIG_ZONE_xxx)
ZONE_xxx,
#endif
NON_OPTIONAL_ZONE_SHIFT,
...
USED_ZONES_NUMBER,
ZONE_MOVABLE_SHIFT = USED_ZONES_NUMBER,
...
};
#if USED_ZONES_NUMBER < MAX_ZONES_32BITS
typedef gfp_zones_t uint32_t
#elif IS_ENABLED(CONFIG_ZONES_64BITS
typedef gfp_zones_t uint64_t
#else
#error
#endif
The type should be adjusted in other places where it is used, but I
didn't find too many occurrences.
#define __ZONE_DMA \
(((gfp_zones_t)IS_ENABLED(CONFIG_ZONE_DMA)) << \
(ZONE_DMA_SHIFT - 0))
[rinse and repeat]
Code referring to these optional zones can be sandboxed in
#if IS_ENABLED(CONFIG_ZONE_DMA)
inline function do_something_dma() {
....
}
#else
#define do_something_dma()
#endif
Or equivalent, effectively removing many #ifdefs from the main code of
functions like those called by kmalloc.
So, would this approach stand a chance?
thanks, igor
[toc] | [prev] | [next] | [standalone]
| From | Michal Hocko <mhocko@kernel.org> |
|---|---|
| Date | 2017-04-28 10:40 +0200 |
| Subject | Re: Generic approach to customizable zones - was: Re: [PATCH v7 0/7] Introduce ZONE_CMA |
| Message-ID | <tB9Bw-4Ua-39@gated-at.bofh.it> |
| In reply to | #1632608 |
I didn't read this thoughly yet because I will be travelling shortly but this point alone just made ask, because it seems there is some misunderstanding On Fri 28-04-17 11:04:27, Igor Stoppa wrote: [...] > * if one is happy to have a 64bits type, allow for as many zones as > it's possible to fit, or anyway more than what is possible with > the 32 bit mask. zones are currently placed in struct page::flags. And that already is 64b size on 64b arches. And we do not really have any room spare there. We encode page flags, zone id, numa_nid/sparse section_nr there. How can you add more without enlarging the struct page itself or using external means to store the same information (page_ext comes to mind)? Even if the later would be possible then note thatpage_zone() is used in many performance sensitive paths and making it perform well with special casing would be far from trivial. -- Michal Hocko SUSE Labs
[toc] | [prev] | [next] | [standalone]
| From | Igor Stoppa <igor.stoppa@huawei.com> |
|---|---|
| Date | 2017-04-28 11:10 +0200 |
| Subject | Re: Generic approach to customizable zones - was: Re: [PATCH v7 0/7] Introduce ZONE_CMA |
| Message-ID | <tBa4y-5m0-11@gated-at.bofh.it> |
| In reply to | #1632634 |
On 28/04/17 11:36, Michal Hocko wrote: > I didn't read this thoughly yet because I will be travelling shortly ok, thanks for bearing with me =) > but > this point alone just made ask, because it seems there is some > misunderstanding It is possible, so far I did some changes, but I have not completed the whole conversion. > On Fri 28-04-17 11:04:27, Igor Stoppa wrote: > [...] >> * if one is happy to have a 64bits type, allow for as many zones as >> it's possible to fit, or anyway more than what is possible with >> the 32 bit mask. > > zones are currently placed in struct page::flags. And that already is > 64b size on 64b arches. Ok, the issues I had so fare were related to the enum for zones being treated as 32b. > And we do not really have any room spare there. > We encode page flags, zone id, numa_nid/sparse section_nr there. How can > you add more without enlarging the struct page itself or using external > means to store the same information (page_ext comes to mind)? Then I'll be conservative and assume I can't, unless I can prove otherwise. There is still the possibility I mentioned of loosely coupling DMA, DMA32 and HIGHMEM with the bits currently reserved for them, right? If my system doesn't use those zones as such, because it doesn't have/need them, those bits are wasted for me. Otoh someone else is probably not interested in what I'm after but needs one or more of those zones. Making the meaning of the bits configurable should still be a viable option. It's not altering their amount, just their purpose on a specific build. > Even if > the later would be possible then note thatpage_zone() is used in many > performance sensitive paths and making it perform well with special > casing would be far from trivial. If the solution I propose is acceptable, I'm willing to bite the bullet and go for implementing the conversion. In my case I really would like to be able to use kmalloc, because it would provide an easy path to convert also other portions of the kernel, besides SE Linux. I suspect I would encounter overall far less resistance if the type of change I propose is limited to: s/GFP_KERNEL/GFP_LOCKABLE/ And if I can guarrantee that GFP_LOCKABLE falls back to GFP_KERNEL when the "lockable" feature is not enabled. -- thanks, igor
[toc] | [prev] | [next] | [standalone]
| From | Joonsoo Kim <js1304@gmail.com> |
|---|---|
| Date | 2017-05-02 06:10 +0200 |
| Message-ID | <tCxip-1nj-15@gated-at.bofh.it> |
| In reply to | #1632163 |
On Thu, Apr 27, 2017 at 05:06:36PM +0200, Michal Hocko wrote:
> On Tue 25-04-17 12:42:57, Joonsoo Kim wrote:
> > On Mon, Apr 24, 2017 at 03:09:36PM +0200, Michal Hocko wrote:
> > > On Mon 17-04-17 11:02:12, Joonsoo Kim wrote:
> > > > On Thu, Apr 13, 2017 at 01:56:15PM +0200, Michal Hocko wrote:
> > > > > On Wed 12-04-17 10:35:06, Joonsoo Kim wrote:
> [...]
> > > not for free. For most common configurations where we have ZONE_DMA,
> > > ZONE_DMA32, ZONE_NORMAL and ZONE_MOVABLE all the 3 bits are already
> > > consumed so a new zone will need a new one AFAICS.
> >
> > Yes, it requires one more bit for a new zone and it's handled by the patch.
>
> I am pretty sure that you are aware that consuming new page flag bits
> is usually a no-go and something we try to avoid as much as possible
> because we are in a great shortage there. So there really have to be a
> _strong_ reason if we go that way. My current understanding that the
> whole zone concept is more about a more convenient implementation rather
> than a fundamental change which will solve unsolvable problems with the
> current approach. More on that below.
If there is a consensus that adding a new zone and one more bit in
page flags bits seems to be unreasonable, I try to find a way to use
ZONE_MOVABLE. As mentioned before, that's not fundamental issue to me.
However, it will have many potential problems as I mentioned so I
*really* don't prefer that way.
>
> [...]
> > MOVABLE allocation will fallback as following sequence.
> >
> > ZONE_CMA -> ZONE_MOVABLE -> ZONE_HIGHMEM -> ZONE_NORMAL -> ...
> >
> > I don't understand what you mean CMA allocation. In MM's context,
> > there is no CMA allocation. That is just MOVABLE allocation.
> >
> > For device's context, there is CMA allocation. It is range specific
> > allocation so it should be succeed for requested range. No fallback is
> > allowed in this case.
>
> OK. that answers my question. I guess... My main confusion comes from
> __alloc_gigantic_page which shares alloc_contig_range with the cma
> allocation. But from what you wrote above and my quick glance over the
> code __alloc_gigantic_page simply changes the migrate type of the pfn
> range and it doesn't move it to the zone CMA. Right?
Yes, it doesn't move it to the zone CMA.
>
> [...]
> > > > At a glance, special migratetype sound natural. I also did. However,
> > > > it's not natural in implementation POV. Zone consists of the same type
> > > > of memory (by definition ?) and MM subsystem is implemented with that
> > > > assumption. If difference type of memory shares the same zone, it easily
> > > > causes the problem and CMA problems are the such case.
> > >
> > > But this is not any different from the highmem vs. lowmem problems we
> > > already have, no? I have looked at your example in the cover where you
> > > mention utilization and the reclaim problems. With the node reclaim we
> > > will have pages from all zones on the same LRU(s). isolate_lru_pages
> > > will skip those from ZONE_CMA because their zone_idx is higher than
> > > gfp_idx(GFP_KERNEL). The same could be achieved by an explicit check for
> > > the pageblock migrate type. So the zone doesn't really help much. Or is
> > > there some aspect that I am missing?
> >
> > Your understanding is correct. It can archieved by an explict check
> > for migratetype. And, this is the main reason that we should avoid
> > such approach.
> >
> > With ZONE approach, all these things are done naturally. We don't need
> > any explicit check to anywhere. We already have a code to skip to
> > reclaim such pages by checking zone_idx.
>
> Yes, and as we have to filter pages anyway doing so for cma blocks
> doesn't sound overly burdensome from the maintenance point of view.
>
> > However, with MIGRATETYPE approach, all these things *cannot* be done
> > naturally. We need extra checks to all the places (allocator fast
> > path, reclaim path, compaction, etc...). It is really error-prone and
> > it already causes many problems due to this aspect. For the
> > performance wise, this approach is also bad since it requires to check
> > migratetype for each pages.
> >
> > Moreover, even if we adds extra checks, things cannot be easily
> > perfect.
>
> I see this point and I agree that using a specific zone might be a
> _nicer_ solution in the end but you have to consider another aspects as
> well. The main one I am worried about is a long term maintainability.
> We are really out of page flags and consuming one for a rather specific
> usecase is not good. Look at ZONE_DMA. I am pretty sure that almost
> no sane HW needs 16MB zone anymore, yet we have hard time to get rid
> of it and so we have that memory laying around unused all the time
> and blocking one page flag bit. CMA falls into a similar category
> AFAIU. I wouldn't be all that surprised if a future HW will not need CMA
> allocations in few years, yet we will have to fight to get rid of it
> like we do with ZONE_DMA. And not only that. We will also have to fight
> finding page flags for other more general usecases in the meantime.
This maintenance problem is inherent. This problem exists even if we
uses MIGRATETYPE approach. We cannot remove many hooks for CMA if a
future HW will not need CMA allocation in few years. The only
difference is that one takes single zone bit only for CMA user and the
other approach takes many hooks that we need to take care about it all
the time.
>
> > See 3) Atomic allocation failure problem. It's inherent
> > problem if we have different types of memory in a single zone.
> > We possibly can make things perfect even with MIGRATETYPE approach,
> > however, it requires additional checks in hotpath than current. It's
> > expensive and undesirable. It will make future maintenance of MM code
> > much difficult.
>
> I believe that the overhead in the hot path is not such a big deal. We
> have means to make it 0 when CMA is not used by jumplabels. I assume
> that the vast majority of systems will not use CMA. Those systems which
> use CMA should be able to cope with some slight overhead IMHO.
Please don't underestimate number of CMA user. Most of android device
uses CMA. So, there would be more devices using CMA than the server
not using CMA. They also have a right to experience the best performance.
>
> I agree that the code maintenance cost is not free. And that is a valid
> concern. CMA maintenance will not be for free in either case, though (if
> for nothing else the page flags space mentioned above). Let's see what
> what this means for mm/page_alloc.c
>
> mm/page_alloc.c | 220 ++++++++++++++++++++++++++++----------------------------
> 1 file changed, 109 insertions(+), 111 deletions(-)
>
> Not very convincing at first glance but this can be quite misleading as
> you have already mentioned because you have moved a lot of code to to
> init path. So let's just focus on the allocator hot paths
>
> @@ -800,7 +805,7 @@ static inline void __free_one_page(struct page *page,
>
> VM_BUG_ON(migratetype == -1);
> if (likely(!is_migrate_isolate(migratetype)))
> - __mod_zone_freepage_state(zone, 1 << order, migratetype);
> + __mod_zone_page_state(zone, NR_FREE_PAGES, 1 << order);
>
> VM_BUG_ON_PAGE(pfn & ((1 << order) - 1), page);
> VM_BUG_ON_PAGE(bad_range(zone, page), page);
> @@ -1804,25 +1831,11 @@ static int fallbacks[MIGRATE_TYPES][4] = {
> [MIGRATE_UNMOVABLE] = { MIGRATE_RECLAIMABLE, MIGRATE_MOVABLE, MIGRATE_TYPES },
> [MIGRATE_RECLAIMABLE] = { MIGRATE_UNMOVABLE, MIGRATE_MOVABLE, MIGRATE_TYPES },
> [MIGRATE_MOVABLE] = { MIGRATE_RECLAIMABLE, MIGRATE_UNMOVABLE, MIGRATE_TYPES },
> -#ifdef CONFIG_CMA
> - [MIGRATE_CMA] = { MIGRATE_TYPES }, /* Never used */
> -#endif
> #ifdef CONFIG_MEMORY_ISOLATION
> [MIGRATE_ISOLATE] = { MIGRATE_TYPES }, /* Never used */
> #endif
> };
>
> -#ifdef CONFIG_CMA
> -static struct page *__rmqueue_cma_fallback(struct zone *zone,
> - unsigned int order)
> -{
> - return __rmqueue_smallest(zone, order, MIGRATE_CMA);
> -}
> -#else
> -static inline struct page *__rmqueue_cma_fallback(struct zone *zone,
> - unsigned int order) { return NULL; }
> -#endif
> -
> /*
> * Move the free pages in a range to the free lists of the requested type.
> * Note that start_page and end_pages are not aligned on a pageblock
> @@ -2090,8 +2103,7 @@ static void reserve_highatomic_pageblock(struct page *page, struct zone *zone,
>
> /* Yoink! */
> mt = get_pageblock_migratetype(page);
> - if (!is_migrate_highatomic(mt) && !is_migrate_isolate(mt)
> - && !is_migrate_cma(mt)) {
> + if (!is_migrate_highatomic(mt) && !is_migrate_isolate(mt)) {
> zone->nr_reserved_highatomic += pageblock_nr_pages;
> set_pageblock_migratetype(page, MIGRATE_HIGHATOMIC);
> move_freepages_block(zone, page, MIGRATE_HIGHATOMIC, NULL);
> @@ -2235,13 +2247,8 @@ static struct page *__rmqueue(struct zone *zone, unsigned int order,
>
> retry:
> page = __rmqueue_smallest(zone, order, migratetype);
> - if (unlikely(!page)) {
> - if (migratetype == MIGRATE_MOVABLE)
> - page = __rmqueue_cma_fallback(zone, order);
> -
> - if (!page && __rmqueue_fallback(zone, order, migratetype))
> - goto retry;
> - }
> + if (unlikely(!page) && __rmqueue_fallback(zone, order, migratetype))
> + goto retry;
>
> trace_mm_page_alloc_zone_locked(page, order, migratetype);
> return page;
> @@ -2283,9 +2290,6 @@ static int rmqueue_bulk(struct zone *zone, unsigned int order,
> list_add_tail(&page->lru, list);
> list = &page->lru;
> alloced++;
> - if (is_migrate_cma(get_pcppage_migratetype(page)))
> - __mod_zone_page_state(zone, NR_FREE_CMA_PAGES,
> - -(1 << order));
> }
>
> /*
> @@ -2636,10 +2640,10 @@ int __isolate_free_page(struct page *page, unsigned int order)
> * exists.
> */
> watermark = min_wmark_pages(zone) + (1UL << order);
> - if (!zone_watermark_ok(zone, 0, watermark, 0, ALLOC_CMA))
> + if (!zone_watermark_ok(zone, 0, watermark, 0, 0))
> return 0;
>
> - __mod_zone_freepage_state(zone, -(1UL << order), mt);
> + __mod_zone_page_state(zone, NR_FREE_PAGES, -(1UL << order));
> }
>
> /* Remove page from free list */
> @@ -2655,8 +2659,8 @@ int __isolate_free_page(struct page *page, unsigned int order)
> struct page *endpage = page + (1 << order) - 1;
> for (; page < endpage; page += pageblock_nr_pages) {
> int mt = get_pageblock_migratetype(page);
> - if (!is_migrate_isolate(mt) && !is_migrate_cma(mt)
> - && !is_migrate_highatomic(mt))
> + if (!is_migrate_isolate(mt) &&
> + !is_migrate_highatomic(mt))
> set_pageblock_migratetype(page,
> MIGRATE_MOVABLE);
> }
> @@ -2783,8 +2787,7 @@ struct page *rmqueue(struct zone *preferred_zone,
> spin_unlock(&zone->lock);
> if (!page)
> goto failed;
> - __mod_zone_freepage_state(zone, -(1 << order),
> - get_pcppage_migratetype(page));
> + __mod_zone_page_state(zone, NR_FREE_PAGES, -(1 << order));
>
> __count_zid_vm_events(PGALLOC, page_zonenum(page), 1 << order);
> zone_statistics(preferred_zone, zone);
> @@ -2907,12 +2910,6 @@ bool __zone_watermark_ok(struct zone *z, unsigned int order, unsigned long mark,
> else
> min -= min / 4;
>
> -#ifdef CONFIG_CMA
> - /* If allocation can't use CMA areas don't use free CMA pages */
> - if (!(alloc_flags & ALLOC_CMA))
> - free_pages -= zone_page_state(z, NR_FREE_CMA_PAGES);
> -#endif
> -
> /*
> * Check watermarks for an order-0 allocation request. If these
> * are not met, then a high-order request also cannot go ahead
> @@ -2940,13 +2937,6 @@ bool __zone_watermark_ok(struct zone *z, unsigned int order, unsigned long mark,
> if (!list_empty(&area->free_list[mt]))
> return true;
> }
> -
> -#ifdef CONFIG_CMA
> - if ((alloc_flags & ALLOC_CMA) &&
> - !list_empty(&area->free_list[MIGRATE_CMA])) {
> - return true;
> - }
> -#endif
> }
> return false;
> }
> @@ -2962,13 +2952,6 @@ static inline bool zone_watermark_fast(struct zone *z, unsigned int order,
> unsigned long mark, int classzone_idx, unsigned int alloc_flags)
> {
> long free_pages = zone_page_state(z, NR_FREE_PAGES);
> - long cma_pages = 0;
> -
> -#ifdef CONFIG_CMA
> - /* If allocation can't use CMA areas don't use free CMA pages */
> - if (!(alloc_flags & ALLOC_CMA))
> - cma_pages = zone_page_state(z, NR_FREE_CMA_PAGES);
> -#endif
>
> /*
> * Fast check for order-0 only. If this fails then the reserves
> @@ -2977,7 +2960,7 @@ static inline bool zone_watermark_fast(struct zone *z, unsigned int order,
> * the caller is !atomic then it'll uselessly search the free
> * list. That corner case is then slower but it is harmless.
> */
> - if (!order && (free_pages - cma_pages) > mark + z->lowmem_reserve[classzone_idx])
> + if (!order && free_pages > mark + z->lowmem_reserve[classzone_idx])
> return true;
>
> return __zone_watermark_ok(z, order, mark, classzone_idx, alloc_flags,
> @@ -3547,10 +3530,6 @@ gfp_to_alloc_flags(gfp_t gfp_mask)
> } else if (unlikely(rt_task(current)) && !in_interrupt())
> alloc_flags |= ALLOC_HARDER;
>
> -#ifdef CONFIG_CMA
> - if (gfpflags_to_migratetype(gfp_mask) == MIGRATE_MOVABLE)
> - alloc_flags |= ALLOC_CMA;
> -#endif
> return alloc_flags;
> }
>
> @@ -3972,9 +3951,6 @@ static inline bool prepare_alloc_pages(gfp_t gfp_mask, unsigned int order,
> if (should_fail_alloc_page(gfp_mask, order))
> return false;
>
> - if (IS_ENABLED(CONFIG_CMA) && ac->migratetype == MIGRATE_MOVABLE)
> - *alloc_flags |= ALLOC_CMA;
> -
> return true;
> }
>
> This looks like a nice clean up. Those ifdefs are ugly as hell. One
> could argue that some of that could be cleaned up by simply adding some
> helpers (with a jump label to reduce the overhead), though. But is this
> really strong enough reason to bring the whole zone in? I am not really
> convinced to be honest.
Please don't underestimate the benefit of this patchset.
I have said that we need *more* hooks to fix all the problems.
And, please think that this code removal is not only code removal but
also concept removal. With this removing, we don't need to consider
ALLOC_CMA for alloc_flags when calling zone_watermark_ok(). There are
many bugs on it and it still remains. We don't need to consider
pageblock migratetype when handling pageblock migratetype. We don't
need to take a great care about calculating the number of CMA
freepages.
>
> [...]
>
> > > Please do _not_ take this as a NAK from me. At least not at this time. I
> > > am still trying to understand all the consequences but my intuition
> > > tells me that building on top of highmem like approach will turn out to
> > > be problematic in future (as we have already seen with the highmem and
> > > movable zones) so this needs a very prudent consideration.
> >
> > I can understand that you are prudent to this issue. However, it takes more
> > than two years and many people already expressed that ZONE approach is the
> > way to go.
>
> I can see a single Acked-by and one Reviewed-by. It would be much more
> convincing to see much larger support. Do not take me wrong I am not
> trying to undermine the feedback so far but we should be clear about one
> thing. CMA is mostly motivated by the industry which tries to overcome
> HW limitations which can change in future very easily. I would rather
> see good enough solution for something like that than a nicer solution
> which is pushing additional burden on more general usecases.
First of all, current MIGRATETYPE approach isn't good enough to me.
They caused too many problems and there are many remanining problems.
It will causes maintenance issue for a long time.
And, although good enough solution can be better than nicer solution
in some cases, it looks like current situation isn't that case.
There is a single reason, saving page flag bit, to support good enough
solution.
I'd like to ask reversly. Is this a enough reason to make CMA user to
suffer from bugs? Is it a enough reason to maintain such dirty and
hard maintainable code, and to hurt other MM devepoler's development
performance? We need a great care on handling pageblock migratetype,
accounting freepage, reclaiming the allocated page. It would affect
many developers.
>
> That being said, I would like to see a much larger consensus in the MM
> community before a new zone is merged. I am staying very skeptical this
> is the right direction though.
As you know, CMA is actively used in industry and they don't review
the patch in public mailing list. Moreover, there is no interest on
CMA by core MM developers. So, I think that 1 acked-by and 1
reviewed-by is a great agreement. I and they are most of developers
who have interest on CMA in MM community.
Anyway, AFAIK, following people are in favor of this patchset.
If I am wrong, please let me know.
Minchan Kim <minchan@kernel.org>
Laura Abbot <lauraa@codeaurora.org>
Bob Liu <liubo95@huawei.com>
As you know (?), Minchan and Laura are also the core developers to CMA
topic.
Thanks.
[toc] | [prev] | [next] | [standalone]
| From | Michal Hocko <mhocko@kernel.org> |
|---|---|
| Date | 2017-05-02 15:40 +0200 |
| Message-ID | <tCGc2-7i0-3@gated-at.bofh.it> |
| In reply to | #1634127 |
On Tue 02-05-17 13:01:32, Joonsoo Kim wrote: > On Thu, Apr 27, 2017 at 05:06:36PM +0200, Michal Hocko wrote: [...] > > I see this point and I agree that using a specific zone might be a > > _nicer_ solution in the end but you have to consider another aspects as > > well. The main one I am worried about is a long term maintainability. > > We are really out of page flags and consuming one for a rather specific > > usecase is not good. Look at ZONE_DMA. I am pretty sure that almost > > no sane HW needs 16MB zone anymore, yet we have hard time to get rid > > of it and so we have that memory laying around unused all the time > > and blocking one page flag bit. CMA falls into a similar category > > AFAIU. I wouldn't be all that surprised if a future HW will not need CMA > > allocations in few years, yet we will have to fight to get rid of it > > like we do with ZONE_DMA. And not only that. We will also have to fight > > finding page flags for other more general usecases in the meantime. > > This maintenance problem is inherent. This problem exists even if we > uses MIGRATETYPE approach. We cannot remove many hooks for CMA if a > future HW will not need CMA allocation in few years. The only > difference is that one takes single zone bit only for CMA user and the > other approach takes many hooks that we need to take care about it all > the time. And I consider this a big difference. Because while hooks are not nice they will affect CMA users (in a sense of bugs/performance etc.). While an additional bit consumed will affect potential future and more generic features. [...] > > I believe that the overhead in the hot path is not such a big deal. We > > have means to make it 0 when CMA is not used by jumplabels. I assume > > that the vast majority of systems will not use CMA. Those systems which > > use CMA should be able to cope with some slight overhead IMHO. > > Please don't underestimate number of CMA user. Most of android device > uses CMA. So, there would be more devices using CMA than the server > not using CMA. They also have a right to experience the best performance. This is not a fair comparison, though. Android development model is much more faster and tend to not care about future maintainability at all. I do not know about any android device that would run on a clean vanilla kernel because vendors simply do not care enough (or have time) to put the code into a proper shape to upstream it. I understand that this model might work quite well for rapidly changing and moving mobile or IoT segment but it is not the greatest fit to motivate the core kernel subsystem development. We are not in the drivers space! [...] > > This looks like a nice clean up. Those ifdefs are ugly as hell. One > > could argue that some of that could be cleaned up by simply adding some > > helpers (with a jump label to reduce the overhead), though. But is this > > really strong enough reason to bring the whole zone in? I am not really > > convinced to be honest. > > Please don't underestimate the benefit of this patchset. > I have said that we need *more* hooks to fix all the problems. > > And, please think that this code removal is not only code removal but > also concept removal. With this removing, we don't need to consider > ALLOC_CMA for alloc_flags when calling zone_watermark_ok(). There are > many bugs on it and it still remains. We don't need to consider > pageblock migratetype when handling pageblock migratetype. We don't > need to take a great care about calculating the number of CMA > freepages. And all this can be isolated to CMA specific hooks with mostly minimum impact to most users. I hear you saying that zone approach is more natural and I would agree if we wouldn't have to care about the number of zones. > > [...] > > > > > > Please do _not_ take this as a NAK from me. At least not at this time. I > > > > am still trying to understand all the consequences but my intuition > > > > tells me that building on top of highmem like approach will turn out to > > > > be problematic in future (as we have already seen with the highmem and > > > > movable zones) so this needs a very prudent consideration. > > > > > > I can understand that you are prudent to this issue. However, it takes more > > > than two years and many people already expressed that ZONE approach is the > > > way to go. > > > > I can see a single Acked-by and one Reviewed-by. It would be much more > > convincing to see much larger support. Do not take me wrong I am not > > trying to undermine the feedback so far but we should be clear about one > > thing. CMA is mostly motivated by the industry which tries to overcome > > HW limitations which can change in future very easily. I would rather > > see good enough solution for something like that than a nicer solution > > which is pushing additional burden on more general usecases. > > First of all, current MIGRATETYPE approach isn't good enough to me. > They caused too many problems and there are many remanining problems. > It will causes maintenance issue for a long time. > > And, although good enough solution can be better than nicer solution > in some cases, it looks like current situation isn't that case. > There is a single reason, saving page flag bit, to support good enough > solution. > > I'd like to ask reversly. Is this a enough reason to make CMA user to > suffer from bugs? No, but I haven't heard any single argument that those bugs are impossible to fix with the current approach. They might be harder to fix but if I can chose between harder for CMA and harder for other more generic HW independent features I will go for the first one. And do not take me wrong, I have nothing against CMA as such. It solves a real life problem. I just believe it doesn't deserve to consume a new bit in page flags because that is just too scarce resource. Thanks! -- Michal Hocko SUSE Labs
[toc] | [prev] | [next] | [standalone]
| From | Joonsoo Kim <js1304@gmail.com> |
|---|---|
| Date | 2017-05-11 04:20 +0200 |
| Message-ID | <tFLRT-iY-3@gated-at.bofh.it> |
| In reply to | #1634413 |
Sorry for the late response. I was on a vacation.
On Tue, May 02, 2017 at 03:32:29PM +0200, Michal Hocko wrote:
> On Tue 02-05-17 13:01:32, Joonsoo Kim wrote:
> > On Thu, Apr 27, 2017 at 05:06:36PM +0200, Michal Hocko wrote:
> [...]
> > > I see this point and I agree that using a specific zone might be a
> > > _nicer_ solution in the end but you have to consider another aspects as
> > > well. The main one I am worried about is a long term maintainability.
> > > We are really out of page flags and consuming one for a rather specific
> > > usecase is not good. Look at ZONE_DMA. I am pretty sure that almost
> > > no sane HW needs 16MB zone anymore, yet we have hard time to get rid
> > > of it and so we have that memory laying around unused all the time
> > > and blocking one page flag bit. CMA falls into a similar category
> > > AFAIU. I wouldn't be all that surprised if a future HW will not need CMA
> > > allocations in few years, yet we will have to fight to get rid of it
> > > like we do with ZONE_DMA. And not only that. We will also have to fight
> > > finding page flags for other more general usecases in the meantime.
> >
> > This maintenance problem is inherent. This problem exists even if we
> > uses MIGRATETYPE approach. We cannot remove many hooks for CMA if a
> > future HW will not need CMA allocation in few years. The only
> > difference is that one takes single zone bit only for CMA user and the
> > other approach takes many hooks that we need to take care about it all
> > the time.
>
> And I consider this a big difference. Because while hooks are not nice
> they will affect CMA users (in a sense of bugs/performance etc.). While
> an additional bit consumed will affect potential future and more generic
> features.
Because these hooks are so tricky and are spread on many places,
bugs about these hooks can be made by *non-CMA* user and they hurt
*CMA* user. These hooks could also delay non-CMA user's development speed
since there are many hooks about CMA and understanding how CMA is managed
is rather difficult. I think that this is a big maintenance overhead
not only for CMA user but also for non-CMA user. So, I think that it
can justify additional bit consumed.
>
> [...]
> > > I believe that the overhead in the hot path is not such a big deal. We
> > > have means to make it 0 when CMA is not used by jumplabels. I assume
> > > that the vast majority of systems will not use CMA. Those systems which
> > > use CMA should be able to cope with some slight overhead IMHO.
> >
> > Please don't underestimate number of CMA user. Most of android device
> > uses CMA. So, there would be more devices using CMA than the server
> > not using CMA. They also have a right to experience the best performance.
>
> This is not a fair comparison, though. Android development model is much
> more faster and tend to not care about future maintainability at all. I
> do not know about any android device that would run on a clean vanilla
> kernel because vendors simply do not care enough (or have time) to put
> the code into a proper shape to upstream it. I understand that this
> model might work quite well for rapidly changing and moving mobile or
> IoT segment but it is not the greatest fit to motivate the core kernel
> subsystem development. We are not in the drivers space!
>
> [...]
> > > This looks like a nice clean up. Those ifdefs are ugly as hell. One
> > > could argue that some of that could be cleaned up by simply adding some
> > > helpers (with a jump label to reduce the overhead), though. But is this
> > > really strong enough reason to bring the whole zone in? I am not really
> > > convinced to be honest.
> >
> > Please don't underestimate the benefit of this patchset.
> > I have said that we need *more* hooks to fix all the problems.
> >
> > And, please think that this code removal is not only code removal but
> > also concept removal. With this removing, we don't need to consider
> > ALLOC_CMA for alloc_flags when calling zone_watermark_ok(). There are
> > many bugs on it and it still remains. We don't need to consider
> > pageblock migratetype when handling pageblock migratetype. We don't
> > need to take a great care about calculating the number of CMA
> > freepages.
>
> And all this can be isolated to CMA specific hooks with mostly minimum
> impact to most users. I hear you saying that zone approach is more natural
> and I would agree if we wouldn't have to care about the number of zones.
I attach a solution about one more bit in page flags although I don't
agree with your opinion that additional bit is no-go approach. Just
note that we have already used three bits for zone encoding in some
configuration due to ZONE_DEVICE.
>
> > > [...]
> > >
> > > > > Please do _not_ take this as a NAK from me. At least not at this time. I
> > > > > am still trying to understand all the consequences but my intuition
> > > > > tells me that building on top of highmem like approach will turn out to
> > > > > be problematic in future (as we have already seen with the highmem and
> > > > > movable zones) so this needs a very prudent consideration.
> > > >
> > > > I can understand that you are prudent to this issue. However, it takes more
> > > > than two years and many people already expressed that ZONE approach is the
> > > > way to go.
> > >
> > > I can see a single Acked-by and one Reviewed-by. It would be much more
> > > convincing to see much larger support. Do not take me wrong I am not
> > > trying to undermine the feedback so far but we should be clear about one
> > > thing. CMA is mostly motivated by the industry which tries to overcome
> > > HW limitations which can change in future very easily. I would rather
> > > see good enough solution for something like that than a nicer solution
> > > which is pushing additional burden on more general usecases.
> >
> > First of all, current MIGRATETYPE approach isn't good enough to me.
> > They caused too many problems and there are many remanining problems.
> > It will causes maintenance issue for a long time.
> >
> > And, although good enough solution can be better than nicer solution
> > in some cases, it looks like current situation isn't that case.
> > There is a single reason, saving page flag bit, to support good enough
> > solution.
> >
> > I'd like to ask reversly. Is this a enough reason to make CMA user to
> > suffer from bugs?
>
> No, but I haven't heard any single argument that those bugs are
> impossible to fix with the current approach. They might be harder to fix
> but if I can chose between harder for CMA and harder for other more
> generic HW independent features I will go for the first one. And do not
> take me wrong, I have nothing against CMA as such. It solves a real life
> problem. I just believe it doesn't deserve to consume a new bit in page
> flags because that is just too scarce resource.
As I mentioned above, I think that maintenance overhead due to CMA
deserves to consume a new bit in page flags. It also provide us
extendability to introduce more zones in the future.
Anyway, this value-judgement is subjective so I guess that we
cannot agree with each other. To solve your concern,
I make following solution. Please let me know your opinion about this.
This patch can be applied on top of my ZONE_CMA series.
Thanks.
------------------->8----------------------
From bd8baee10fd83121b4172375524b97c3296a61e1 Mon Sep 17 00:00:00 2001
From: Joonsoo Kim <iamjoonsoo.kim@lge.com>
Date: Wed, 10 May 2017 15:12:26 +0900
Subject: [RFC PATCH] mm/cma: don't use one more bit on page flags for ZONE_CMA
This is just for showing my idea to solve the page flags problem
due to ZONE_CMA. There is some consensus that ZONE_CMA is a nicer
solution than MIGRATETYPE approach but there is also a worry about
using one more bit on page flags. This patch tries to solve this
worry. This patch is a temporary implementation and I will
optimize it more if everyone agree with this approach.
Adding a new zone (ZONE_CMA) needs one more bit on page flags
in some configuration. This resource is very precious so that
it's better not to consume it as much as possible. Therefore,
this patch implements following tricky magic not to use one more bit.
1. If the number of the zone are five due to the ZONE_CMA, start
encoding magic.
2. ZONE_MOVABLE is written on the pages for ZONE_CMA. Since we don't
use ZONE_CMA (enum value 4) that needs one more bit for encoding,
we can save one bit on page flags.
3. Check ZONE_MOVABLE when retrieving page's zone index.
If the zone index is ZONE_MOVABLE, we use a special handler function
to get the actual zone index.
I found no regression on kernel build test with applying this magic.
Signed-off-by: Joonsoo Kim <iamjoonsoo.kim@lge.com>
---
include/linux/cma.h | 24 ++++++++++++++++++++++++
include/linux/gfp.h | 24 ++++++++++++++++++++++--
include/linux/mm.h | 8 +++++++-
include/linux/page-flags-layout.h | 15 ++++++++++++++-
mm/cma.c | 19 +++++++++++++++++++
mm/page_alloc.c | 10 +++++++++-
6 files changed, 95 insertions(+), 5 deletions(-)
diff --git a/include/linux/cma.h b/include/linux/cma.h
index 2433d5e..d36695c 100644
--- a/include/linux/cma.h
+++ b/include/linux/cma.h
@@ -3,6 +3,7 @@
#include <linux/init.h>
#include <linux/types.h>
+#include <linux/mmzone.h>
/*
* There is always at least global CMA area and a few optional
@@ -34,9 +35,32 @@ extern struct page *cma_alloc(struct cma *cma, size_t count, unsigned int align,
extern bool cma_release(struct cma *cma, const struct page *pages, unsigned int count);
#ifdef CONFIG_CMA
+extern unsigned cma_area_count;
extern unsigned long cma_get_free(void);
+extern bool is_zone_cma_page(const struct page *page);
+
+static inline enum zone_type page_zonenum_special(const struct page *page,
+ enum zone_type zone_type)
+{
+ if (!cma_area_count)
+ return zone_type;
+
+ if (zone_type != ZONE_MOVABLE)
+ return zone_type;
+
+ if (is_zone_cma_page(page))
+ return ZONE_CMA;
+
+ return ZONE_MOVABLE;
+}
+
#else
static inline unsigned long cma_get_free(void) { return 0; }
+static inline enum zone_type page_zonenum_special(const struct page *page,
+ enum zone_type zone_type)
+{
+ return zone_type;
+}
#endif
#endif
diff --git a/include/linux/gfp.h b/include/linux/gfp.h
index 15987cc..bc5e443 100644
--- a/include/linux/gfp.h
+++ b/include/linux/gfp.h
@@ -348,13 +348,33 @@ static inline bool gfpflags_allow_blocking(const gfp_t gfp_flags)
* 0xf => BAD (MOVABLE+DMA32+HIGHMEM+DMA)
*/
-#if defined(CONFIG_ZONE_DEVICE) && (MAX_NR_ZONES-1) <= 4
+#if MAX_NR_ZONES <= 4
+#define GFP_ZONES_SHIFT ZONES_SHIFT
+#elif (MAX_NR_ZONES-1) == 4
+
+/*
+ * ZONE_CMA is encoded as ZONE_MOVABLE and ZONES_SHIFT would be one less
+ * than what GFP_ZONES_SHIFT usually needs.
+ */
+#if defined(CONFIG_ZONE_DEVICE) && defined(CONFIG_CMA)
+#define GFP_ZONES_SHIFT ZONES_SHIFT
+
/* ZONE_DEVICE is not a valid GFP zone specifier */
-#define GFP_ZONES_SHIFT 2
+#elif defined(CONFIG_ZONE_DEVICE)
+#define GFP_ZONES_SHIFT Z
+
+#elif defined(CONFIG_CMA)
+#define GFP_ZONES_SHIFT (ZONES_SHIFT + 1)
+
+#else
+#define GFP_ZONES_SHIFT ZONES_SHIFT
+#endif
+
#else
#define GFP_ZONES_SHIFT ZONES_SHIFT
#endif
+
#if !defined(CONFIG_64BITS) && GFP_ZONES_SHIFT > 2
typedef unsigned long long GFP_ZONE_TABLE_TYPE;
#else
diff --git a/include/linux/mm.h b/include/linux/mm.h
index 693dc6e..50228ef 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -23,6 +23,7 @@
#include <linux/page_ext.h>
#include <linux/err.h>
#include <linux/page_ref.h>
+#include <linux/cma.h>
struct mempolicy;
struct anon_vma;
@@ -780,7 +781,12 @@ int finish_mkwrite_fault(struct vm_fault *vmf);
static inline enum zone_type page_zonenum(const struct page *page)
{
- return (page->flags >> ZONES_PGSHIFT) & ZONES_MASK;
+ enum zone_type zone_type = (page->flags >> ZONES_PGSHIFT) & ZONES_MASK;
+
+ if (ZONE_CMA_IN_PAGE_FLAGS)
+ return zone_type;
+
+ return page_zonenum_special(page, zone_type);
}
#ifdef CONFIG_ZONE_DEVICE
diff --git a/include/linux/page-flags-layout.h b/include/linux/page-flags-layout.h
index 77b078c..8a4ae38 100644
--- a/include/linux/page-flags-layout.h
+++ b/include/linux/page-flags-layout.h
@@ -17,12 +17,25 @@
#define ZONES_SHIFT 1
#elif MAX_NR_ZONES <= 4
#define ZONES_SHIFT 2
+
#elif MAX_NR_ZONES <= 8
-#define ZONES_SHIFT 3
+
+#if defined(CONFIG_CMA) && (MAX_NR_ZONES-1) <= 4
+#define ZONES_SHIFT 2
+#define ZONE_CMA_IN_PAGE_FLAGS 0
#else
+#define ZONES_SHIFT 3
+#endif
+
+#else /* MAX_NR_ZONES <= 8 */
+
#error ZONES_SHIFT -- too many zones configured adjust calculation
#endif
+#ifndef ZONE_CMA_IN_PAGE_FLAGS
+#define ZONE_CMA_IN_PAGE_FLAGS 1
+#endif
+
#ifdef CONFIG_SPARSEMEM
#include <asm/sparsemem.h>
diff --git a/mm/cma.c b/mm/cma.c
index adfda1c..1833973 100644
--- a/mm/cma.c
+++ b/mm/cma.c
@@ -110,6 +110,25 @@ static void cma_clear_bitmap(struct cma *cma, unsigned long pfn,
mutex_unlock(&cma->lock);
}
+bool is_zone_cma_page(const struct page *page)
+{
+ struct cma *cma;
+ int i;
+ unsigned long pfn = page_to_pfn(page);
+
+ for (i = 0; i < cma_area_count; i++) {
+ cma = &cma_areas[i];
+
+ if (pfn < cma->base_pfn)
+ continue;
+
+ if (pfn < cma->base_pfn + cma->count)
+ return true;
+ }
+
+ return false;
+}
+
static int __init cma_activate_area(struct cma *cma)
{
int bitmap_size = BITS_TO_LONGS(cma_bitmap_maxno(cma)) * sizeof(long);
diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index cf29227..9399cc4 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -1596,6 +1596,14 @@ void __init init_cma_reserved_pageblock(struct page *page)
unsigned long pfn = page_to_pfn(page);
struct page *p = page;
int nid = page_to_nid(page);
+ enum zone_type zone_type;
+
+ /*
+ * In some cases, we don't have enough place to encode ZONE_CMA
+ * in page flags. Use ZONE_MOVABLE in this case and do
+ * post-processing when getting the page's zone.
+ */
+ zone_type = ZONE_CMA_IN_PAGE_FLAGS ? ZONE_CMA : ZONE_MOVABLE;
/*
* ZONE_CMA will steal present pages from other zones by changing
@@ -1609,7 +1617,7 @@ void __init init_cma_reserved_pageblock(struct page *page)
set_page_count(p, 0);
/* Steal pages from other zones */
- set_page_links(p, ZONE_CMA, nid, pfn);
+ set_page_links(p, zone_type, nid, pfn);
} while (++p, ++pfn, --i);
adjust_present_page_count(page, pageblock_nr_pages);
--
2.7.4
[toc] | [prev] | [next] | [standalone]
| From | Michal Hocko <mhocko@kernel.org> |
|---|---|
| Date | 2017-05-11 11:20 +0200 |
| Message-ID | <tFSql-4t8-5@gated-at.bofh.it> |
| In reply to | #1639153 |
On Thu 11-05-17 11:12:43, Joonsoo Kim wrote: > Sorry for the late response. I was on a vacation. > > On Tue, May 02, 2017 at 03:32:29PM +0200, Michal Hocko wrote: > > On Tue 02-05-17 13:01:32, Joonsoo Kim wrote: > > > On Thu, Apr 27, 2017 at 05:06:36PM +0200, Michal Hocko wrote: > > [...] > > > > I see this point and I agree that using a specific zone might be a > > > > _nicer_ solution in the end but you have to consider another aspects as > > > > well. The main one I am worried about is a long term maintainability. > > > > We are really out of page flags and consuming one for a rather specific > > > > usecase is not good. Look at ZONE_DMA. I am pretty sure that almost > > > > no sane HW needs 16MB zone anymore, yet we have hard time to get rid > > > > of it and so we have that memory laying around unused all the time > > > > and blocking one page flag bit. CMA falls into a similar category > > > > AFAIU. I wouldn't be all that surprised if a future HW will not need CMA > > > > allocations in few years, yet we will have to fight to get rid of it > > > > like we do with ZONE_DMA. And not only that. We will also have to fight > > > > finding page flags for other more general usecases in the meantime. > > > > > > This maintenance problem is inherent. This problem exists even if we > > > uses MIGRATETYPE approach. We cannot remove many hooks for CMA if a > > > future HW will not need CMA allocation in few years. The only > > > difference is that one takes single zone bit only for CMA user and the > > > other approach takes many hooks that we need to take care about it all > > > the time. > > > > And I consider this a big difference. Because while hooks are not nice > > they will affect CMA users (in a sense of bugs/performance etc.). While > > an additional bit consumed will affect potential future and more generic > > features. > > Because these hooks are so tricky and are spread on many places, > bugs about these hooks can be made by *non-CMA* user and they hurt > *CMA* user. These hooks could also delay non-CMA user's development speed > since there are many hooks about CMA and understanding how CMA is managed > is rather difficult. Than make those hooks easier to maintain. Seriously this is a non-argument. [...] > > And all this can be isolated to CMA specific hooks with mostly minimum > > impact to most users. I hear you saying that zone approach is more natural > > and I would agree if we wouldn't have to care about the number of zones. > > I attach a solution about one more bit in page flags although I don't > agree with your opinion that additional bit is no-go approach. Just > note that we have already used three bits for zone encoding in some > configuration due to ZONE_DEVICE. I am absolutely not happy about ZONE_DEVICE but there is _no_ other viable solution right now. I know that people behind this are still considering struct page over direct pfn usage but they are not in the same situation as CMA which _can_ work without additional zone. If you _really_ insist on using zone for CMA then reuse ZONE_MOVABLE. I absolutely miss why do you push a specialized zone so hard. [...] > > No, but I haven't heard any single argument that those bugs are > > impossible to fix with the current approach. They might be harder to fix > > but if I can chose between harder for CMA and harder for other more > > generic HW independent features I will go for the first one. And do not > > take me wrong, I have nothing against CMA as such. It solves a real life > > problem. I just believe it doesn't deserve to consume a new bit in page > > flags because that is just too scarce resource. > > As I mentioned above, I think that maintenance overhead due to CMA > deserves to consume a new bit in page flags. It also provide us > extendability to introduce more zones in the future. > > Anyway, this value-judgement is subjective so I guess that we > cannot agree with each other. To solve your concern, > I make following solution. Please let me know your opinion about this. > This patch can be applied on top of my ZONE_CMA series. I don not think this makes situation any easier or more acceptable for merging. But I feel we are looping without much progress. So let me NAK this until it is _proven_ that the current code is unfixable nor ZONE_MOVABLE can be reused -- Michal Hocko SUSE Labs
[toc] | [prev] | [next] | [standalone]
| From | Vlastimil Babka <vbabka@suse.cz> |
|---|---|
| Date | 2017-05-02 10:10 +0200 |
| Message-ID | <tCB2F-3YK-13@gated-at.bofh.it> |
| In reply to | #1632163 |
On 04/27/2017 05:06 PM, Michal Hocko wrote: > On Tue 25-04-17 12:42:57, Joonsoo Kim wrote: >> On Mon, Apr 24, 2017 at 03:09:36PM +0200, Michal Hocko wrote: >>> On Mon 17-04-17 11:02:12, Joonsoo Kim wrote: >>>> On Thu, Apr 13, 2017 at 01:56:15PM +0200, Michal Hocko wrote: >>>>> On Wed 12-04-17 10:35:06, Joonsoo Kim wrote: > [...] >>> not for free. For most common configurations where we have ZONE_DMA, >>> ZONE_DMA32, ZONE_NORMAL and ZONE_MOVABLE all the 3 bits are already >>> consumed so a new zone will need a new one AFAICS. >> >> Yes, it requires one more bit for a new zone and it's handled by the patch. > > I am pretty sure that you are aware that consuming new page flag bits > is usually a no-go and something we try to avoid as much as possible > because we are in a great shortage there. So there really have to be a > _strong_ reason if we go that way. My current understanding that the > whole zone concept is more about a more convenient implementation rather > than a fundamental change which will solve unsolvable problems with the > current approach. More on that below. I don't see it as such a big issue. It's behind a CONFIG option (so we also don't need the jump labels you suggest later) and enabling it reduces the number of possible NUMA nodes (not page flags). So either you are building a kernel for android phone that needs CMA but will have a single NUMA node, or for a large server with many nodes that won't have CMA. As long as there won't be large servers that need CMA, we should be fine (yes, I know some HW vendors can be very creative, but then it's their problem?). > [...] >> MOVABLE allocation will fallback as following sequence. >> >> ZONE_CMA -> ZONE_MOVABLE -> ZONE_HIGHMEM -> ZONE_NORMAL -> ... Hmm, so this in effect resembles some of the aggressive CMA utilization efforts that were never merged due to issues. Joonsoo, could you summarize/expand the cover letter part on what were the issues with aggressive CMA utilization, and why they no longer apply with ZONE_CMA, especially given the current node-lru reclaim? Thanks.
[toc] | [prev] | [next] | [standalone]
Page 1 of 2 [1] 2 Next page →
Back to top | Article view | linux.kernel
csiph-web