2022-04-26 08:02:19

by Zi Yan

[permalink] [raw]
Subject: [PATCH v11 0/6] Use pageblock_order for cma and alloc_contig_range alignment.

From: Zi Yan <[email protected]>

Hi David,

This patchset tries to remove the MAX_ORDER-1 alignment requirement for CMA
and alloc_contig_range(). It prepares for my upcoming changes to make
MAX_ORDER adjustable at boot time[1]. It is on top of mmotm-2022-04-20-17-12.

Changelog
===
V11
---
1. Moved start_isolate_page_range()/undo_isolate_page_range() alignment
change to a separate patch after the unmovable page check change and
alloc_contig_range() change to avoid some unwanted memory
hotplug/hotremove failures.
2. Cleaned up has_unmovable_pages() in Patch 2.

V10
---
1. Reverted back to the original outer_start, outer_end range for
test_pages_isolated() and isolate_freepages_range() in Patch 3,
otherwise isolation will fail if start in alloc_contig_range() is in
the middle of a free page.

V9
---
1. Limited has_unmovable_pages() check within a pageblock.
2. Added a check to ensure page isolation is done within a single zone
in isolate_single_pageblock().
3. Fixed an off-by-one bug in isolate_single_pageblock().
4. Fixed a NULL-deferencing bug when the pages before to-be-isolated pageblock
is not online in isolate_single_pageblock().

V8
---
1. Cleaned up has_unmovable_pages() to remove page argument.

V7
---
1. Added page validity check in isolate_single_pageblock() to avoid out
of zone pages.
2. Fixed a bug in split_free_page() to split and free pages in correct
page order.

V6
---
1. Resolved compilation error/warning reported by kernel test robot.
2. Tried to solve the coding concerns from Christophe Leroy.
3. Shortened lengthy lines (pointed out by Christoph Hellwig).

V5
---
1. Moved isolation address alignment handling in start_isolate_page_range().
2. Rewrote and simplified how alloc_contig_range() works at pageblock
granularity (Patch 3). Only two pageblock migratetypes need to be saved and
restored. start_isolate_page_range() might need to migrate pages in this
version, but it prevents the caller from worrying about
max(MAX_ORDER_NR_PAEGS, pageblock_nr_pages) alignment after the page range
is isolated.

V4
---
1. Dropped two irrelevant patches on non-lru compound page handling, as
it is not supported upstream.
2. Renamed migratetype_has_fallback() to migratetype_is_mergeable().
3. Always check whether two pageblocks can be merged in
__free_one_page() when order is >= pageblock_order, as the case (not
mergeable pageblocks are isolated, CMA, and HIGHATOMIC) becomes more common.
3. Moving has_unmovable_pages() is now a separate patch.
4. Removed MAX_ORDER-1 alignment requirement in the comment in virtio_mem code.

Description
===

The MAX_ORDER - 1 alignment requirement comes from that alloc_contig_range()
isolates pageblocks to remove free memory from buddy allocator but isolating
only a subset of pageblocks within a page spanning across multiple pageblocks
causes free page accounting issues. Isolated page might not be put into the
right free list, since the code assumes the migratetype of the first pageblock
as the whole free page migratetype. This is based on the discussion at [2].

To remove the requirement, this patchset:
1. isolates pages at pageblock granularity instead of
max(MAX_ORDER_NR_PAEGS, pageblock_nr_pages);
2. splits free pages across the specified range or migrates in-use pages
across the specified range then splits the freed page to avoid free page
accounting issues (it happens when multiple pageblocks within a single page
have different migratetypes);
3. only checks unmovable pages within the range instead of MAX_ORDER - 1 aligned
range during isolation to avoid alloc_contig_range() failure when pageblocks
within a MAX_ORDER - 1 aligned range are allocated separately.
4. returns pages not in the range as it did before.

One optimization might come later:
1. make MIGRATE_ISOLATE a separate bit to be able to restore the original
migratetypes when isolation fails in the middle of the range.

Feel free to give comments and suggestions. Thanks.

[1] https://lore.kernel.org/linux-mm/[email protected]/
[2] https://lore.kernel.org/linux-mm/[email protected]/

Zi Yan (6):
mm: page_isolation: move has_unmovable_pages() to mm/page_isolation.c
mm: page_isolation: check specified range for unmovable pages
mm: make alloc_contig_range work at pageblock granularity
mm: page_isolation: enable arbitrary range page isolation.
mm: cma: use pageblock_order as the single alignment
drivers: virtio_mem: use pageblock size as the minimum virtio_mem
size.

drivers/virtio/virtio_mem.c | 6 +-
include/linux/cma.h | 4 +-
include/linux/mmzone.h | 5 +-
include/linux/page-isolation.h | 6 +-
mm/internal.h | 6 +
mm/memory_hotplug.c | 3 +-
mm/page_alloc.c | 191 +++++-------------
mm/page_isolation.c | 345 +++++++++++++++++++++++++++++++--
8 files changed, 392 insertions(+), 174 deletions(-)

--
2.35.1


2022-04-27 06:37:14

by Zi Yan

[permalink] [raw]
Subject: Re: [PATCH v11 0/6] Use pageblock_order for cma and alloc_contig_range alignment.

On 26 Apr 2022, at 17:08, Qian Cai wrote:

> On Tue, Apr 26, 2022 at 04:26:08PM -0400, Zi Yan wrote:
>> Thanks for reporting the issue. Do you have a reproducer I can use to debug the code?
>
> Nothing fancy. It just try to remove and add back each memory section.
>
> #!/usr/bin/env python3
> # SPDX-License-Identifier: GPL-2.0
>
> import os
> import re
> import subprocess
>
>
> def mem_iter():
> base_dir = '/sys/devices/system/memory/'
> for curr_dir in os.listdir(base_dir):
> if re.match(r'memory\d+', curr_dir):
> yield base_dir + curr_dir
>
>
> if __name__ == '__main__':
> print('- Try to remove each memory section and then add it back.')
> for mem_dir in mem_iter():
> status = f'{mem_dir}/online'
> if open(status).read().rstrip() == '1':
> # This could expectedly fail due to many reasons.
> section = os.path.basename(mem_dir)
> print(f'- Try to remove {section}.')
> proc = subprocess.run([f'echo 0 | sudo tee {status}'], shell=True)
> if proc.returncode == 0:
> print(f'- Try to add {section}.')
> subprocess.check_call([f'echo 1 | sudo tee {status}'], shell=True)

Thanks. Do you mind attaching your config file? I cannot reproduce
the deadlock locally using my own config. I also see kmemleak_scan
in the dumped stack, so it must be something else in addition to
memory online/offline causing the issue.

--
Best Regards,
Yan, Zi


Attachments:
signature.asc (871.00 B)
OpenPGP digital signature

2022-04-27 07:56:13

by Qian Cai

[permalink] [raw]
Subject: Re: [PATCH v11 0/6] Use pageblock_order for cma and alloc_contig_range alignment.

On Tue, Apr 26, 2022 at 04:26:08PM -0400, Zi Yan wrote:
> Thanks for reporting the issue. Do you have a reproducer I can use to debug the code?

Nothing fancy. It just try to remove and add back each memory section.

#!/usr/bin/env python3
# SPDX-License-Identifier: GPL-2.0

import os
import re
import subprocess


def mem_iter():
base_dir = '/sys/devices/system/memory/'
for curr_dir in os.listdir(base_dir):
if re.match(r'memory\d+', curr_dir):
yield base_dir + curr_dir


if __name__ == '__main__':
print('- Try to remove each memory section and then add it back.')
for mem_dir in mem_iter():
status = f'{mem_dir}/online'
if open(status).read().rstrip() == '1':
# This could expectedly fail due to many reasons.
section = os.path.basename(mem_dir)
print(f'- Try to remove {section}.')
proc = subprocess.run([f'echo 0 | sudo tee {status}'], shell=True)
if proc.returncode == 0:
print(f'- Try to add {section}.')
subprocess.check_call([f'echo 1 | sudo tee {status}'], shell=True)

2022-04-27 10:34:10

by Zi Yan

[permalink] [raw]
Subject: Re: [PATCH v11 0/6] Use pageblock_order for cma and alloc_contig_range alignment.

On 26 Apr 2022, at 16:18, Qian Cai wrote:

> On Mon, Apr 25, 2022 at 10:31:12AM -0400, Zi Yan wrote:
>> From: Zi Yan <[email protected]>
>>
>> Hi David,
>>
>> This patchset tries to remove the MAX_ORDER-1 alignment requirement for CMA
>> and alloc_contig_range(). It prepares for my upcoming changes to make
>> MAX_ORDER adjustable at boot time[1]. It is on top of mmotm-2022-04-20-17-12.
>>
>> Changelog
>> ===
>> V11
>> ---
>> 1. Moved start_isolate_page_range()/undo_isolate_page_range() alignment
>> change to a separate patch after the unmovable page check change and
>> alloc_contig_range() change to avoid some unwanted memory
>> hotplug/hotremove failures.
>> 2. Cleaned up has_unmovable_pages() in Patch 2.
>>
>> V10
>> ---
>> 1. Reverted back to the original outer_start, outer_end range for
>> test_pages_isolated() and isolate_freepages_range() in Patch 3,
>> otherwise isolation will fail if start in alloc_contig_range() is in
>> the middle of a free page.
>>
>> V9
>> ---
>> 1. Limited has_unmovable_pages() check within a pageblock.
>> 2. Added a check to ensure page isolation is done within a single zone
>> in isolate_single_pageblock().
>> 3. Fixed an off-by-one bug in isolate_single_pageblock().
>> 4. Fixed a NULL-deferencing bug when the pages before to-be-isolated pageblock
>> is not online in isolate_single_pageblock().
>>
>> V8
>> ---
>> 1. Cleaned up has_unmovable_pages() to remove page argument.
>>
>> V7
>> ---
>> 1. Added page validity check in isolate_single_pageblock() to avoid out
>> of zone pages.
>> 2. Fixed a bug in split_free_page() to split and free pages in correct
>> page order.
>>
>> V6
>> ---
>> 1. Resolved compilation error/warning reported by kernel test robot.
>> 2. Tried to solve the coding concerns from Christophe Leroy.
>> 3. Shortened lengthy lines (pointed out by Christoph Hellwig).
>>
>> V5
>> ---
>> 1. Moved isolation address alignment handling in start_isolate_page_range().
>> 2. Rewrote and simplified how alloc_contig_range() works at pageblock
>> granularity (Patch 3). Only two pageblock migratetypes need to be saved and
>> restored. start_isolate_page_range() might need to migrate pages in this
>> version, but it prevents the caller from worrying about
>> max(MAX_ORDER_NR_PAEGS, pageblock_nr_pages) alignment after the page range
>> is isolated.
>>
>> V4
>> ---
>> 1. Dropped two irrelevant patches on non-lru compound page handling, as
>> it is not supported upstream.
>> 2. Renamed migratetype_has_fallback() to migratetype_is_mergeable().
>> 3. Always check whether two pageblocks can be merged in
>> __free_one_page() when order is >= pageblock_order, as the case (not
>> mergeable pageblocks are isolated, CMA, and HIGHATOMIC) becomes more common.
>> 3. Moving has_unmovable_pages() is now a separate patch.
>> 4. Removed MAX_ORDER-1 alignment requirement in the comment in virtio_mem code.
>>
>> Description
>> ===
>>
>> The MAX_ORDER - 1 alignment requirement comes from that alloc_contig_range()
>> isolates pageblocks to remove free memory from buddy allocator but isolating
>> only a subset of pageblocks within a page spanning across multiple pageblocks
>> causes free page accounting issues. Isolated page might not be put into the
>> right free list, since the code assumes the migratetype of the first pageblock
>> as the whole free page migratetype. This is based on the discussion at [2].
>>
>> To remove the requirement, this patchset:
>> 1. isolates pages at pageblock granularity instead of
>> max(MAX_ORDER_NR_PAEGS, pageblock_nr_pages);
>> 2. splits free pages across the specified range or migrates in-use pages
>> across the specified range then splits the freed page to avoid free page
>> accounting issues (it happens when multiple pageblocks within a single page
>> have different migratetypes);
>> 3. only checks unmovable pages within the range instead of MAX_ORDER - 1 aligned
>> range during isolation to avoid alloc_contig_range() failure when pageblocks
>> within a MAX_ORDER - 1 aligned range are allocated separately.
>> 4. returns pages not in the range as it did before.
>>
>> One optimization might come later:
>> 1. make MIGRATE_ISOLATE a separate bit to be able to restore the original
>> migratetypes when isolation fails in the middle of the range.
>>
>> Feel free to give comments and suggestions. Thanks.
>>
>> [1] https://lore.kernel.org/linux-mm/[email protected]/
>> [2] https://lore.kernel.org/linux-mm/[email protected]/
>>
>> Zi Yan (6):
>> mm: page_isolation: move has_unmovable_pages() to mm/page_isolation.c
>> mm: page_isolation: check specified range for unmovable pages
>> mm: make alloc_contig_range work at pageblock granularity
>> mm: page_isolation: enable arbitrary range page isolation.
>> mm: cma: use pageblock_order as the single alignment
>> drivers: virtio_mem: use pageblock size as the minimum virtio_mem
>> size.
>>
>> drivers/virtio/virtio_mem.c | 6 +-
>> include/linux/cma.h | 4 +-
>> include/linux/mmzone.h | 5 +-
>> include/linux/page-isolation.h | 6 +-
>> mm/internal.h | 6 +
>> mm/memory_hotplug.c | 3 +-
>> mm/page_alloc.c | 191 +++++-------------
>> mm/page_isolation.c | 345 +++++++++++++++++++++++++++++++--
>> 8 files changed, 392 insertions(+), 174 deletions(-)
>
> Reverting this series fixed a deadlock during memory offline/online
> tests and then a crash.

Hi Qian,

Thanks for reporting the issue. Do you have a reproducer I can use to debug the code?

>
> INFO: task kmemleak:1027 blocked for more than 120 seconds.
> Not tainted 5.18.0-rc4-next-20220426-dirty #27
> "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
> task:kmemleak state:D stack:27744 pid: 1027 ppid: 2 flags:0x00000008
> Call trace:
> __switch_to
> __schedule
> schedule
> percpu_rwsem_wait
> __percpu_down_read
> percpu_down_read.constprop.0
> get_online_mems
> kmemleak_scan
> kmemleak_scan_thread
> kthread
> ret_from_fork
>
> Showing all locks held in the system:
> 1 lock held by rcu_tasks_kthre/11:
> #0: ffffc1e2cefc17f0 (rcu_tasks.tasks_gp_mutex){+.+.}-{3:3}, at: rcu_tasks_one_gp
> 1 lock held by rcu_tasks_rude_/12:
> #0: ffffc1e2cefc1a90 (rcu_tasks_rude.tasks_gp_mutex){+.+.}-{3:3}, at: rcu_tasks_one_gp
> 1 lock held by rcu_tasks_trace/13:
> #0: ffffc1e2cefc1db0 (rcu_tasks_trace.tasks_gp_mutex){+.+.}-{3:3}, at: rcu_tasks_one_gp
> 1 lock held by khungtaskd/824:
> #0: ffffc1e2cefc2820 (rcu_read_lock){....}-{1:2}, at: debug_show_all_locks
> 2 locks held by kmemleak/1027:
> #0: ffffc1e2cf1aa628 (scan_mutex){+.+.}-{3:3}, at: kmemleak_scan_thread
> #1: ffffc1e2cf14e690 (mem_hotplug_lock){++++}-{0:0}, at: get_online_mems
> 2 locks held by cppc_fie/1805:
> 1 lock held by in:imklog/2822:
> 8 locks held by tee/3334:
> #0: ffff0816d65c9438 (sb_writers#6){.+.+}-{0:0}, at: vfs_write
> #1: ffff40025438be88 (&of->mutex){+.+.}-{3:3}, at: kernfs_fop_write_iter
> #2: ffff4000c8261eb0 (kn->active#298){.+.+}-{0:0}, at: kernfs_fop_write_iter
> #3: ffffc1e2d0013f68 (device_hotplug_lock){+.+.}-{3:3}, at: online_store
> #4: ffff0800cd8bb998 (&dev->mutex){....}-{3:3}, at: device_offline
> #5: ffffc1e2ceed3750 (cpu_hotplug_lock){++++}-{0:0}, at: cpus_read_lock
> #6: ffffc1e2cf14e690 (mem_hotplug_lock){++++}-{0:0}, at: offline_pages
> #7: ffffc1e2cf13bf68 (pcp_batch_high_lock){+.+.}-{3:3}, at: zone_pcp_disable
> __zone_set_pageset_high_and_batch at mm/page_alloc.c:7005
> (inlined by) zone_pcp_disable at mm/page_alloc.c:9286
>
> Later, running some kernel compilation workloads could trigger a crash.
>
> Unable to handle kernel paging request at virtual address fffffbfffe000030
> KASAN: maybe wild-memory-access in range [0x0003dffff0000180-0x0003dffff0000187]
> Mem abort info:
> ESR = 0x96000006
> EC = 0x25: DABT (current EL), IL = 32 bits
> SET = 0, FnV = 0
> EA = 0, S1PTW = 0
> FSC = 0x06: level 2 translation fault
> Data abort info:
> ISV = 0, ISS = 0x00000006
> CM = 0, WnR = 0
> swapper pgtable: 4k pages, 48-bit VAs, pgdp=00000817545fd000
> [fffffbfffe000030] pgd=00000817581e9003, p4d=00000817581e9003, pud=00000817581ea003, pmd=0000000000000000
> Internal error: Oops: 96000006 [#1] PREEMPT SMP
> Modules linked in: bridge stp llc cdc_ether usbnet ipmi_devintf ipmi_msghandler cppc_cpufreq fuse ip_tables x_tables ipv6 btrfs blake2b_generic libcrc32c xor xor_neon raid6_pq zstd_compress dm_mod nouveau drm_ttm_helper ttm crct10dif_ce mlx5_core drm_display_helper drm_kms_helper nvme mpt3sas xhci_pci nvme_core drm raid_class xhci_pci_renesas
> CPU: 147 PID: 3334 Comm: tee Not tainted 5.18.0-rc4-next-20220426-dirty #27
> pstate: 10400009 (nzcV daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--)
> pc : isolate_single_pageblock
> lr : isolate_single_pageblock
> sp : ffff80003e767500
> x29: ffff80003e767500 x28: 0000000000000000 x27: ffff783c59963b1f
> x26: dfff800000000000 x25: ffffc1e2ccb1d000 x24: ffffc1e2ccb1d8f8
> x23: 00000000803bfe00 x22: ffffc1e2cee39098 x21: 0000000000000020
> x20: 00000000803c0000 x19: fffffbfffe000000 x18: ffffc1e2cee37d1c
> x17: 0000000000000000 x16: 1fffe8004a86f14c x15: 1fffe806c89e154a
> x14: 1fffe8004a86f11c x13: 0000000000000004 x12: ffff783c5c455e6d
> x11: 1ffff83c5c455e6c x10: ffff783c5c455e6c x9 : dfff800000000000
> x8 : ffffc1e2e22af363 x7 : 0000000000000001 x6 : 0000000000000003
> x5 : ffffc1e2e22af360 x4 : ffff783c5c455e6c x3 : ffff700007cece90
> x2 : 0000000000000003 x1 : 0000000000000000 x0 : fffffbfffe000030
> Call trace:
> Call trace:
> isolate_single_pageblock
> PageBuddy at ./include/linux/page-flags.h:969 (discriminator 3)
> (inlined by) isolate_single_pageblock at mm/page_isolation.c:414 (discriminator 3)
> start_isolate_page_range
> offline_pages
> memory_subsys_offline
> device_offline
> online_store
> dev_attr_store
> sysfs_kf_write
> kernfs_fop_write_iter
> new_sync_write
> vfs_write
> ksys_write
> __arm64_sys_write
> invoke_syscall
> el0_svc_common.constprop.0
> do_el0_svc
> el0_svc
> el0t_64_sync_handler
> el0t_64_sync
> Code: 38fa6821 7100003f 7a411041 54000dca (b9403260)
> ---[ end trace 0000000000000000 ]---
> Kernel panic - not syncing: Oops: Fatal exception
> SMP: stopping secondary CPUs
> Kernel Offset: 0x41e2c0720000 from 0xffff800008000000
> PHYS_OFFSET: 0x80000000
> CPU features: 0x000,0021700d,19801c82
> Memory Limit: none

--
Best Regards,
Yan, Zi


Attachments:
signature.asc (871.00 B)
OpenPGP digital signature

2022-04-27 11:01:33

by Qian Cai

[permalink] [raw]
Subject: Re: [PATCH v11 0/6] Use pageblock_order for cma and alloc_contig_range alignment.

On Mon, Apr 25, 2022 at 10:31:12AM -0400, Zi Yan wrote:
> From: Zi Yan <[email protected]>
>
> Hi David,
>
> This patchset tries to remove the MAX_ORDER-1 alignment requirement for CMA
> and alloc_contig_range(). It prepares for my upcoming changes to make
> MAX_ORDER adjustable at boot time[1]. It is on top of mmotm-2022-04-20-17-12.
>
> Changelog
> ===
> V11
> ---
> 1. Moved start_isolate_page_range()/undo_isolate_page_range() alignment
> change to a separate patch after the unmovable page check change and
> alloc_contig_range() change to avoid some unwanted memory
> hotplug/hotremove failures.
> 2. Cleaned up has_unmovable_pages() in Patch 2.
>
> V10
> ---
> 1. Reverted back to the original outer_start, outer_end range for
> test_pages_isolated() and isolate_freepages_range() in Patch 3,
> otherwise isolation will fail if start in alloc_contig_range() is in
> the middle of a free page.
>
> V9
> ---
> 1. Limited has_unmovable_pages() check within a pageblock.
> 2. Added a check to ensure page isolation is done within a single zone
> in isolate_single_pageblock().
> 3. Fixed an off-by-one bug in isolate_single_pageblock().
> 4. Fixed a NULL-deferencing bug when the pages before to-be-isolated pageblock
> is not online in isolate_single_pageblock().
>
> V8
> ---
> 1. Cleaned up has_unmovable_pages() to remove page argument.
>
> V7
> ---
> 1. Added page validity check in isolate_single_pageblock() to avoid out
> of zone pages.
> 2. Fixed a bug in split_free_page() to split and free pages in correct
> page order.
>
> V6
> ---
> 1. Resolved compilation error/warning reported by kernel test robot.
> 2. Tried to solve the coding concerns from Christophe Leroy.
> 3. Shortened lengthy lines (pointed out by Christoph Hellwig).
>
> V5
> ---
> 1. Moved isolation address alignment handling in start_isolate_page_range().
> 2. Rewrote and simplified how alloc_contig_range() works at pageblock
> granularity (Patch 3). Only two pageblock migratetypes need to be saved and
> restored. start_isolate_page_range() might need to migrate pages in this
> version, but it prevents the caller from worrying about
> max(MAX_ORDER_NR_PAEGS, pageblock_nr_pages) alignment after the page range
> is isolated.
>
> V4
> ---
> 1. Dropped two irrelevant patches on non-lru compound page handling, as
> it is not supported upstream.
> 2. Renamed migratetype_has_fallback() to migratetype_is_mergeable().
> 3. Always check whether two pageblocks can be merged in
> __free_one_page() when order is >= pageblock_order, as the case (not
> mergeable pageblocks are isolated, CMA, and HIGHATOMIC) becomes more common.
> 3. Moving has_unmovable_pages() is now a separate patch.
> 4. Removed MAX_ORDER-1 alignment requirement in the comment in virtio_mem code.
>
> Description
> ===
>
> The MAX_ORDER - 1 alignment requirement comes from that alloc_contig_range()
> isolates pageblocks to remove free memory from buddy allocator but isolating
> only a subset of pageblocks within a page spanning across multiple pageblocks
> causes free page accounting issues. Isolated page might not be put into the
> right free list, since the code assumes the migratetype of the first pageblock
> as the whole free page migratetype. This is based on the discussion at [2].
>
> To remove the requirement, this patchset:
> 1. isolates pages at pageblock granularity instead of
> max(MAX_ORDER_NR_PAEGS, pageblock_nr_pages);
> 2. splits free pages across the specified range or migrates in-use pages
> across the specified range then splits the freed page to avoid free page
> accounting issues (it happens when multiple pageblocks within a single page
> have different migratetypes);
> 3. only checks unmovable pages within the range instead of MAX_ORDER - 1 aligned
> range during isolation to avoid alloc_contig_range() failure when pageblocks
> within a MAX_ORDER - 1 aligned range are allocated separately.
> 4. returns pages not in the range as it did before.
>
> One optimization might come later:
> 1. make MIGRATE_ISOLATE a separate bit to be able to restore the original
> migratetypes when isolation fails in the middle of the range.
>
> Feel free to give comments and suggestions. Thanks.
>
> [1] https://lore.kernel.org/linux-mm/[email protected]/
> [2] https://lore.kernel.org/linux-mm/[email protected]/
>
> Zi Yan (6):
> mm: page_isolation: move has_unmovable_pages() to mm/page_isolation.c
> mm: page_isolation: check specified range for unmovable pages
> mm: make alloc_contig_range work at pageblock granularity
> mm: page_isolation: enable arbitrary range page isolation.
> mm: cma: use pageblock_order as the single alignment
> drivers: virtio_mem: use pageblock size as the minimum virtio_mem
> size.
>
> drivers/virtio/virtio_mem.c | 6 +-
> include/linux/cma.h | 4 +-
> include/linux/mmzone.h | 5 +-
> include/linux/page-isolation.h | 6 +-
> mm/internal.h | 6 +
> mm/memory_hotplug.c | 3 +-
> mm/page_alloc.c | 191 +++++-------------
> mm/page_isolation.c | 345 +++++++++++++++++++++++++++++++--
> 8 files changed, 392 insertions(+), 174 deletions(-)

Reverting this series fixed a deadlock during memory offline/online
tests and then a crash.

INFO: task kmemleak:1027 blocked for more than 120 seconds.
Not tainted 5.18.0-rc4-next-20220426-dirty #27
"echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
task:kmemleak state:D stack:27744 pid: 1027 ppid: 2 flags:0x00000008
Call trace:
__switch_to
__schedule
schedule
percpu_rwsem_wait
__percpu_down_read
percpu_down_read.constprop.0
get_online_mems
kmemleak_scan
kmemleak_scan_thread
kthread
ret_from_fork

Showing all locks held in the system:
1 lock held by rcu_tasks_kthre/11:
#0: ffffc1e2cefc17f0 (rcu_tasks.tasks_gp_mutex){+.+.}-{3:3}, at: rcu_tasks_one_gp
1 lock held by rcu_tasks_rude_/12:
#0: ffffc1e2cefc1a90 (rcu_tasks_rude.tasks_gp_mutex){+.+.}-{3:3}, at: rcu_tasks_one_gp
1 lock held by rcu_tasks_trace/13:
#0: ffffc1e2cefc1db0 (rcu_tasks_trace.tasks_gp_mutex){+.+.}-{3:3}, at: rcu_tasks_one_gp
1 lock held by khungtaskd/824:
#0: ffffc1e2cefc2820 (rcu_read_lock){....}-{1:2}, at: debug_show_all_locks
2 locks held by kmemleak/1027:
#0: ffffc1e2cf1aa628 (scan_mutex){+.+.}-{3:3}, at: kmemleak_scan_thread
#1: ffffc1e2cf14e690 (mem_hotplug_lock){++++}-{0:0}, at: get_online_mems
2 locks held by cppc_fie/1805:
1 lock held by in:imklog/2822:
8 locks held by tee/3334:
#0: ffff0816d65c9438 (sb_writers#6){.+.+}-{0:0}, at: vfs_write
#1: ffff40025438be88 (&of->mutex){+.+.}-{3:3}, at: kernfs_fop_write_iter
#2: ffff4000c8261eb0 (kn->active#298){.+.+}-{0:0}, at: kernfs_fop_write_iter
#3: ffffc1e2d0013f68 (device_hotplug_lock){+.+.}-{3:3}, at: online_store
#4: ffff0800cd8bb998 (&dev->mutex){....}-{3:3}, at: device_offline
#5: ffffc1e2ceed3750 (cpu_hotplug_lock){++++}-{0:0}, at: cpus_read_lock
#6: ffffc1e2cf14e690 (mem_hotplug_lock){++++}-{0:0}, at: offline_pages
#7: ffffc1e2cf13bf68 (pcp_batch_high_lock){+.+.}-{3:3}, at: zone_pcp_disable
__zone_set_pageset_high_and_batch at mm/page_alloc.c:7005
(inlined by) zone_pcp_disable at mm/page_alloc.c:9286

Later, running some kernel compilation workloads could trigger a crash.

Unable to handle kernel paging request at virtual address fffffbfffe000030
KASAN: maybe wild-memory-access in range [0x0003dffff0000180-0x0003dffff0000187]
Mem abort info:
ESR = 0x96000006
EC = 0x25: DABT (current EL), IL = 32 bits
SET = 0, FnV = 0
EA = 0, S1PTW = 0
FSC = 0x06: level 2 translation fault
Data abort info:
ISV = 0, ISS = 0x00000006
CM = 0, WnR = 0
swapper pgtable: 4k pages, 48-bit VAs, pgdp=00000817545fd000
[fffffbfffe000030] pgd=00000817581e9003, p4d=00000817581e9003, pud=00000817581ea003, pmd=0000000000000000
Internal error: Oops: 96000006 [#1] PREEMPT SMP
Modules linked in: bridge stp llc cdc_ether usbnet ipmi_devintf ipmi_msghandler cppc_cpufreq fuse ip_tables x_tables ipv6 btrfs blake2b_generic libcrc32c xor xor_neon raid6_pq zstd_compress dm_mod nouveau drm_ttm_helper ttm crct10dif_ce mlx5_core drm_display_helper drm_kms_helper nvme mpt3sas xhci_pci nvme_core drm raid_class xhci_pci_renesas
CPU: 147 PID: 3334 Comm: tee Not tainted 5.18.0-rc4-next-20220426-dirty #27
pstate: 10400009 (nzcV daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--)
pc : isolate_single_pageblock
lr : isolate_single_pageblock
sp : ffff80003e767500
x29: ffff80003e767500 x28: 0000000000000000 x27: ffff783c59963b1f
x26: dfff800000000000 x25: ffffc1e2ccb1d000 x24: ffffc1e2ccb1d8f8
x23: 00000000803bfe00 x22: ffffc1e2cee39098 x21: 0000000000000020
x20: 00000000803c0000 x19: fffffbfffe000000 x18: ffffc1e2cee37d1c
x17: 0000000000000000 x16: 1fffe8004a86f14c x15: 1fffe806c89e154a
x14: 1fffe8004a86f11c x13: 0000000000000004 x12: ffff783c5c455e6d
x11: 1ffff83c5c455e6c x10: ffff783c5c455e6c x9 : dfff800000000000
x8 : ffffc1e2e22af363 x7 : 0000000000000001 x6 : 0000000000000003
x5 : ffffc1e2e22af360 x4 : ffff783c5c455e6c x3 : ffff700007cece90
x2 : 0000000000000003 x1 : 0000000000000000 x0 : fffffbfffe000030
Call trace:
Call trace:
isolate_single_pageblock
PageBuddy at ./include/linux/page-flags.h:969 (discriminator 3)
(inlined by) isolate_single_pageblock at mm/page_isolation.c:414 (discriminator 3)
start_isolate_page_range
offline_pages
memory_subsys_offline
device_offline
online_store
dev_attr_store
sysfs_kf_write
kernfs_fop_write_iter
new_sync_write
vfs_write
ksys_write
__arm64_sys_write
invoke_syscall
el0_svc_common.constprop.0
do_el0_svc
el0_svc
el0t_64_sync_handler
el0t_64_sync
Code: 38fa6821 7100003f 7a411041 54000dca (b9403260)
---[ end trace 0000000000000000 ]---
Kernel panic - not syncing: Oops: Fatal exception
SMP: stopping secondary CPUs
Kernel Offset: 0x41e2c0720000 from 0xffff800008000000
PHYS_OFFSET: 0x80000000
CPU features: 0x000,0021700d,19801c82
Memory Limit: none

2022-04-27 13:08:33

by Qian Cai

[permalink] [raw]
Subject: Re: [PATCH v11 0/6] Use pageblock_order for cma and alloc_contig_range alignment.

On Tue, Apr 26, 2022 at 05:38:58PM -0400, Zi Yan wrote:
> Thanks. Do you mind attaching your config file? I cannot reproduce
> the deadlock locally using my own config. I also see kmemleak_scan
> in the dumped stack, so it must be something else in addition to
> memory online/offline causing the issue.

This is on an arm64 server.

$ make defconfig debug.config

2022-04-27 13:38:36

by Qian Cai

[permalink] [raw]
Subject: Re: [PATCH v11 0/6] Use pageblock_order for cma and alloc_contig_range alignment.

On Tue, Apr 26, 2022 at 05:38:58PM -0400, Zi Yan wrote:
> Thanks. Do you mind attaching your config file? I cannot reproduce
> the deadlock locally using my own config. I also see kmemleak_scan
> in the dumped stack, so it must be something else in addition to
> memory online/offline causing the issue.

Of course it also need to enable those. The kmemleak_scan is just a
symptom of one of the online operations is blocking forever, as the
locks were never released.

CONFIG_MEMORY_HOTPLUG=y
CONFIG_MEMORY_HOTPLUG_DEFAULT_ONLINE=y
CONFIG_MEMORY_HOTREMOVE=y

2022-04-27 13:55:38

by Qian Cai

[permalink] [raw]
Subject: Re: [PATCH v11 0/6] Use pageblock_order for cma and alloc_contig_range alignment.

On Tue, Apr 26, 2022 at 05:38:58PM -0400, Zi Yan wrote:
> Thanks. Do you mind attaching your config file? I cannot reproduce
> the deadlock locally using my own config. I also see kmemleak_scan
> in the dumped stack, so it must be something else in addition to
> memory online/offline causing the issue.

Actually, it is one of those *offline* operations, i.e.,

echo 0 > /sys/devices/system/memory/memoryNNN/online

looping forever which never finish after more than 2-hour.

2022-04-27 14:00:47

by Zi Yan

[permalink] [raw]
Subject: Re: [PATCH v11 0/6] Use pageblock_order for cma and alloc_contig_range alignment.

On 27 Apr 2022, at 9:27, Qian Cai wrote:

> On Tue, Apr 26, 2022 at 05:38:58PM -0400, Zi Yan wrote:
>> Thanks. Do you mind attaching your config file? I cannot reproduce
>> the deadlock locally using my own config. I also see kmemleak_scan
>> in the dumped stack, so it must be something else in addition to
>> memory online/offline causing the issue.
>
> Actually, it is one of those *offline* operations, i.e.,
>
> echo 0 > /sys/devices/system/memory/memoryNNN/online
>
> looping forever which never finish after more than 2-hour.

Thank you for the detailed information. I am able to reproduce the
issue locally. I will update the patch once I fix the bug.

--
Best Regards,
Yan, Zi


Attachments:
signature.asc (871.00 B)
OpenPGP digital signature

2022-04-28 15:07:30

by Zi Yan

[permalink] [raw]
Subject: Re: [PATCH v11 0/6] Use pageblock_order for cma and alloc_contig_range alignment.

On 27 Apr 2022, at 9:30, Zi Yan wrote:

> On 27 Apr 2022, at 9:27, Qian Cai wrote:
>
>> On Tue, Apr 26, 2022 at 05:38:58PM -0400, Zi Yan wrote:
>>> Thanks. Do you mind attaching your config file? I cannot reproduce
>>> the deadlock locally using my own config. I also see kmemleak_scan
>>> in the dumped stack, so it must be something else in addition to
>>> memory online/offline causing the issue.
>>
>> Actually, it is one of those *offline* operations, i.e.,
>>
>> echo 0 > /sys/devices/system/memory/memoryNNN/online
>>
>> looping forever which never finish after more than 2-hour.
>
> Thank you for the detailed information. I am able to reproduce the
> issue locally. I will update the patch once I fix the bug.

Hi Qian,

Do you mind checking if the patch below fixes the issue? It works
for me.

The original code was trying to migrate non-migratible compound pages
(high-order slab pages from my tests) during isolation and caused
an infinite loop. The patch avoids non-migratible pages.

I will update my patch series once we confirm the patch fixes
the bug.

Thanks.

diff --git a/mm/page_isolation.c b/mm/page_isolation.c
index 75e454f5cf45..c39980fce626 100644
--- a/mm/page_isolation.c
+++ b/mm/page_isolation.c
@@ -367,58 +367,68 @@ static int isolate_single_pageblock(unsigned long boundary_pfn, gfp_t gfp_flags,
}
/*
* migrate compound pages then let the free page handling code
- * above do the rest. If migration is not enabled, just fail.
+ * above do the rest. If migration is not possible, just fail.
*/
- if (PageHuge(page) || PageTransCompound(page)) {
-#if defined CONFIG_COMPACTION || defined CONFIG_CMA
+ if (PageCompound(page)) {
unsigned long nr_pages = compound_nr(page);
- int order = compound_order(page);
struct page *head = compound_head(page);
unsigned long head_pfn = page_to_pfn(head);
- int ret;
- struct compact_control cc = {
- .nr_migratepages = 0,
- .order = -1,
- .zone = page_zone(pfn_to_page(head_pfn)),
- .mode = MIGRATE_SYNC,
- .ignore_skip_hint = true,
- .no_set_skip_hint = true,
- .gfp_mask = gfp_flags,
- .alloc_contig = true,
- };
- INIT_LIST_HEAD(&cc.migratepages);

if (head_pfn + nr_pages < boundary_pfn) {
- pfn += nr_pages;
+ pfn = head_pfn + nr_pages;
continue;
}

- ret = __alloc_contig_migrate_range(&cc, head_pfn,
- head_pfn + nr_pages);
-
- if (ret)
- goto failed;
+#if defined CONFIG_MIGRATION
/*
- * reset pfn, let the free page handling code above
- * split the free page to the right migratetype list.
- *
- * head_pfn is not used here as a hugetlb page order
- * can be bigger than MAX_ORDER-1, but after it is
- * freed, the free page order is not. Use pfn within
- * the range to find the head of the free page and
- * reset order to 0 if a hugetlb page with
- * >MAX_ORDER-1 order is encountered.
+ * hugetlb, lru compound (THP), and movable compound pages
+ * can be migrated. Otherwise, fail the isolation.
*/
- if (order > MAX_ORDER-1)
+ if (PageHuge(page) || PageLRU(page) || __PageMovable(page)) {
+ int order;
+ unsigned long outer_pfn;
+ int ret;
+ struct compact_control cc = {
+ .nr_migratepages = 0,
+ .order = -1,
+ .zone = page_zone(pfn_to_page(head_pfn)),
+ .mode = MIGRATE_SYNC,
+ .ignore_skip_hint = true,
+ .no_set_skip_hint = true,
+ .gfp_mask = gfp_flags,
+ .alloc_contig = true,
+ };
+ INIT_LIST_HEAD(&cc.migratepages);
+
+ ret = __alloc_contig_migrate_range(&cc, head_pfn,
+ head_pfn + nr_pages);
+
+ if (ret)
+ goto failed;
+ /*
+ * reset pfn to the head of the free page, so
+ * that the free page handling code above can split
+ * the free page to the right migratetype list.
+ *
+ * head_pfn is not used here as a hugetlb page order
+ * can be bigger than MAX_ORDER-1, but after it is
+ * freed, the free page order is not. Use pfn within
+ * the range to find the head of the free page.
+ */
order = 0;
- while (!PageBuddy(pfn_to_page(pfn))) {
- order++;
- pfn &= ~0UL << order;
- }
- continue;
-#else
- goto failed;
+ outer_pfn = pfn;
+ while (!PageBuddy(pfn_to_page(outer_pfn))) {
+ if (++order >= MAX_ORDER) {
+ outer_pfn = pfn;
+ break;
+ }
+ outer_pfn &= ~0UL << order;
+ }
+ pfn = outer_pfn;
+ continue;
+ } else
#endif
+ goto failed;
}

pfn++;
--
Best Regards,
Yan, Zi


Attachments:
signature.asc (871.00 B)
OpenPGP digital signature

2022-04-28 18:19:24

by Qian Cai

[permalink] [raw]
Subject: Re: [PATCH v11 0/6] Use pageblock_order for cma and alloc_contig_range alignment.

On Thu, Apr 28, 2022 at 08:39:06AM -0400, Zi Yan wrote:
> How about the one attached? I can apply it to next-20220428. Let me know
> if you are using a different branch. Thanks.

The original endless loop is gone, but running some syscall fuzzer
afterwards for a while would trigger the warning here. I have yet to
figure out if this is related to this series.

/*
* There are several places where we assume that the order value is sane
* so bail out early if the request is out of bound.
*/
if (unlikely(order >= MAX_ORDER)) {
WARN_ON_ONCE(!(gfp & __GFP_NOWARN));
return NULL;
}

WARNING: CPU: 26 PID: 172874 at mm/page_alloc.c:5368 __alloc_pages
CPU: 26 PID: 172874 Comm: trinity-main Not tainted 5.18.0-rc4-next-20220428-dirty #67
pstate: 20400009 (nzCv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--)
tpidr_el2 : ffff28cf80a61000
pc : __alloc_pages
lr : alloc_pages
sp : ffff8000597b70f0
x29: ffff8000597b70f0 x28: ffff0801e68d34c0 x27: 0000000000000000
x26: 1ffff0000b2f6ea2 x25: ffff8000597b7510 x24: 0000000000000dc0
x23: ffff28cf80a61000 x22: 000000000000000e x21: 1ffff0000b2f6e28
x20: 0000000000040dc0 x19: ffffdf670d4a6fe0 x18: ffffdf66fa017d1c
x17: ffffdf66f42f8348 x16: 1fffe1003cd1a7b3 x15: 000000000000001a
x14: 1fffe1003cd1a7a6 x13: 0000000000000004 x12: ffff70000b2f6e05
x11: 1ffff0000b2f6e04 x10: 00000000f204f1f1 x9 : 000000000000f204
x8 : dfff800000000000 x7 : 00000000f3000000 x6 : 00000000f3f3f3f3
x5 : ffff70000b2f6e28 x4 : ffff0801e68d34c0 x3 : 0000000000000000
x2 : 0000000000000000 x1 : 0000000000000001 x0 : 0000000000040dc0
Call trace:
__alloc_pages
alloc_pages
kmalloc_order
kmalloc_order_trace
__kmalloc
__regset_get
regset_get_alloc
fill_thread_core_info
fill_note_info
elf_core_dump
do_coredump
get_signal
do_signal
do_notify_resume
el0_svc
el0t_64_sync_handler
el0t_64_sync
irq event stamp: 3614
hardirqs last enabled at (3613): _raw_spin_unlock_irqrestore
hardirqs last disabled at (3614): el1_dbg
softirqs last enabled at (2988): fpsimd_preserve_current_state
softirqs last disabled at (2986): fpsimd_preserve_current_state

2022-04-29 11:03:08

by Zi Yan

[permalink] [raw]
Subject: Re: [PATCH v11 0/6] Use pageblock_order for cma and alloc_contig_range alignment.

On 28 Apr 2022, at 8:33, Qian Cai wrote:

> On Wed, Apr 27, 2022 at 05:04:39PM -0400, Zi Yan wrote:
>> Do you mind checking if the patch below fixes the issue? It works
>> for me.
>>
>> The original code was trying to migrate non-migratible compound pages
>> (high-order slab pages from my tests) during isolation and caused
>> an infinite loop. The patch avoids non-migratible pages.
>>
>> I will update my patch series once we confirm the patch fixes
>> the bug.
>
> I am not able to apply it on today's linux-next tree.
>
> $ patch -Np1 --dry-run < ../patch/migrate.patch
> checking file mm/page_isolation.c
> Hunk #1 FAILED at 367.
> 1 out of 1 hunk FAILED

How about the one attached? I can apply it to next-20220428. Let me know
if you are using a different branch. Thanks.


--
Best Regards,
Yan, Zi


Attachments:
0001-fix-what-can-be-migrated-what-cannot.patch (3.79 kB)
signature.asc (871.00 B)
OpenPGP digital signature
Download all attachments

2022-05-02 23:53:09

by Qian Cai

[permalink] [raw]
Subject: Re: [PATCH v11 0/6] Use pageblock_order for cma and alloc_contig_range alignment.

On Wed, Apr 27, 2022 at 05:04:39PM -0400, Zi Yan wrote:
> Do you mind checking if the patch below fixes the issue? It works
> for me.
>
> The original code was trying to migrate non-migratible compound pages
> (high-order slab pages from my tests) during isolation and caused
> an infinite loop. The patch avoids non-migratible pages.
>
> I will update my patch series once we confirm the patch fixes
> the bug.

I am not able to apply it on today's linux-next tree.

$ patch -Np1 --dry-run < ../patch/migrate.patch
checking file mm/page_isolation.c
Hunk #1 FAILED at 367.
1 out of 1 hunk FAILED

2022-05-03 00:01:35

by Zi Yan

[permalink] [raw]
Subject: Re: [PATCH v11 0/6] Use pageblock_order for cma and alloc_contig_range alignment.

On 28 Apr 2022, at 12:19, Qian Cai wrote:

> On Thu, Apr 28, 2022 at 08:39:06AM -0400, Zi Yan wrote:
>> How about the one attached? I can apply it to next-20220428. Let me know
>> if you are using a different branch. Thanks.
>
> The original endless loop is gone, but running some syscall fuzzer

Thanks for the confirmation.

> afterwards for a while would trigger the warning here. I have yet to
> figure out if this is related to this series.
>
> /*
> * There are several places where we assume that the order value is sane
> * so bail out early if the request is out of bound.
> */
> if (unlikely(order >= MAX_ORDER)) {
> WARN_ON_ONCE(!(gfp & __GFP_NOWARN));
> return NULL;
> }
>
> WARNING: CPU: 26 PID: 172874 at mm/page_alloc.c:5368 __alloc_pages
> CPU: 26 PID: 172874 Comm: trinity-main Not tainted 5.18.0-rc4-next-20220428-dirty #67
> pstate: 20400009 (nzCv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--)
> tpidr_el2 : ffff28cf80a61000
> pc : __alloc_pages
> lr : alloc_pages
> sp : ffff8000597b70f0
> x29: ffff8000597b70f0 x28: ffff0801e68d34c0 x27: 0000000000000000
> x26: 1ffff0000b2f6ea2 x25: ffff8000597b7510 x24: 0000000000000dc0
> x23: ffff28cf80a61000 x22: 000000000000000e x21: 1ffff0000b2f6e28
> x20: 0000000000040dc0 x19: ffffdf670d4a6fe0 x18: ffffdf66fa017d1c
> x17: ffffdf66f42f8348 x16: 1fffe1003cd1a7b3 x15: 000000000000001a
> x14: 1fffe1003cd1a7a6 x13: 0000000000000004 x12: ffff70000b2f6e05
> x11: 1ffff0000b2f6e04 x10: 00000000f204f1f1 x9 : 000000000000f204
> x8 : dfff800000000000 x7 : 00000000f3000000 x6 : 00000000f3f3f3f3
> x5 : ffff70000b2f6e28 x4 : ffff0801e68d34c0 x3 : 0000000000000000
> x2 : 0000000000000000 x1 : 0000000000000001 x0 : 0000000000040dc0
> Call trace:
> __alloc_pages
> alloc_pages
> kmalloc_order
> kmalloc_order_trace
> __kmalloc
> __regset_get
> regset_get_alloc
> fill_thread_core_info
> fill_note_info
> elf_core_dump
> do_coredump
> get_signal
> do_signal
> do_notify_resume
> el0_svc
> el0t_64_sync_handler
> el0t_64_sync
> irq event stamp: 3614
> hardirqs last enabled at (3613): _raw_spin_unlock_irqrestore
> hardirqs last disabled at (3614): el1_dbg
> softirqs last enabled at (2988): fpsimd_preserve_current_state
> softirqs last disabled at (2986): fpsimd_preserve_current_state

I got an email this morning reporting a warning with the same call trace:
https://lore.kernel.org/linux-mm/CA+G9fYveMF-NU-rvrsbaora2g2QWxrkF7AWViuDrJyN9mNScJg@mail.gmail.com/

The email says the warning appeared from next-20220427, but my
patchset was in linux-next since next-20220426. In addition,
my patches do not touch any function in the call trace. I assume
this warning is not related to my patchset. But let me know
if my patchset is related.

Thanks.

--
Best Regards,
Yan, Zi


Attachments:
signature.asc (871.00 B)
OpenPGP digital signature

2022-05-10 01:39:29

by Andrew Morton

[permalink] [raw]
Subject: Re: [PATCH v11 0/6] Use pageblock_order for cma and alloc_contig_range alignment.

On Mon, 25 Apr 2022 10:31:12 -0400 Zi Yan <[email protected]> wrote:

> This patchset tries to remove the MAX_ORDER-1 alignment requirement for CMA
> and alloc_contig_range(). It prepares for my upcoming changes to make
> MAX_ORDER adjustable at boot time[1].

I'm thinking this looks ready to be merged into mm-stable later this week, for
the 5.19-rc1 merge window.

I believe the build error at
https://lkml.kernel.org/r/CA+G9fYveMF-NU-rvrsbaora2g2QWxrkF7AWViuDrJyN9mNScJg@mail.gmail.com
was addressed in ARM?

I have one -fix to be squashed,
https://git.kernel.org/pub/scm/linux/kernel/git/akpm/25-new.git/tree/patches/mm-make-alloc_contig_range-work-at-pageblock-granularity-fix.patch


2022-05-10 11:08:12

by Zi Yan

[permalink] [raw]
Subject: Re: [PATCH v11 0/6] Use pageblock_order for cma and alloc_contig_range alignment.

On 9 May 2022, at 21:03, Andrew Morton wrote:

> On Mon, 25 Apr 2022 10:31:12 -0400 Zi Yan <[email protected]> wrote:
>
>> This patchset tries to remove the MAX_ORDER-1 alignment requirement for CMA
>> and alloc_contig_range(). It prepares for my upcoming changes to make
>> MAX_ORDER adjustable at boot time[1].
>
> I'm thinking this looks ready to be merged into mm-stable later this week, for
> the 5.19-rc1 merge window.
>
> I believe the build error at
> https://lkml.kernel.org/r/CA+G9fYveMF-NU-rvrsbaora2g2QWxrkF7AWViuDrJyN9mNScJg@mail.gmail.com
> was addressed in ARM?

Right. The warning is caused by CONFIG_ARM64_SME=y not this patchset,
see https://lore.kernel.org/all/YnGrbEt3oBBTly7u@qian/.

>
> I have one -fix to be squashed,
> https://git.kernel.org/pub/scm/linux/kernel/git/akpm/25-new.git/tree/patches/mm-make-alloc_contig_range-work-at-pageblock-granularity-fix.patch

Yes. Thanks.

--
Best Regards,
Yan, Zi


Attachments:
signature.asc (871.00 B)
OpenPGP digital signature

2022-05-20 11:25:06

by Zi Yan

[permalink] [raw]
Subject: Re: [PATCH v11 0/6] Use pageblock_order for cma and alloc_contig_range alignment.

On 19 May 2022, at 17:35, Zi Yan wrote:

> On 19 May 2022, at 16:57, Qian Cai wrote:
>
>> On Thu, Apr 28, 2022 at 08:39:06AM -0400, Zi Yan wrote:
>>> How about the one attached? I can apply it to next-20220428. Let me know
>>> if you are using a different branch. Thanks.
>>
>> Zi, it turns out that the endless loop in isolate_single_pageblock() can
>> still be reproduced on today's linux-next tree by running the reproducer a
>> few times. With this debug patch applied, it keeps printing the same
>> values.
>>
>> --- a/mm/page_isolation.c
>> +++ b/mm/page_isolation.c
>> @@ -399,6 +399,8 @@ static int isolate_single_pageblock(unsigned long boundary_pfn, gfp_t gfp_flags,
>> };
>> INIT_LIST_HEAD(&cc.migratepages);
>>
>> + printk_ratelimited("KK stucked pfn=%lu head_pfn=%lu nr_pages=%lu boundary_pfn=%lu\n", pfn, head_pfn, nr_pages, boundary_pfn);
>> ret = __alloc_contig_migrate_range(&cc, head_pfn,
>> head_pfn + nr_pages);
>>
>> isolate_single_pageblock: 179 callbacks suppressed
>> KK stucked pfn=2151120384 head_pfn=2151120384 nr_pages=512 boundary_pfn=2151120896
>> KK stucked pfn=2151120384 head_pfn=2151120384 nr_pages=512 boundary_pfn=2151120896
>> KK stucked pfn=2151120384 head_pfn=2151120384 nr_pages=512 boundary_pfn=2151120896
>> KK stucked pfn=2151120384 head_pfn=2151120384 nr_pages=512 boundary_pfn=2151120896
>> KK stucked pfn=2151120384 head_pfn=2151120384 nr_pages=512 boundary_pfn=2151120896
>> KK stucked pfn=2151120384 head_pfn=2151120384 nr_pages=512 boundary_pfn=2151120896
>> KK stucked pfn=2151120384 head_pfn=2151120384 nr_pages=512 boundary_pfn=2151120896
>> KK stucked pfn=2151120384 head_pfn=2151120384 nr_pages=512 boundary_pfn=2151120896
>> KK stucked pfn=2151120384 head_pfn=2151120384 nr_pages=512 boundary_pfn=2151120896
>> KK stucked pfn=2151120384 head_pfn=2151120384 nr_pages=512 boundary_pfn=2151120896
>
> Hi Qian,
>
> Thanks for your testing.
>
> Do you have a complete reproducer? From your printout, it is clear that a 512-page compound
> page caused the infinite loop, because the page was not migrated and the code kept
> retrying. But __alloc_contig_migrate_range() is supposed to return non-zero to tell the
> code the page cannot be migrated and the code will goto failed without retrying. It will be
> great you can share what exactly has run after boot, so that I can reproduce locally to
> identify what makes __alloc_contig_migrate_range() return 0 without migrating the page.
>
> Can you also try the patch below to see if it fixes the infinite loop?

I also have an off-by-one error in the code. The error caused unnecessary effort of
trying to migrate some pages. Your endless loop case seems to be caused by it.
Can you actually try the patch below? Thanks.

diff --git a/mm/page_isolation.c b/mm/page_isolation.c
index b3f074d1682e..5c8099bb822f 100644
--- a/mm/page_isolation.c
+++ b/mm/page_isolation.c
@@ -374,7 +374,7 @@ static int isolate_single_pageblock(unsigned long boundary_pfn, gfp_t gfp_flags,
struct page *head = compound_head(page);
unsigned long head_pfn = page_to_pfn(head);

- if (head_pfn + nr_pages < boundary_pfn) {
+ if (head_pfn + nr_pages <= boundary_pfn) {
pfn = head_pfn + nr_pages;
continue;
}
@@ -417,10 +417,8 @@ static int isolate_single_pageblock(unsigned long boundary_pfn, gfp_t gfp_flags,
order = 0;
outer_pfn = pfn;
while (!PageBuddy(pfn_to_page(outer_pfn))) {
- if (++order >= MAX_ORDER) {
- outer_pfn = pfn;
- break;
- }
+ if (++order >= MAX_ORDER)
+ goto failed;
outer_pfn &= ~0UL << order;
}
pfn = outer_pfn;

--
Best Regards,
Yan, Zi


Attachments:
signature.asc (871.00 B)
OpenPGP digital signature

2022-05-20 23:33:38

by Qian Cai

[permalink] [raw]
Subject: Re: [PATCH v11 0/6] Use pageblock_order for cma and alloc_contig_range alignment.

On Thu, Apr 28, 2022 at 08:39:06AM -0400, Zi Yan wrote:
> How about the one attached? I can apply it to next-20220428. Let me know
> if you are using a different branch. Thanks.

Zi, it turns out that the endless loop in isolate_single_pageblock() can
still be reproduced on today's linux-next tree by running the reproducer a
few times. With this debug patch applied, it keeps printing the same
values.

--- a/mm/page_isolation.c
+++ b/mm/page_isolation.c
@@ -399,6 +399,8 @@ static int isolate_single_pageblock(unsigned long boundary_pfn, gfp_t gfp_flags,
};
INIT_LIST_HEAD(&cc.migratepages);

+ printk_ratelimited("KK stucked pfn=%lu head_pfn=%lu nr_pages=%lu boundary_pfn=%lu\n", pfn, head_pfn, nr_pages, boundary_pfn);
ret = __alloc_contig_migrate_range(&cc, head_pfn,
head_pfn + nr_pages);

isolate_single_pageblock: 179 callbacks suppressed
KK stucked pfn=2151120384 head_pfn=2151120384 nr_pages=512 boundary_pfn=2151120896
KK stucked pfn=2151120384 head_pfn=2151120384 nr_pages=512 boundary_pfn=2151120896
KK stucked pfn=2151120384 head_pfn=2151120384 nr_pages=512 boundary_pfn=2151120896
KK stucked pfn=2151120384 head_pfn=2151120384 nr_pages=512 boundary_pfn=2151120896
KK stucked pfn=2151120384 head_pfn=2151120384 nr_pages=512 boundary_pfn=2151120896
KK stucked pfn=2151120384 head_pfn=2151120384 nr_pages=512 boundary_pfn=2151120896
KK stucked pfn=2151120384 head_pfn=2151120384 nr_pages=512 boundary_pfn=2151120896
KK stucked pfn=2151120384 head_pfn=2151120384 nr_pages=512 boundary_pfn=2151120896
KK stucked pfn=2151120384 head_pfn=2151120384 nr_pages=512 boundary_pfn=2151120896
KK stucked pfn=2151120384 head_pfn=2151120384 nr_pages=512 boundary_pfn=2151120896

2022-05-21 14:32:28

by Zi Yan

[permalink] [raw]
Subject: Re: [PATCH v11 0/6] Use pageblock_order for cma and alloc_contig_range alignment.

On 20 May 2022, at 15:41, Qian Cai wrote:

> On Fri, May 20, 2022 at 10:13:51AM -0400, Zi Yan wrote:
>> Also, do you mind providing the page dump of the 512-page compound page? I would like
>> to know what page caused the issue.
>
> page last allocated via order 9, migratetype Movable, gfp_mask 0x3c24ca(GFP_TRANSHUGE|__GFP_THISNODE), pid 831, tgid 831 (khugepaged), ts 3899865924520, free_ts 3821953009040
> post_alloc_hook
> get_page_from_freelist
> __alloc_pages
> khugepaged_alloc_page
> collapse_huge_page
> khugepaged_scan_pmd
> khugepaged_scan_mm_slot
> khugepaged
> kthread
> ret_from_fork
> page last free stack trace:
> free_pcp_prepare
> free_unref_page
> free_compound_page
> free_transhuge_page
> __put_compound_page
> release_pages
> free_pages_and_swap_cache
> tlb_batch_pages_flush
> tlb_finish_mmu
> exit_mmap
> __mmput
> mmput
> exit_mm
> do_exit
> do_group_exit
> __arm64_sys_exit_group

Do you have the page information like refcount, map count, mapping, index, and
page flags? That would be more helpful. Thanks.

I cannot reproduce it locally after hundreds of iterations of flip_mem.py on my
x86_64 VM and bare metal.

What ARM machine are you using? I wonder if I am able to get one locally.

Thanks.

--
Best Regards,
Yan, Zi


Attachments:
signature.asc (871.00 B)
OpenPGP digital signature

2022-05-21 15:54:48

by Zi Yan

[permalink] [raw]
Subject: Re: [PATCH v11 0/6] Use pageblock_order for cma and alloc_contig_range alignment.

On 20 May 2022, at 9:43, Zi Yan wrote:

> On 20 May 2022, at 7:30, Qian Cai wrote:
>
>> On Thu, May 19, 2022 at 05:35:15PM -0400, Zi Yan wrote:
>>> Do you have a complete reproducer? From your printout, it is clear that a 512-page compound
>>> page caused the infinite loop, because the page was not migrated and the code kept
>>> retrying. But __alloc_contig_migrate_range() is supposed to return non-zero to tell the
>>> code the page cannot be migrated and the code will goto failed without retrying. It will be
>>> great you can share what exactly has run after boot, so that I can reproduce locally to
>>> identify what makes __alloc_contig_migrate_range() return 0 without migrating the page.
>>
>> The reproducer is just to run the same script I shared with you previously
>> multiple times instead. It is still quite reproducible here as it usually
>> happens within a hour.
>>
>> $ for i in `seq 1 100`; do ./flip_mem.py; done

Also, do you mind providing the page dump of the 512-page compound page? I would like
to know what page caused the issue.

Thanks.

>>
>>> Can you also try the patch below to see if it fixes the infinite loop?
>>>
>>> diff --git a/mm/page_isolation.c b/mm/page_isolation.c
>>> index b3f074d1682e..abde1877bbcb 100644
>>> --- a/mm/page_isolation.c
>>> +++ b/mm/page_isolation.c
>>> @@ -417,10 +417,9 @@ static int isolate_single_pageblock(unsigned long boundary_pfn, gfp_t gfp_flags,
>>> order = 0;
>>> outer_pfn = pfn;
>>> while (!PageBuddy(pfn_to_page(outer_pfn))) {
>>> - if (++order >= MAX_ORDER) {
>>> - outer_pfn = pfn;
>>> - break;
>>> - }
>>> + /* abort if the free page cannot be found */
>>> + if (++order >= MAX_ORDER)
>>> + goto failed;
>>> outer_pfn &= ~0UL << order;
>>> }
>>> pfn = outer_pfn;
>>>
>>
>> Can you explain a bit how this patch is the right thing to do here? I am a
>> little bit worry about shooting into the dark. Otherwise, I'll be running
>> the off-by-one part over the weekend to see if that helps.
>
> The code kept retrying to migrate a 512-page compound page, so it seems to me
> that __alloc_contig_migrate_range() did not migrate the page but returned
> 0 every time, otherwise, if (ret) goto failed; would bail out of the loop
> already. The original code above assumed a free page can always be found after
> __alloc_contig_migrate_range(), so it will retry if no free page is found.
> But that assumption is not true from your infinite loop result, the new
> code quits retrying when no free page can be found.
>
> I will dig into it deeper to make sure it is the correct fix. I will
> update you when I am done.
>
> Thanks.
>
> --
> Best Regards,
> Yan, Zi


--
Best Regards,
Yan, Zi


Attachments:
signature.asc (871.00 B)
OpenPGP digital signature

2022-05-21 18:40:50

by Qian Cai

[permalink] [raw]
Subject: Re: [PATCH v11 0/6] Use pageblock_order for cma and alloc_contig_range alignment.

On Fri, May 20, 2022 at 10:13:51AM -0400, Zi Yan wrote:
> Also, do you mind providing the page dump of the 512-page compound page? I would like
> to know what page caused the issue.

page last allocated via order 9, migratetype Movable, gfp_mask 0x3c24ca(GFP_TRANSHUGE|__GFP_THISNODE), pid 831, tgid 831 (khugepaged), ts 3899865924520, free_ts 3821953009040
post_alloc_hook
get_page_from_freelist
__alloc_pages
khugepaged_alloc_page
collapse_huge_page
khugepaged_scan_pmd
khugepaged_scan_mm_slot
khugepaged
kthread
ret_from_fork
page last free stack trace:
free_pcp_prepare
free_unref_page
free_compound_page
free_transhuge_page
__put_compound_page
release_pages
free_pages_and_swap_cache
tlb_batch_pages_flush
tlb_finish_mmu
exit_mmap
__mmput
mmput
exit_mm
do_exit
do_group_exit
__arm64_sys_exit_group

2022-05-22 00:02:11

by Zi Yan

[permalink] [raw]
Subject: Re: [PATCH v11 0/6] Use pageblock_order for cma and alloc_contig_range alignment.

On 20 May 2022, at 7:30, Qian Cai wrote:

> On Thu, May 19, 2022 at 05:35:15PM -0400, Zi Yan wrote:
>> Do you have a complete reproducer? From your printout, it is clear that a 512-page compound
>> page caused the infinite loop, because the page was not migrated and the code kept
>> retrying. But __alloc_contig_migrate_range() is supposed to return non-zero to tell the
>> code the page cannot be migrated and the code will goto failed without retrying. It will be
>> great you can share what exactly has run after boot, so that I can reproduce locally to
>> identify what makes __alloc_contig_migrate_range() return 0 without migrating the page.
>
> The reproducer is just to run the same script I shared with you previously
> multiple times instead. It is still quite reproducible here as it usually
> happens within a hour.
>
> $ for i in `seq 1 100`; do ./flip_mem.py; done
>
>> Can you also try the patch below to see if it fixes the infinite loop?
>>
>> diff --git a/mm/page_isolation.c b/mm/page_isolation.c
>> index b3f074d1682e..abde1877bbcb 100644
>> --- a/mm/page_isolation.c
>> +++ b/mm/page_isolation.c
>> @@ -417,10 +417,9 @@ static int isolate_single_pageblock(unsigned long boundary_pfn, gfp_t gfp_flags,
>> order = 0;
>> outer_pfn = pfn;
>> while (!PageBuddy(pfn_to_page(outer_pfn))) {
>> - if (++order >= MAX_ORDER) {
>> - outer_pfn = pfn;
>> - break;
>> - }
>> + /* abort if the free page cannot be found */
>> + if (++order >= MAX_ORDER)
>> + goto failed;
>> outer_pfn &= ~0UL << order;
>> }
>> pfn = outer_pfn;
>>
>
> Can you explain a bit how this patch is the right thing to do here? I am a
> little bit worry about shooting into the dark. Otherwise, I'll be running
> the off-by-one part over the weekend to see if that helps.

The code kept retrying to migrate a 512-page compound page, so it seems to me
that __alloc_contig_migrate_range() did not migrate the page but returned
0 every time, otherwise, if (ret) goto failed; would bail out of the loop
already. The original code above assumed a free page can always be found after
__alloc_contig_migrate_range(), so it will retry if no free page is found.
But that assumption is not true from your infinite loop result, the new
code quits retrying when no free page can be found.

I will dig into it deeper to make sure it is the correct fix. I will
update you when I am done.

Thanks.

--
Best Regards,
Yan, Zi


Attachments:
signature.asc (871.00 B)
OpenPGP digital signature

2022-05-23 05:50:48

by Qian Cai

[permalink] [raw]
Subject: Re: [PATCH v11 0/6] Use pageblock_order for cma and alloc_contig_range alignment.

On Fri, May 20, 2022 at 05:56:52PM -0400, Zi Yan wrote:
> Do you have the page information like refcount, map count, mapping, index, and
> page flags? That would be more helpful. Thanks.

page:fffffc200c7f8000 refcount:393 mapcount:1 mapping:0000000000000000 index:0xffffbb800 pfn:0x8039fe00
head:fffffc200c7f8000 order:9 compound_mapcount:0 compound_pincount:0
memcg:ffff40026005a000
anon flags: 0xbfffc000009001c(uptodate|dirty|lru|head|swapbacked|node=0|zone=2|lastcpupid=0xffff)
raw: 0bfffc000009001c fffffc2007b74048 fffffc2009c087c8 ffff08038dab9189
raw: 0000000ffffbb800 0000000000000000 0000018900000000 ffff40026005a000

> I cannot reproduce it locally after hundreds of iterations of flip_mem.py on my
> x86_64 VM and bare metal.
>
> What ARM machine are you using? I wonder if I am able to get one locally.

Ampere Altra.

2022-05-23 06:51:40

by Zi Yan

[permalink] [raw]
Subject: Re: [PATCH v11 0/6] Use pageblock_order for cma and alloc_contig_range alignment.

On 19 May 2022, at 16:57, Qian Cai wrote:

> On Thu, Apr 28, 2022 at 08:39:06AM -0400, Zi Yan wrote:
>> How about the one attached? I can apply it to next-20220428. Let me know
>> if you are using a different branch. Thanks.
>
> Zi, it turns out that the endless loop in isolate_single_pageblock() can
> still be reproduced on today's linux-next tree by running the reproducer a
> few times. With this debug patch applied, it keeps printing the same
> values.
>
> --- a/mm/page_isolation.c
> +++ b/mm/page_isolation.c
> @@ -399,6 +399,8 @@ static int isolate_single_pageblock(unsigned long boundary_pfn, gfp_t gfp_flags,
> };
> INIT_LIST_HEAD(&cc.migratepages);
>
> + printk_ratelimited("KK stucked pfn=%lu head_pfn=%lu nr_pages=%lu boundary_pfn=%lu\n", pfn, head_pfn, nr_pages, boundary_pfn);
> ret = __alloc_contig_migrate_range(&cc, head_pfn,
> head_pfn + nr_pages);
>
> isolate_single_pageblock: 179 callbacks suppressed
> KK stucked pfn=2151120384 head_pfn=2151120384 nr_pages=512 boundary_pfn=2151120896
> KK stucked pfn=2151120384 head_pfn=2151120384 nr_pages=512 boundary_pfn=2151120896
> KK stucked pfn=2151120384 head_pfn=2151120384 nr_pages=512 boundary_pfn=2151120896
> KK stucked pfn=2151120384 head_pfn=2151120384 nr_pages=512 boundary_pfn=2151120896
> KK stucked pfn=2151120384 head_pfn=2151120384 nr_pages=512 boundary_pfn=2151120896
> KK stucked pfn=2151120384 head_pfn=2151120384 nr_pages=512 boundary_pfn=2151120896
> KK stucked pfn=2151120384 head_pfn=2151120384 nr_pages=512 boundary_pfn=2151120896
> KK stucked pfn=2151120384 head_pfn=2151120384 nr_pages=512 boundary_pfn=2151120896
> KK stucked pfn=2151120384 head_pfn=2151120384 nr_pages=512 boundary_pfn=2151120896
> KK stucked pfn=2151120384 head_pfn=2151120384 nr_pages=512 boundary_pfn=2151120896

Hi Qian,

Thanks for your testing.

Do you have a complete reproducer? From your printout, it is clear that a 512-page compound
page caused the infinite loop, because the page was not migrated and the code kept
retrying. But __alloc_contig_migrate_range() is supposed to return non-zero to tell the
code the page cannot be migrated and the code will goto failed without retrying. It will be
great you can share what exactly has run after boot, so that I can reproduce locally to
identify what makes __alloc_contig_migrate_range() return 0 without migrating the page.

Can you also try the patch below to see if it fixes the infinite loop?

diff --git a/mm/page_isolation.c b/mm/page_isolation.c
index b3f074d1682e..abde1877bbcb 100644
--- a/mm/page_isolation.c
+++ b/mm/page_isolation.c
@@ -417,10 +417,9 @@ static int isolate_single_pageblock(unsigned long boundary_pfn, gfp_t gfp_flags,
order = 0;
outer_pfn = pfn;
while (!PageBuddy(pfn_to_page(outer_pfn))) {
- if (++order >= MAX_ORDER) {
- outer_pfn = pfn;
- break;
- }
+ /* abort if the free page cannot be found */
+ if (++order >= MAX_ORDER)
+ goto failed;
outer_pfn &= ~0UL << order;
}
pfn = outer_pfn;

--
Best Regards,
Yan, Zi


Attachments:
signature.asc (871.00 B)
OpenPGP digital signature

2022-05-23 07:04:41

by Qian Cai

[permalink] [raw]
Subject: Re: [PATCH v11 0/6] Use pageblock_order for cma and alloc_contig_range alignment.

On Thu, May 19, 2022 at 05:35:15PM -0400, Zi Yan wrote:
> Do you have a complete reproducer? From your printout, it is clear that a 512-page compound
> page caused the infinite loop, because the page was not migrated and the code kept
> retrying. But __alloc_contig_migrate_range() is supposed to return non-zero to tell the
> code the page cannot be migrated and the code will goto failed without retrying. It will be
> great you can share what exactly has run after boot, so that I can reproduce locally to
> identify what makes __alloc_contig_migrate_range() return 0 without migrating the page.

The reproducer is just to run the same script I shared with you previously
multiple times instead. It is still quite reproducible here as it usually
happens within a hour.

$ for i in `seq 1 100`; do ./flip_mem.py; done

> Can you also try the patch below to see if it fixes the infinite loop?
>
> diff --git a/mm/page_isolation.c b/mm/page_isolation.c
> index b3f074d1682e..abde1877bbcb 100644
> --- a/mm/page_isolation.c
> +++ b/mm/page_isolation.c
> @@ -417,10 +417,9 @@ static int isolate_single_pageblock(unsigned long boundary_pfn, gfp_t gfp_flags,
> order = 0;
> outer_pfn = pfn;
> while (!PageBuddy(pfn_to_page(outer_pfn))) {
> - if (++order >= MAX_ORDER) {
> - outer_pfn = pfn;
> - break;
> - }
> + /* abort if the free page cannot be found */
> + if (++order >= MAX_ORDER)
> + goto failed;
> outer_pfn &= ~0UL << order;
> }
> pfn = outer_pfn;
>

Can you explain a bit how this patch is the right thing to do here? I am a
little bit worry about shooting into the dark. Otherwise, I'll be running
the off-by-one part over the weekend to see if that helps.

2022-05-23 07:25:14

by Zi Yan

[permalink] [raw]
Subject: Re: [PATCH v11 0/6] Use pageblock_order for cma and alloc_contig_range alignment.

On 20 May 2022, at 19:41, Qian Cai wrote:

> On Fri, May 20, 2022 at 05:56:52PM -0400, Zi Yan wrote:
>> Do you have the page information like refcount, map count, mapping, index, and
>> page flags? That would be more helpful. Thanks.
>
> page:fffffc200c7f8000 refcount:393 mapcount:1 mapping:0000000000000000 index:0xffffbb800 pfn:0x8039fe00
> head:fffffc200c7f8000 order:9 compound_mapcount:0 compound_pincount:0
> memcg:ffff40026005a000
> anon flags: 0xbfffc000009001c(uptodate|dirty|lru|head|swapbacked|node=0|zone=2|lastcpupid=0xffff)
> raw: 0bfffc000009001c fffffc2007b74048 fffffc2009c087c8 ffff08038dab9189
> raw: 0000000ffffbb800 0000000000000000 0000018900000000 ffff40026005a000
>

This is a PTE-mapped THP, unless <393 subpages are mapped, meaning extra refcount is present,
the page should be migratable. Even if it is not migratible due to the extra pin,
__alloc_contig_migrate_range() will return non-zero and bails out the code.
No idea why it caused the infinite loop.

>> I cannot reproduce it locally after hundreds of iterations of flip_mem.py on my
>> x86_64 VM and bare metal.
>>
>> What ARM machine are you using? I wonder if I am able to get one locally.
>
> Ampere Altra.

Sorry, I have no access to such a machine right now and cannot afford to buy one.

Can you try the patch below on top of linux-next to see if it fixes the infinite loop issue?
Thanks.

1. split_free_page() change is irrelevant but to make the code more robust.
2. using set_migratetype_isolate() in isolate_single_pageblock() is to properly mark the pageblock
MIGRATE_ISOLATE.
3. setting to-be-migrated page's pageblock to MIGRATE_ISOLATE is to avoid a possible race
that another thread might take the free page after migration.
4. off-by-one fix and no retry if free page is not found after migration like I added before.

diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index 4dcfa0ceca45..ad8f73b00466 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -1122,13 +1122,16 @@ void split_free_page(struct page *free_page,
unsigned long flags;
int free_page_order;

+ if (split_pfn_offset == 0)
+ return;
+
spin_lock_irqsave(&zone->lock, flags);
del_page_from_free_list(free_page, zone, order);
for (pfn = free_page_pfn;
pfn < free_page_pfn + (1UL << order);) {
int mt = get_pfnblock_migratetype(pfn_to_page(pfn), pfn);

- free_page_order = ffs(split_pfn_offset) - 1;
+ free_page_order = min(pfn ? __ffs(pfn) : order, __fls(split_pfn_offset));
__free_one_page(pfn_to_page(pfn), pfn, zone, free_page_order,
mt, FPI_NONE);
pfn += 1UL << free_page_order;
diff --git a/mm/page_isolation.c b/mm/page_isolation.c
index b3f074d1682e..706915c9a380 100644
--- a/mm/page_isolation.c
+++ b/mm/page_isolation.c
@@ -283,6 +283,7 @@ __first_valid_page(unsigned long pfn, unsigned long nr_pages)
* isolate_single_pageblock() -- tries to isolate a pageblock that might be
* within a free or in-use page.
* @boundary_pfn: pageblock-aligned pfn that a page might cross
+ * @flags: isolation flags
* @gfp_flags: GFP flags used for migrating pages
* @isolate_before: isolate the pageblock before the boundary_pfn
*
@@ -298,14 +299,15 @@ __first_valid_page(unsigned long pfn, unsigned long nr_pages)
* either. The function handles this by splitting the free page or migrating
* the in-use page then splitting the free page.
*/
-static int isolate_single_pageblock(unsigned long boundary_pfn, gfp_t gfp_flags,
- bool isolate_before)
+static int isolate_single_pageblock(unsigned long boundary_pfn, int flags,
+ gfp_t gfp_flags, bool isolate_before)
{
unsigned char saved_mt;
unsigned long start_pfn;
unsigned long isolate_pageblock;
unsigned long pfn;
struct zone *zone;
+ int ret;

VM_BUG_ON(!IS_ALIGNED(boundary_pfn, pageblock_nr_pages));

@@ -325,7 +327,11 @@ static int isolate_single_pageblock(unsigned long boundary_pfn, gfp_t gfp_flags,
zone->zone_start_pfn);

saved_mt = get_pageblock_migratetype(pfn_to_page(isolate_pageblock));
- set_pageblock_migratetype(pfn_to_page(isolate_pageblock), MIGRATE_ISOLATE);
+ ret = set_migratetype_isolate(pfn_to_page(isolate_pageblock), saved_mt, flags,
+ isolate_pageblock, isolate_pageblock + pageblock_nr_pages);
+
+ if (ret)
+ return ret;

/*
* Bail out early when the to-be-isolated pageblock does not form
@@ -374,7 +380,7 @@ static int isolate_single_pageblock(unsigned long boundary_pfn, gfp_t gfp_flags,
struct page *head = compound_head(page);
unsigned long head_pfn = page_to_pfn(head);

- if (head_pfn + nr_pages < boundary_pfn) {
+ if (head_pfn + nr_pages <= boundary_pfn) {
pfn = head_pfn + nr_pages;
continue;
}
@@ -386,7 +392,8 @@ static int isolate_single_pageblock(unsigned long boundary_pfn, gfp_t gfp_flags,
if (PageHuge(page) || PageLRU(page) || __PageMovable(page)) {
int order;
unsigned long outer_pfn;
- int ret;
+ int page_mt = get_pageblock_migratetype(page);
+ bool isolate_page = !is_migrate_isolate_page(page);
struct compact_control cc = {
.nr_migratepages = 0,
.order = -1,
@@ -399,9 +406,31 @@ static int isolate_single_pageblock(unsigned long boundary_pfn, gfp_t gfp_flags,
};
INIT_LIST_HEAD(&cc.migratepages);

+ /*
+ * XXX: mark the page as MIGRATE_ISOLATE so that
+ * no one else can grab the freed page after migration.
+ * Ideally, the page should be freed as two separate
+ * pages to be added into separate migratetype free
+ * lists.
+ */
+ if (isolate_page) {
+ ret = set_migratetype_isolate(page, page_mt,
+ flags, head_pfn, boundary_pfn - 1);
+ if (ret)
+ goto failed;
+ }
+
ret = __alloc_contig_migrate_range(&cc, head_pfn,
head_pfn + nr_pages);

+ /*
+ * restore the page's migratetype so that it can
+ * be split into separate migratetype free lists
+ * later.
+ */
+ if (isolate_page)
+ unset_migratetype_isolate(page, page_mt);
+
if (ret)
goto failed;
/*
@@ -417,10 +446,9 @@ static int isolate_single_pageblock(unsigned long boundary_pfn, gfp_t gfp_flags,
order = 0;
outer_pfn = pfn;
while (!PageBuddy(pfn_to_page(outer_pfn))) {
- if (++order >= MAX_ORDER) {
- outer_pfn = pfn;
- break;
- }
+ /* stop if we cannot find the free page */
+ if (++order >= MAX_ORDER)
+ goto failed;
outer_pfn &= ~0UL << order;
}
pfn = outer_pfn;
@@ -435,7 +463,7 @@ static int isolate_single_pageblock(unsigned long boundary_pfn, gfp_t gfp_flags,
return 0;
failed:
/* restore the original migratetype */
- set_pageblock_migratetype(pfn_to_page(isolate_pageblock), saved_mt);
+ unset_migratetype_isolate(pfn_to_page(isolate_pageblock), saved_mt);
return -EBUSY;
}

@@ -496,12 +524,12 @@ int start_isolate_page_range(unsigned long start_pfn, unsigned long end_pfn,
int ret;

/* isolate [isolate_start, isolate_start + pageblock_nr_pages) pageblock */
- ret = isolate_single_pageblock(isolate_start, gfp_flags, false);
+ ret = isolate_single_pageblock(isolate_start, flags, gfp_flags, false);
if (ret)
return ret;

/* isolate [isolate_end - pageblock_nr_pages, isolate_end) pageblock */
- ret = isolate_single_pageblock(isolate_end, gfp_flags, true);
+ ret = isolate_single_pageblock(isolate_end, flags, gfp_flags, true);
if (ret) {
unset_migratetype_isolate(pfn_to_page(isolate_start), migratetype);
return ret;


--
Best Regards,
Yan, Zi


Attachments:
signature.asc (871.00 B)
OpenPGP digital signature

2022-05-23 08:19:24

by Zi Yan

[permalink] [raw]
Subject: Re: [PATCH v11 0/6] Use pageblock_order for cma and alloc_contig_range alignment.

On 22 May 2022, at 12:54, Zi Yan wrote:

> On 20 May 2022, at 19:41, Qian Cai wrote:
>
>> On Fri, May 20, 2022 at 05:56:52PM -0400, Zi Yan wrote:
>>> Do you have the page information like refcount, map count, mapping, index, and
>>> page flags? That would be more helpful. Thanks.
>>
>> page:fffffc200c7f8000 refcount:393 mapcount:1 mapping:0000000000000000 index:0xffffbb800 pfn:0x8039fe00
>> head:fffffc200c7f8000 order:9 compound_mapcount:0 compound_pincount:0
>> memcg:ffff40026005a000
>> anon flags: 0xbfffc000009001c(uptodate|dirty|lru|head|swapbacked|node=0|zone=2|lastcpupid=0xffff)
>> raw: 0bfffc000009001c fffffc2007b74048 fffffc2009c087c8 ffff08038dab9189
>> raw: 0000000ffffbb800 0000000000000000 0000018900000000 ffff40026005a000

OK. I replicated two scenarios, which can have the above page dump:
1. a PTE-mapped THP with 393 subpages mapped without any extra pin,
2. a PTE-mapped THP with 392 subpages mapped with an extra pin on the first subpage.

For scenario 1, there is no infinite looping on next-20220519 and next-20220520.

For scenario 2, an infinite loop happens on next-20220519, next-20220520, and next-20220520
with my fixup patch from another email, when the memory block, in which the page resides,
is being offlined. However, after reverting all my patches, the infinite loop remains.

So it looks to me that having an infinite loop during memory offline is not a regression
based on the experiments I have done. David Hildenbranch can correct me if I am wrong.
A better issue description, other than infinite loop during memory offlining, and a
better reproducer are needed for me to identify potential bugs in my code and fix them.

Of course, my fixup patch should be applied anyway.

Thanks for your testing.


--
Best Regards,
Yan, Zi


Attachments:
signature.asc (871.00 B)
OpenPGP digital signature

2022-05-24 20:33:21

by Qian Cai

[permalink] [raw]
Subject: Re: [PATCH v11 0/6] Use pageblock_order for cma and alloc_contig_range alignment.

On Sun, May 22, 2022 at 12:54:04PM -0400, Zi Yan wrote:
> Can you try the patch below on top of linux-next to see if it fixes the infinite loop issue?
> Thanks.
>
> 1. split_free_page() change is irrelevant but to make the code more robust.
> 2. using set_migratetype_isolate() in isolate_single_pageblock() is to properly mark the pageblock
> MIGRATE_ISOLATE.
> 3. setting to-be-migrated page's pageblock to MIGRATE_ISOLATE is to avoid a possible race
> that another thread might take the free page after migration.
> 4. off-by-one fix and no retry if free page is not found after migration like I added before.

Cool. I'll be running it this week and report back next week.