| Message ID | a2a48baca69f103aa431fd201f8a06e3b95e203d.1712648441.git.baolin.wang@linux.alibaba.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | mm: page_alloc: consolidate free page accounting fix 3 | expand |
Hi Baolin, kernel test robot noticed the following build errors: url: https://github.com/intel-lab-lkp/linux/commits/UPDATE-20240409-154935/Johannes-Weiner/mm-page_alloc-remove-pcppage-migratetype-caching/20240321-020814 base: the 10th patch of https://lore.kernel.org/r/20240320180429.678181-11-hannes%40cmpxchg.org patch link: https://lore.kernel.org/r/a2a48baca69f103aa431fd201f8a06e3b95e203d.1712648441.git.baolin.wang%40linux.alibaba.com patch subject: [PATCH] mm: page_alloc: consolidate free page accounting fix 3 config: x86_64-kexec (https://download.01.org/0day-ci/archive/20240410/202404100519.mVXXF6kV-lkp@intel.com/config) compiler: clang version 17.0.6 (https://github.com/llvm/llvm-project 6009708b4367171ccdbf4b5905cb6a803753fe18) reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240410/202404100519.mVXXF6kV-lkp@intel.com/reproduce) If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags | Reported-by: kernel test robot <lkp@intel.com> | Closes: https://lore.kernel.org/oe-kbuild-all/202404100519.mVXXF6kV-lkp@intel.com/ All errors (new ones prefixed by >>): >> mm/page_alloc.c:808:54: error: too few arguments to function call, expected 4, have 3 808 | account_freepages(zone, -(1 << order), migratetype); | ~~~~~~~~~~~~~~~~~ ^ mm/page_alloc.c:645:20: note: 'account_freepages' declared here 645 | static inline void account_freepages(struct page *page, struct zone *zone, | ^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ 646 | int nr_pages, int migratetype) | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ 1 error generated. vim +808 mm/page_alloc.c 759 760 /* 761 * Freeing function for a buddy system allocator. 762 * 763 * The concept of a buddy system is to maintain direct-mapped table 764 * (containing bit values) for memory blocks of various "orders". 765 * The bottom level table contains the map for the smallest allocatable 766 * units of memory (here, pages), and each level above it describes 767 * pairs of units from the levels below, hence, "buddies". 768 * At a high level, all that happens here is marking the table entry 769 * at the bottom level available, and propagating the changes upward 770 * as necessary, plus some accounting needed to play nicely with other 771 * parts of the VM system. 772 * At each level, we keep a list of pages, which are heads of continuous 773 * free pages of length of (1 << order) and marked with PageBuddy. 774 * Page's order is recorded in page_private(page) field. 775 * So when we are allocating or freeing one, we can derive the state of the 776 * other. That is, if we allocate a small block, and both were 777 * free, the remainder of the region must be split into blocks. 778 * If a block is freed, and its buddy is also free, then this 779 * triggers coalescing into a block of larger size. 780 * 781 * -- nyc 782 */ 783 784 static inline void __free_one_page(struct page *page, 785 unsigned long pfn, 786 struct zone *zone, unsigned int order, 787 int migratetype, fpi_t fpi_flags) 788 { 789 struct capture_control *capc = task_capc(zone); 790 unsigned long buddy_pfn = 0; 791 unsigned long combined_pfn; 792 struct page *buddy; 793 bool to_tail; 794 795 VM_BUG_ON(!zone_is_initialized(zone)); 796 VM_BUG_ON_PAGE(page->flags & PAGE_FLAGS_CHECK_AT_PREP, page); 797 798 VM_BUG_ON(migratetype == -1); 799 VM_BUG_ON_PAGE(pfn & ((1 << order) - 1), page); 800 VM_BUG_ON_PAGE(bad_range(zone, page), page); 801 802 account_freepages(page, zone, 1 << order, migratetype); 803 804 while (order < MAX_PAGE_ORDER) { 805 int buddy_mt = migratetype; 806 807 if (compaction_capture(capc, page, order, migratetype)) { > 808 account_freepages(zone, -(1 << order), migratetype); 809 return; 810 } 811 812 buddy = find_buddy_page_pfn(page, pfn, order, &buddy_pfn); 813 if (!buddy) 814 goto done_merging; 815 816 if (unlikely(order >= pageblock_order)) { 817 /* 818 * We want to prevent merge between freepages on pageblock 819 * without fallbacks and normal pageblock. Without this, 820 * pageblock isolation could cause incorrect freepage or CMA 821 * accounting or HIGHATOMIC accounting. 822 */ 823 buddy_mt = get_pfnblock_migratetype(buddy, buddy_pfn); 824 825 if (migratetype != buddy_mt && 826 (!migratetype_is_mergeable(migratetype) || 827 !migratetype_is_mergeable(buddy_mt))) 828 goto done_merging; 829 } 830 831 /* 832 * Our buddy is free or it is CONFIG_DEBUG_PAGEALLOC guard page, 833 * merge with it and move up one order. 834 */ 835 if (page_is_guard(buddy)) 836 clear_page_guard(zone, buddy, order); 837 else 838 __del_page_from_free_list(buddy, zone, order, buddy_mt); 839 840 if (unlikely(buddy_mt != migratetype)) { 841 /* 842 * Match buddy type. This ensures that an 843 * expand() down the line puts the sub-blocks 844 * on the right freelists. 845 */ 846 set_pageblock_migratetype(buddy, migratetype); 847 } 848 849 combined_pfn = buddy_pfn & pfn; 850 page = page + (combined_pfn - pfn); 851 pfn = combined_pfn; 852 order++; 853 } 854 855 done_merging: 856 set_buddy_order(page, order); 857 858 if (fpi_flags & FPI_TO_TAIL) 859 to_tail = true; 860 else if (is_shuffle_order(order)) 861 to_tail = shuffle_pick_tail(); 862 else 863 to_tail = buddy_merge_likely(pfn, buddy_pfn, page, order); 864 865 __add_to_free_list(page, zone, order, migratetype, to_tail); 866 867 /* Notify page reporting subsystem of freed page */ 868 if (!(fpi_flags & FPI_SKIP_REPORT_NOTIFY)) 869 page_reporting_notify_free(order); 870 } 871
Hi Baolin, kernel test robot noticed the following build errors: url: https://github.com/intel-lab-lkp/linux/commits/UPDATE-20240409-154935/Johannes-Weiner/mm-page_alloc-remove-pcppage-migratetype-caching/20240321-020814 base: the 10th patch of https://lore.kernel.org/r/20240320180429.678181-11-hannes%40cmpxchg.org patch link: https://lore.kernel.org/r/a2a48baca69f103aa431fd201f8a06e3b95e203d.1712648441.git.baolin.wang%40linux.alibaba.com patch subject: [PATCH] mm: page_alloc: consolidate free page accounting fix 3 config: x86_64-rhel-8.3 (https://download.01.org/0day-ci/archive/20240410/202404100551.aq0YQFuT-lkp@intel.com/config) compiler: gcc-13 (Ubuntu 13.2.0-4ubuntu3) 13.2.0 reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240410/202404100551.aq0YQFuT-lkp@intel.com/reproduce) If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags | Reported-by: kernel test robot <lkp@intel.com> | Closes: https://lore.kernel.org/oe-kbuild-all/202404100551.aq0YQFuT-lkp@intel.com/ All error/warnings (new ones prefixed by >>): mm/page_alloc.c: In function '__free_one_page': >> mm/page_alloc.c:808:43: error: passing argument 1 of 'account_freepages' from incompatible pointer type [-Werror=incompatible-pointer-types] 808 | account_freepages(zone, -(1 << order), migratetype); | ^~~~ | | | struct zone * mm/page_alloc.c:645:51: note: expected 'struct page *' but argument is of type 'struct zone *' 645 | static inline void account_freepages(struct page *page, struct zone *zone, | ~~~~~~~~~~~~~^~~~ >> mm/page_alloc.c:808:49: warning: passing argument 2 of 'account_freepages' makes pointer from integer without a cast [-Wint-conversion] 808 | account_freepages(zone, -(1 << order), migratetype); | ^~~~~~~~~~~~~ | | | int mm/page_alloc.c:645:70: note: expected 'struct zone *' but argument is of type 'int' 645 | static inline void account_freepages(struct page *page, struct zone *zone, | ~~~~~~~~~~~~~^~~~ >> mm/page_alloc.c:808:25: error: too few arguments to function 'account_freepages' 808 | account_freepages(zone, -(1 << order), migratetype); | ^~~~~~~~~~~~~~~~~ mm/page_alloc.c:645:20: note: declared here 645 | static inline void account_freepages(struct page *page, struct zone *zone, | ^~~~~~~~~~~~~~~~~ cc1: some warnings being treated as errors vim +/account_freepages +808 mm/page_alloc.c 759 760 /* 761 * Freeing function for a buddy system allocator. 762 * 763 * The concept of a buddy system is to maintain direct-mapped table 764 * (containing bit values) for memory blocks of various "orders". 765 * The bottom level table contains the map for the smallest allocatable 766 * units of memory (here, pages), and each level above it describes 767 * pairs of units from the levels below, hence, "buddies". 768 * At a high level, all that happens here is marking the table entry 769 * at the bottom level available, and propagating the changes upward 770 * as necessary, plus some accounting needed to play nicely with other 771 * parts of the VM system. 772 * At each level, we keep a list of pages, which are heads of continuous 773 * free pages of length of (1 << order) and marked with PageBuddy. 774 * Page's order is recorded in page_private(page) field. 775 * So when we are allocating or freeing one, we can derive the state of the 776 * other. That is, if we allocate a small block, and both were 777 * free, the remainder of the region must be split into blocks. 778 * If a block is freed, and its buddy is also free, then this 779 * triggers coalescing into a block of larger size. 780 * 781 * -- nyc 782 */ 783 784 static inline void __free_one_page(struct page *page, 785 unsigned long pfn, 786 struct zone *zone, unsigned int order, 787 int migratetype, fpi_t fpi_flags) 788 { 789 struct capture_control *capc = task_capc(zone); 790 unsigned long buddy_pfn = 0; 791 unsigned long combined_pfn; 792 struct page *buddy; 793 bool to_tail; 794 795 VM_BUG_ON(!zone_is_initialized(zone)); 796 VM_BUG_ON_PAGE(page->flags & PAGE_FLAGS_CHECK_AT_PREP, page); 797 798 VM_BUG_ON(migratetype == -1); 799 VM_BUG_ON_PAGE(pfn & ((1 << order) - 1), page); 800 VM_BUG_ON_PAGE(bad_range(zone, page), page); 801 802 account_freepages(page, zone, 1 << order, migratetype); 803 804 while (order < MAX_PAGE_ORDER) { 805 int buddy_mt = migratetype; 806 807 if (compaction_capture(capc, page, order, migratetype)) { > 808 account_freepages(zone, -(1 << order), migratetype); 809 return; 810 } 811 812 buddy = find_buddy_page_pfn(page, pfn, order, &buddy_pfn); 813 if (!buddy) 814 goto done_merging; 815 816 if (unlikely(order >= pageblock_order)) { 817 /* 818 * We want to prevent merge between freepages on pageblock 819 * without fallbacks and normal pageblock. Without this, 820 * pageblock isolation could cause incorrect freepage or CMA 821 * accounting or HIGHATOMIC accounting. 822 */ 823 buddy_mt = get_pfnblock_migratetype(buddy, buddy_pfn); 824 825 if (migratetype != buddy_mt && 826 (!migratetype_is_mergeable(migratetype) || 827 !migratetype_is_mergeable(buddy_mt))) 828 goto done_merging; 829 } 830 831 /* 832 * Our buddy is free or it is CONFIG_DEBUG_PAGEALLOC guard page, 833 * merge with it and move up one order. 834 */ 835 if (page_is_guard(buddy)) 836 clear_page_guard(zone, buddy, order); 837 else 838 __del_page_from_free_list(buddy, zone, order, buddy_mt); 839 840 if (unlikely(buddy_mt != migratetype)) { 841 /* 842 * Match buddy type. This ensures that an 843 * expand() down the line puts the sub-blocks 844 * on the right freelists. 845 */ 846 set_pageblock_migratetype(buddy, migratetype); 847 } 848 849 combined_pfn = buddy_pfn & pfn; 850 page = page + (combined_pfn - pfn); 851 pfn = combined_pfn; 852 order++; 853 } 854 855 done_merging: 856 set_buddy_order(page, order); 857 858 if (fpi_flags & FPI_TO_TAIL) 859 to_tail = true; 860 else if (is_shuffle_order(order)) 861 to_tail = shuffle_pick_tail(); 862 else 863 to_tail = buddy_merge_likely(pfn, buddy_pfn, page, order); 864 865 __add_to_free_list(page, zone, order, migratetype, to_tail); 866 867 /* Notify page reporting subsystem of freed page */ 868 if (!(fpi_flags & FPI_SKIP_REPORT_NOTIFY)) 869 page_reporting_notify_free(order); 870 } 871
On Wed, Apr 10, 2024 at 05:15:01AM +0800, kernel test robot wrote: > Hi Baolin, > > kernel test robot noticed the following build errors: > > > > url: https://github.com/intel-lab-lkp/linux/commits/UPDATE-20240409-154935/Johannes-Weiner/mm-page_alloc-remove-pcppage-migratetype-caching/20240321-020814 > base: the 10th patch of https://lore.kernel.org/r/20240320180429.678181-11-hannes%40cmpxchg.org > patch link: https://lore.kernel.org/r/a2a48baca69f103aa431fd201f8a06e3b95e203d.1712648441.git.baolin.wang%40linux.alibaba.com > patch subject: [PATCH] mm: page_alloc: consolidate free page accounting fix 3 > config: x86_64-kexec (https://download.01.org/0day-ci/archive/20240410/202404100519.mVXXF6kV-lkp@intel.com/config) > compiler: clang version 17.0.6 (https://github.com/llvm/llvm-project 6009708b4367171ccdbf4b5905cb6a803753fe18) > reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240410/202404100519.mVXXF6kV-lkp@intel.com/reproduce) > > If you fix the issue in a separate patch/commit (i.e. not just a new version of > the same patch/commit), kindly add following tags > | Reported-by: kernel test robot <lkp@intel.com> > | Closes: https://lore.kernel.org/oe-kbuild-all/202404100519.mVXXF6kV-lkp@intel.com/ > > All errors (new ones prefixed by >>): > > >> mm/page_alloc.c:808:54: error: too few arguments to function call, expected 4, have 3 > 808 | account_freepages(zone, -(1 << order), migratetype); > | ~~~~~~~~~~~~~~~~~ ^ > mm/page_alloc.c:645:20: note: 'account_freepages' declared here > 645 | static inline void account_freepages(struct page *page, struct zone *zone, > | ^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > 646 | int nr_pages, int migratetype) > | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > 1 error generated. Looks like a false alarm because the test bot didn't pick up the fixlet to remove the page parameter from account_freepages(): https://lore.kernel.org/all/20240327185831.GB7597@cmpxchg.org/ It's right in Andrew's tree.
diff --git a/mm/page_alloc.c b/mm/page_alloc.c index 8330c5c2de6b..2facf844ef84 100644 --- a/mm/page_alloc.c +++ b/mm/page_alloc.c @@ -805,8 +805,10 @@ static inline void __free_one_page(struct page *page, while (order < MAX_PAGE_ORDER) { int buddy_mt = migratetype; - if (compaction_capture(capc, page, order, migratetype)) + if (compaction_capture(capc, page, order, migratetype)) { + account_freepages(zone, -(1 << order), migratetype); return; + } buddy = find_buddy_page_pfn(page, pfn, order, &buddy_pfn); if (!buddy)
If the released page is captured by compaction, now the free page accounting is not correspondingly decreased, which can make the watermark checks false positive and result in depleted reserves etc. And I can see the false positive watermark checks in thpcompact benchmark, that led to a slight regression: thpcompact Percentage Faults Huge k6.9-rc2-base base + patch10 + 2 fixes Percentage huge-1 78.18 ( 0.00%) 71.92 ( -8.01%) Percentage huge-3 86.70 ( 0.00%) 86.07 ( -0.73%) Percentage huge-5 90.26 ( 0.00%) 78.02 ( -13.57%) Percentage huge-7 92.34 ( 0.00%) 78.67 ( -14.81%) Percentage huge-12 91.18 ( 0.00%) 81.04 ( -11.12%) Percentage huge-18 89.00 ( 0.00%) 79.57 ( -10.60%) Percentage huge-24 90.52 ( 0.00%) 80.07 ( -11.54%) Percentage huge-30 94.44 ( 0.00%) 96.28 ( 1.95%) Percentage huge-32 93.09 ( 0.00%) 99.39 ( 6.77%) After the fix, the regression is gone. Signed-off-by: Baolin Wang <baolin.wang@linux.alibaba.com> Acked-by: Johannes Weiner <hannes@cmpxchg.org> Acked-by: Vlastimil Babka <vbabka@suse.cz> --- mm/page_alloc.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)