diff mbox series

[08/10] vmscan: Add check_move_unevictable_folios()

Message ID 20220605193854.2371230-9-willy@infradead.org
State Not Applicable
Headers show
Series Convert to filemap_get_folios() | expand

Commit Message

Matthew Wilcox June 5, 2022, 7:38 p.m. UTC 
Change the guts of check_move_unevictable_pages() over to use folios
and add check_move_unevictable_pages() as a wrapper.

Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
---
 include/linux/swap.h |  3 ++-
 mm/vmscan.c          | 55 ++++++++++++++++++++++++++------------------
 2 files changed, 35 insertions(+), 23 deletions(-)

Comments

Christoph Hellwig June 8, 2022, 8:07 a.m. UTC | #1 
On Sun, Jun 05, 2022 at 08:38:52PM +0100, Matthew Wilcox (Oracle) wrote:
> Change the guts of check_move_unevictable_pages() over to use folios
> and add check_move_unevictable_pages() as a wrapper.

The changes here look fine, but please also add patches for converting
the two callers (which looks mostly trivial to me).
kernel test robot June 8, 2022, 3:33 p.m. UTC | #2 
Greeting,

FYI, we noticed the following commit (built with gcc-11):

commit: bc9eb0d5ef0a437e399f0fe3b7430b9da5ef9f95 ("[PATCH 08/10] vmscan: Add check_move_unevictable_folios()")
url: https://github.com/intel-lab-lkp/linux/commits/Matthew-Wilcox-Oracle/Convert-to-filemap_get_folios/20220606-034220
base: https://git.kernel.org/cgit/linux/kernel/git/tytso/ext4.git dev
patch link: https://lore.kernel.org/linux-fsdevel/20220605193854.2371230-9-willy@infradead.org

in testcase: boot

on test machine: qemu-system-x86_64 -enable-kvm -cpu SandyBridge -smp 2 -m 16G

caused below changes (please refer to attached dmesg/kmsg for entire log/backtrace):


+---------------------------------------------------------------+------------+------------+
|                                                               | 9f1a7a0465 | bc9eb0d5ef |
+---------------------------------------------------------------+------------+------------+
| boot_successes                                                | 24         | 0          |
| boot_failures                                                 | 0          | 26         |
| BUG:KASAN:stack-out-of-bounds_in_check_move_unevictable_pages | 0          | 26         |
| canonical_address#:#[##]                                      | 0          | 26         |
| RIP:check_move_unevictable_folios                             | 0          | 26         |
| Kernel_panic-not_syncing:Fatal_exception                      | 0          | 26         |
+---------------------------------------------------------------+------------+------------+


If you fix the issue, kindly add following tag
Reported-by: kernel test robot <oliver.sang@intel.com>


[   40.053150][    T1] ==================================================================
[   40.054793][    T1] BUG: KASAN: stack-out-of-bounds in check_move_unevictable_pages+0x34c/0x40f
[   40.056610][    T1] Write of size 8 at addr ffff88810083fd88 by task swapper/0/1
[   40.056720][    T1]
[   40.056720][    T1] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 5.18.0-rc5-00033-gbc9eb0d5ef0a #1
[   40.056720][    T1] Call Trace:
[   40.056720][    T1]  <TASK>
[   40.056720][    T1]  dump_stack_lvl+0x45/0x59
[   40.056720][    T1]  ? check_move_unevictable_pages+0x34c/0x40f
[   40.056720][    T1]  print_address_description.cold+0xcc/0x404
[   40.056720][    T1]  ? check_move_unevictable_pages+0x34c/0x40f
[   40.056720][    T1]  print_report.cold+0x36/0x20b
[   40.056720][    T1]  kasan_report+0xbe/0x199
[   40.056720][    T1]  ? check_move_unevictable_pages+0x34c/0x40f
[   40.056720][    T1]  check_move_unevictable_pages+0x34c/0x40f
[   40.056720][    T1]  ? check_move_unevictable_folios+0xba6/0xba6
[   40.056720][    T1]  ? lock_is_held_type+0x9c/0x111
[   40.056720][    T1]  ? lru_lazyfree_fn+0x245/0x245
[   40.056720][    T1]  ? lock_release+0xdf/0x1fb
[   40.056720][    T1]  ? mlock_page_drain_local+0x1a9/0x31b
[   40.056720][    T1]  ? lock_is_held_type+0x96/0x111
[   40.056720][    T1]  drm_gem_put_pages+0x206/0x363
[   40.056720][    T1]  ? drm_gem_vm_open+0x75/0x75
[   40.056720][    T1]  ? slab_free_freelist_hook+0xba/0x167
[   40.056720][    T1]  ? drm_gem_shmem_vunmap+0x121/0x1a2
[   40.056720][    T1]  drm_gem_shmem_put_pages_locked+0xf8/0x1f2
[   40.056720][    T1]  drm_gem_shmem_vunmap+0x13a/0x1a2
[   40.056720][    T1]  drm_gem_vunmap+0xcb/0x1d0
[   40.056720][    T1]  drm_fbdev_cleanup+0x251/0x33d
[   40.056720][    T1]  drm_fbdev_client_hotplug+0x3fa/0x50e
[   40.056720][    T1]  drm_fbdev_generic_setup+0x16b/0x3f0
[   40.056720][    T1]  vkms_create+0x401/0x470
[   40.056720][    T1]  ? drm_sched_pick_best.cold+0x32/0x32
[   40.056720][    T1]  ? drm_sched_fence_slab_init+0x31/0x31
[   40.056720][    T1]  ? __kasan_kmalloc+0x81/0x95
[   40.056720][    T1]  ? vgem_init+0x1cb/0x1cb
[   40.056720][    T1]  do_one_initcall+0xbc/0x3db
[   40.056720][    T1]  ? trace_event_raw_event_initcall_level+0x19c/0x19c
[   40.056720][    T1]  ? parameq+0xcc/0xcc
[   40.056720][    T1]  do_initcalls+0x1ce/0x202
[   40.056720][    T1]  kernel_init_freeable+0x21f/0x250
[   40.056720][    T1]  ? rest_init+0x202/0x202
[   40.056720][    T1]  kernel_init+0x19/0x12b
[   40.056720][    T1]  ret_from_fork+0x22/0x30
[   40.056720][    T1]  </TASK>
[   40.056720][    T1]
[   40.056720][    T1] The buggy address belongs to stack of task swapper/0/1
[   40.056720][    T1]  and is located at offset 120 in frame:
[   40.056720][    T1]  vkms_create+0x0/0x470
[   40.056720][    T1]
[   40.056720][    T1] This frame has 1 object:
[   40.056720][    T1]  [32, 120) 'pdevinfo'
[   40.056720][    T1]
[   40.056720][    T1] The buggy address belongs to the physical page:
[   40.056720][    T1] page:(____ptrval____) refcount:0 mapcount:0 mapping:0000000000000000 index:0x0 pfn:0x10083f
[   40.056720][    T1] flags: 0x8000000000000000(zone=2)
[   40.056720][    T1] raw: 8000000000000000 ffffea0004020fc8 ffffea0004020fc8 0000000000000000
[   40.056720][    T1] raw: 0000000000000000 0000000000000000 00000000ffffffff 0000000000000000
[   40.056720][    T1] page dumped because: kasan: bad access detected
[   40.056720][    T1] page_owner tracks the page as allocated
[   40.056720][    T1] page last allocated via order 0, migratetype Unmovable, gfp_mask 0x0(), pid 1, tgid 1 (swapper/0), ts 22266683981, free_ts 0
[   40.056720][    T1]  register_early_stack+0x65/0xa4
[   40.056720][    T1]  init_page_owner+0x2a/0xa7
[   40.056720][    T1]  kernel_init_freeable+0x1d7/0x250
[   40.056720][    T1]  kernel_init+0x19/0x12b
[   40.056720][    T1] page_owner free stack trace missing
[   40.056720][    T1]
[   40.056720][    T1] Memory state around the buggy address:
[   40.056720][    T1]  ffff88810083fc80: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
[   40.056720][    T1]  ffff88810083fd00: 00 00 f1 f1 f1 f1 00 00 00 00 00 00 00 00 00 00
[   40.056720][    T1] >ffff88810083fd80: 00 f3 f3 f3 f3 f3 00 00 00 00 00 00 00 00 00 00
[   40.056720][    T1]                       ^
[   40.056720][    T1]  ffff88810083fe00: 00 f1 f1 f1 f1 00 00 00 00 00 00 00 00 f3 f3 f3
[   40.056720][    T1]  ffff88810083fe80: f3 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
[   40.056720][    T1] ==================================================================
[   40.146127][    T1] Disabling lock debugging due to kernel taint
[   40.147606][    T1] general protection fault, probably for non-canonical address 0xdffffc0000000048: 0000 [#1] SMP KASAN PTI
[   40.150060][    T1] KASAN: null-ptr-deref in range [0x0000000000000240-0x0000000000000247]
[   40.150060][    T1] CPU: 0 PID: 1 Comm: swapper/0 Tainted: G    B             5.18.0-rc5-00033-gbc9eb0d5ef0a #1
[   40.150060][    T1] RIP: 0010:check_move_unevictable_folios+0x140/0xba6
[   40.150060][    T1] Code: 48 c1 e8 03 80 3c 28 00 0f 85 b1 07 00 00 48 63 c3 4d 8b 7c c5 08 be 08 00 00 00 4c 89 ff e8 e2 e6 12 00 4c 89 f8 48 c1 e8 03 <80> 3c 28 00 0f 85 a8 07 00 00 49 8b 07 a9 00 00 01 00 74 05 0f 1f
[   40.150060][    T1] RSP: 0000:ffff88810083f930 EFLAGS: 00010207
[   40.150060][    T1] RAX: 0000000000000048 RBX: 0000000000000000 RCX: ffffffff8154d44e
[   40.150060][    T1] RDX: 0000000000000000 RSI: 0000000000000008 RDI: 0000000000000246
[   40.150060][    T1] RBP: dffffc0000000000 R08: 000000000000024e R09: 0000000000000000
[   40.150060][    T1] R10: ffffffff8154d44e R11: fffffbfff0e09dc8 R12: ffff88810083f9e0
[   40.150060][    T1] R13: ffff88810083f9d8 R14: 000000000000007e R15: 0000000000000246
[   40.150060][    T1] FS:  0000000000000000(0000) GS:ffff8883aec00000(0000) knlGS:0000000000000000
[   40.150060][    T1] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[   40.150060][    T1] CR2: ffff88843ffff000 CR3: 0000000005466000 CR4: 00000000000406f0
[   40.150060][    T1] Call Trace:
[   40.150060][    T1]  <TASK>
[   40.150060][    T1]  ? kasan_report.cold+0x10/0x1a
[   40.150060][    T1]  check_move_unevictable_pages+0x156/0x40f
[   40.150060][    T1]  ? check_move_unevictable_folios+0xba6/0xba6
[   40.150060][    T1]  ? lock_is_held_type+0xa5/0x111
[   40.150060][    T1]  ? lru_lazyfree_fn+0x245/0x245
[   40.150060][    T1]  ? lock_release+0xdf/0x1fb
[   40.150060][    T1]  ? mlock_page_drain_local+0x1a9/0x31b
[   40.150060][    T1]  ? lock_is_held_type+0x96/0x111
[   40.150060][    T1]  drm_gem_put_pages+0x206/0x363
[   40.150060][    T1]  ? drm_gem_vm_open+0x75/0x75
[   40.150060][    T1]  ? slab_free_freelist_hook+0xba/0x167
[   40.150060][    T1]  ? drm_gem_shmem_vunmap+0x121/0x1a2
[   40.150060][    T1]  drm_gem_shmem_put_pages_locked+0xf8/0x1f2
[   40.150060][    T1]  drm_gem_shmem_vunmap+0x13a/0x1a2
[   40.150060][    T1]  drm_gem_vunmap+0xcb/0x1d0
[   40.150060][    T1]  drm_fbdev_cleanup+0x251/0x33d
[   40.150060][    T1]  drm_fbdev_client_hotplug+0x3fa/0x50e
[   40.150060][    T1]  drm_fbdev_generic_setup+0x16b/0x3f0
[   40.150060][    T1]  vkms_create+0x401/0x470
[   40.150060][    T1]  ? drm_sched_pick_best.cold+0x32/0x32
[   40.150060][    T1]  ? drm_sched_fence_slab_init+0x31/0x31
[   40.150060][    T1]  ? vgem_init+0x1cb/0x1cb
[   40.150060][    T1]  do_one_initcall+0xbc/0x3db
[   40.150060][    T1]  ? trace_event_raw_event_initcall_level+0x19c/0x19c
[   40.150060][    T1]  ? parameq+0xcc/0xcc
[   40.150060][    T1]  do_initcalls+0x1ce/0x202
[   40.150060][    T1]  kernel_init_freeable+0x21f/0x250
[   40.150060][    T1]  ? rest_init+0x202/0x202
[   40.150060][    T1]  kernel_init+0x19/0x12b
[   40.150060][    T1]  ret_from_fork+0x22/0x30
[   40.150060][    T1]  </TASK>



To reproduce:

        # build kernel
	cd linux
	cp config-5.18.0-rc5-00033-gbc9eb0d5ef0a .config
	make HOSTCC=gcc-11 CC=gcc-11 ARCH=x86_64 olddefconfig prepare modules_prepare bzImage modules
	make HOSTCC=gcc-11 CC=gcc-11 ARCH=x86_64 INSTALL_MOD_PATH=<mod-install-dir> modules_install
	cd <mod-install-dir>
	find lib/ | cpio -o -H newc --quiet | gzip > modules.cgz


        git clone https://github.com/intel/lkp-tests.git
        cd lkp-tests
        bin/lkp qemu -k <bzImage> -m modules.cgz job-script # job-script is attached in this email

        # if come across any failure that blocks the test,
        # please remove ~/.lkp and /lkp dir to run from a clean state.
Matthew Wilcox June 8, 2022, 4:32 p.m. UTC | #3 
On Wed, Jun 08, 2022 at 01:07:32AM -0700, Christoph Hellwig wrote:
> On Sun, Jun 05, 2022 at 08:38:52PM +0100, Matthew Wilcox (Oracle) wrote:
> > Change the guts of check_move_unevictable_pages() over to use folios
> > and add check_move_unevictable_pages() as a wrapper.
> 
> The changes here look fine, but please also add patches for converting
> the two callers (which looks mostly trivial to me).

I do want to get rid of pagevecs entirely, but that conversion isn't
going to happen in time for the next merge window.  for_each_sgt_page()
is a little intimidating.
Christoph Hellwig June 9, 2022, 3:56 a.m. UTC | #4 
On Wed, Jun 08, 2022 at 05:32:34PM +0100, Matthew Wilcox wrote:
> On Wed, Jun 08, 2022 at 01:07:32AM -0700, Christoph Hellwig wrote:
> > On Sun, Jun 05, 2022 at 08:38:52PM +0100, Matthew Wilcox (Oracle) wrote:
> > > Change the guts of check_move_unevictable_pages() over to use folios
> > > and add check_move_unevictable_pages() as a wrapper.
> > 
> > The changes here look fine, but please also add patches for converting
> > the two callers (which looks mostly trivial to me).
> 
> I do want to get rid of pagevecs entirely, but that conversion isn't
> going to happen in time for the next merge window.  for_each_sgt_page()
> is a little intimidating.

for_each_sgt_page, just like other creative scatterlist abuse in the gpu
code is a beast.  But, instead of doing a for_each_sgt_page to add
pages to the pagevec and then do a loop over the pagevec to add to
the folio batch it should be pretty trivial to just cut out the
middle man.
diff mbox series

Patch

diff --git a/include/linux/swap.h b/include/linux/swap.h
index 0c0fed1b348f..8672a7123ccd 100644
--- a/include/linux/swap.h
+++ b/include/linux/swap.h
@@ -438,7 +438,8 @@  static inline bool node_reclaim_enabled(void)
 	return node_reclaim_mode & (RECLAIM_ZONE|RECLAIM_WRITE|RECLAIM_UNMAP);
 }
 
-extern void check_move_unevictable_pages(struct pagevec *pvec);
+void check_move_unevictable_folios(struct folio_batch *fbatch);
+void check_move_unevictable_pages(struct pagevec *pvec);
 
 extern void kswapd_run(int nid);
 extern void kswapd_stop(int nid);
diff --git a/mm/vmscan.c b/mm/vmscan.c
index f7d9a683e3a7..5222c5ad600a 100644
--- a/mm/vmscan.c
+++ b/mm/vmscan.c
@@ -4790,45 +4790,56 @@  int node_reclaim(struct pglist_data *pgdat, gfp_t gfp_mask, unsigned int order)
 }
 #endif
 
+void check_move_unevictable_pages(struct pagevec *pvec)
+{
+	struct folio_batch fbatch;
+	unsigned i;
+
+	for (i = 0; i < pvec->nr; i++) {
+		struct page *page = pvec->pages[i];
+
+		if (PageTransTail(page))
+			continue;
+		folio_batch_add(&fbatch, page_folio(page));
+	}
+	check_move_unevictable_folios(&fbatch);
+}
+EXPORT_SYMBOL_GPL(check_move_unevictable_pages);
+
 /**
- * check_move_unevictable_pages - check pages for evictability and move to
- * appropriate zone lru list
- * @pvec: pagevec with lru pages to check
+ * check_move_unevictable_folios - Move evictable folios to appropriate zone
+ * lru list
+ * @fbatch: Batch of lru folios to check.
  *
- * Checks pages for evictability, if an evictable page is in the unevictable
+ * Checks folios for evictability, if an evictable folio is in the unevictable
  * lru list, moves it to the appropriate evictable lru list. This function
- * should be only used for lru pages.
+ * should be only used for lru folios.
  */
-void check_move_unevictable_pages(struct pagevec *pvec)
+void check_move_unevictable_folios(struct folio_batch *fbatch)
 {
 	struct lruvec *lruvec = NULL;
 	int pgscanned = 0;
 	int pgrescued = 0;
 	int i;
 
-	for (i = 0; i < pvec->nr; i++) {
-		struct page *page = pvec->pages[i];
-		struct folio *folio = page_folio(page);
-		int nr_pages;
-
-		if (PageTransTail(page))
-			continue;
+	for (i = 0; i < fbatch->nr; i++) {
+		struct folio *folio = fbatch->folios[i];
+		int nr_pages = folio_nr_pages(folio);
 
-		nr_pages = thp_nr_pages(page);
 		pgscanned += nr_pages;
 
-		/* block memcg migration during page moving between lru */
-		if (!TestClearPageLRU(page))
+		/* block memcg migration while the folio moves between lrus */
+		if (!folio_test_clear_lru(folio))
 			continue;
 
 		lruvec = folio_lruvec_relock_irq(folio, lruvec);
-		if (page_evictable(page) && PageUnevictable(page)) {
-			del_page_from_lru_list(page, lruvec);
-			ClearPageUnevictable(page);
-			add_page_to_lru_list(page, lruvec);
+		if (folio_evictable(folio) && folio_test_unevictable(folio)) {
+			lruvec_del_folio(lruvec, folio);
+			folio_clear_unevictable(folio);
+			lruvec_add_folio(lruvec, folio);
 			pgrescued += nr_pages;
 		}
-		SetPageLRU(page);
+		folio_set_lru(folio);
 	}
 
 	if (lruvec) {
@@ -4839,4 +4850,4 @@  void check_move_unevictable_pages(struct pagevec *pvec)
 		count_vm_events(UNEVICTABLE_PGSCANNED, pgscanned);
 	}
 }
-EXPORT_SYMBOL_GPL(check_move_unevictable_pages);
+EXPORT_SYMBOL_GPL(check_move_unevictable_folios);