Re: [PATCH v7 06/21] mm/vma: Change munmap to use vma_munmap_struct() for accounting and surrounding vmas

[Bert Karwatzki <spasswolf@web.de>]

Am Donnerstag, dem 22.08.2024 um 15:25 -0400 schrieb Liam R. Howlett:
> From: "Liam R. Howlett" <Liam.Howlett@Oracle.com>
>
> Clean up the code by changing the munmap operation to use a structure
> for the accounting and munmap variables.
>
> Since remove_mt() is only called in one location and the contents will
> be reduced to almost nothing.  The remains of the function can be added
> to vms_complete_munmap_vmas().
>
> Signed-off-by: Liam R. Howlett <Liam.Howlett@oracle.com>
> Reviewed-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
> Reviewed-by: Suren Baghdasaryan <surenb@google.com>
> ---
>  mm/vma.c | 83 +++++++++++++++++++++++++++++---------------------------
>  mm/vma.h |  6 ++++
>  2 files changed, 49 insertions(+), 40 deletions(-)
>
> diff --git a/mm/vma.c b/mm/vma.c
> index e1aee43a3dc4..58604fe3bd03 100644
> --- a/mm/vma.c
> +++ b/mm/vma.c
> @@ -103,7 +103,8 @@ static inline void init_vma_munmap(struct vma_munmap_struct *vms,
>  	vms->unlock = unlock;
>  	vms->uf = uf;
>  	vms->vma_count = 0;
> -	vms->nr_pages = vms->locked_vm = 0;
> +	vms->nr_pages = vms->locked_vm = vms->nr_accounted = 0;
> +	vms->exec_vm = vms->stack_vm = vms->data_vm = 0;
>  }
>
>  /*
> @@ -299,30 +300,6 @@ static int split_vma(struct vma_iterator *vmi, struct vm_area_struct *vma,
>  	return __split_vma(vmi, vma, addr, new_below);
>  }
>
> -/*
> - * Ok - we have the memory areas we should free on a maple tree so release them,
> - * and do the vma updates.
> - *
> - * Called with the mm semaphore held.
> - */
> -static inline void remove_mt(struct mm_struct *mm, struct ma_state *mas)
> -{
> -	unsigned long nr_accounted = 0;
> -	struct vm_area_struct *vma;
> -
> -	/* Update high watermark before we lower total_vm */
> -	update_hiwater_vm(mm);
> -	mas_for_each(mas, vma, ULONG_MAX) {
> -		long nrpages = vma_pages(vma);
> -
> -		if (vma->vm_flags & VM_ACCOUNT)
> -			nr_accounted += nrpages;
> -		vm_stat_account(mm, vma->vm_flags, -nrpages);
> -		remove_vma(vma, false);
> -	}
> -	vm_unacct_memory(nr_accounted);
> -}
> -
>  /*
>   * init_vma_prep() - Initializer wrapper for vma_prepare struct
>   * @vp: The vma_prepare struct
> @@ -722,7 +699,7 @@ static inline void abort_munmap_vmas(struct ma_state *mas_detach)
>  static void vms_complete_munmap_vmas(struct vma_munmap_struct *vms,
>  		struct ma_state *mas_detach)
>  {
> -	struct vm_area_struct *prev, *next;
> +	struct vm_area_struct *vma;
>  	struct mm_struct *mm;
>
>  	mm = vms->mm;
> @@ -731,21 +708,31 @@ static void vms_complete_munmap_vmas(struct vma_munmap_struct *vms,
>  	if (vms->unlock)
>  		mmap_write_downgrade(mm);
>
> -	prev = vma_iter_prev_range(vms->vmi);
> -	next = vma_next(vms->vmi);
> -	if (next)
> -		vma_iter_prev_range(vms->vmi);
> -
>  	/*
>  	 * We can free page tables without write-locking mmap_lock because VMAs
>  	 * were isolated before we downgraded mmap_lock.
>  	 */
>  	mas_set(mas_detach, 1);
> -	unmap_region(mm, mas_detach, vms->vma, prev, next, vms->start, vms->end,
> -		     vms->vma_count, !vms->unlock);
> -	/* Statistics and freeing VMAs */
> +	unmap_region(mm, mas_detach, vms->vma, vms->prev, vms->next,
> +		     vms->start, vms->end, vms->vma_count, !vms->unlock);
> +	/* Update high watermark before we lower total_vm */
> +	update_hiwater_vm(mm);
> +	/* Stat accounting */
> +	WRITE_ONCE(mm->total_vm, READ_ONCE(mm->total_vm) - vms->nr_pages);
> +	mm->exec_vm -= vms->exec_vm;
> +	mm->stack_vm -= vms->stack_vm;
> +	mm->data_vm -= vms->data_vm;
> +	/* Paranoid bookkeeping */
> +	VM_WARN_ON(vms->exec_vm > mm->exec_vm);
> +	VM_WARN_ON(vms->stack_vm > mm->stack_vm);
> +	VM_WARN_ON(vms->data_vm > mm->data_vm);
> +

I'm running the v7 Patchset on linux-next-20240822 and I get lots of these
errors (right on boot) (both when using the complete patchset and when using
only the patches up to this):

[  T620] WARNING: CPU: 6 PID: 620 at mm/vma.c:725
vms_complete_munmap_vmas+0x1d8/0x200
[  T620] Modules linked in: amd_atl ecc mc sparse_keymap wmi_bmof edac_mce_amd
snd snd_pci_acp3x k10temp soundcore ccp battery ac button hid_sensor_gyro_3d
hid_sensor_als hid_sensor_magn_3d hid_sensor_prox hid_sensor_accel_3d
hid_sensor_trigger industrialio_triggered_buffer kfifo_buf industrialio amd_pmc
hid_sensor_iio_common joydev evdev serio_raw mt7921e mt7921_common mt792x_lib
mt76_connac_lib mt76 mac80211 libarc4 cfg80211 rfkill msr nvme_fabrics fuse
efi_pstore configfs efivarfs autofs4 ext4 crc32c_generic mbcache jbd2 usbhid
amdgpu i2c_algo_bit drm_ttm_helper ttm drm_exec drm_suballoc_helper amdxcp
xhci_pci drm_buddy hid_sensor_hub xhci_hcd nvme mfd_core gpu_sched
hid_multitouch hid_generic crc32c_intel psmouse usbcore i2c_piix4
drm_display_helper amd_sfh i2c_hid_acpi i2c_smbus usb_common crc16 nvme_core
r8169 i2c_hid hid i2c_designware_platform i2c_designware_core
[  T620] CPU: 6 UID: 0 PID: 620 Comm: fsck.vfat Not tainted 6.11.0-rc4-next-
20240822-liamh-v7-00021-gc6686c81601f #322
[  T620] Hardware name: Micro-Star International Co., Ltd. Alpha 15 B5EEK/MS-
158L, BIOS E158LAMS.107 11/10/2021
[  T620] RIP: 0010:vms_complete_munmap_vmas+0x1d8/0x200
[  T620] Code: 8b 85 a8 00 00 00 a8 01 74 35 8b 85 e0 00 00 00 48 8d bd a8 00 00
00 83 c0 01 89 85 e0 00 00 00 e8 7d 39 e8 ff e9 63 fe ff ff <0f> 0b e9 eb fe ff
ff 0f 0b e9 d0 fe ff ff 0f 0b e9 d3 fe ff ff 0f
[  T620] RSP: 0018:ffffa415c09d7d10 EFLAGS: 00010283
[  T620] RAX: 00000000000000cd RBX: ffffa415c09d7d90 RCX: 000000000000018e
[  T620] RDX: 0000000000000021 RSI: 00000000000019d9 RDI: ffff9073ee7a6400
[  T620] RBP: ffff906541341f80 R08: 0000000000000000 R09: 000000000000080a
[  T620] R10: 000000000001d4de R11: 0000000000000140 R12: ffffa415c09d7d48
[  T620] R13: 00007fbd5ea5f000 R14: 00007fbd5eb5efff R15: ffffa415c09d7d90
[  T620] FS:  00007fbd5ec38740(0000) GS:ffff9073ee780000(0000)
knlGS:0000000000000000
[  T620] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[  T620] CR2: 00007fc336339c90 CR3: 000000010a39e000 CR4: 0000000000750ef0
[  T620] PKRU: 55555554
[  T620] Call Trace:
[  T620]  <TASK>
[  T620]  ? __warn.cold+0x90/0x9e
[  T620]  ? vms_complete_munmap_vmas+0x1d8/0x200
[  T620]  ? report_bug+0xfa/0x140
[  T620]  ? handle_bug+0x53/0x90
[  T620]  ? exc_invalid_op+0x17/0x70
[  T620]  ? asm_exc_invalid_op+0x1a/0x20
[  T620]  ? vms_complete_munmap_vmas+0x1d8/0x200
[  T620]  do_vmi_align_munmap+0x1e0/0x260
[  T620]  do_vmi_munmap+0xbe/0x160
[  T620]  __vm_munmap+0x96/0x110
[  T620]  __x64_sys_munmap+0x16/0x20
[  T620]  do_syscall_64+0x5f/0x170
[  T620]  entry_SYSCALL_64_after_hwframe+0x55/0x5d
[  T620] RIP: 0033:0x7fbd5ed3ec57
[  T620] Code: 73 01 c3 48 8b 0d c1 71 0d 00 f7 d8 64 89 01 48 83 c8 ff c3 66 2e
0f 1f 84 00 00 00 00 00 0f 1f 44 00 00 b8 0b 00 00 00 0f 05 <48> 3d 01 f0 ff ff
73 01 c3 48 8b 0d 91 71 0d 00 f7 d8 64 89 01 48
[  T620] RSP: 002b:00007fff0b04d298 EFLAGS: 00000202 ORIG_RAX: 000000000000000b
[  T620] RAX: ffffffffffffffda RBX: ffffffffffffff88 RCX: 00007fbd5ed3ec57
[  T620] RDX: 0000000000000000 RSI: 0000000000100000 RDI: 00007fbd5ea5f000
[  T620] RBP: 0000000000000002 R08: 0000000000100000 R09: 0000000000000007
[  T620] R10: 0000000000000007 R11: 0000000000000202 R12: 00007fff0b04d588
[  T620] R13: 000055b76c789fc6 R14: 00007fff0b04d360 R15: 00007fff0b04d3c0
[  T620]  </TASK>
[  T620] ---[ end trace 0000000000000000 ]---


Bert Karwatzki