diff mbox series

mm: get rid of WARN if failed to cow user pages

Message ID 20191225054227.gii6ctjkuddjnprs@xzhoux.usersys.redhat.com (mailing list archive)
State New, archived
Headers show
Series mm: get rid of WARN if failed to cow user pages | expand

Commit Message

Murphy Zhou Dec. 25, 2019, 5:42 a.m. UTC 
By running xfstests with fsdax enabled, generic/437 always hits this
warning[1] since this commit:

commit 83d116c53058d505ddef051e90ab27f57015b025
Author: Jia He <justin.he@arm.com>
Date:   Fri Oct 11 22:09:39 2019 +0800

    mm: fix double page fault on arm64 if PTE_AF is cleared

Looking at the test program[2] generic/437 uses, it's pretty easy
to hit this warning. Remove this WARN as it seems not necessary.

[2] https://git.kernel.org/pub/scm/fs/xfs/xfstests-dev.git/tree/src/t_mmap_cow_race.c
[1] warning message:
-----------------------------------------------------------------------
[   97.344077] WARNING: CPU: 0 PID: 2486 at mm/memory.c:2281 wp_page_copy+0x687/0x6e0
[   97.348354] Modules linked in: nf_tables nfnetlink rfkill sunrpc snd_hda_codec_generic ledtrig_audio qxl snd_hda_intel snd_intel_dspcfg drm_ttm_helper snd_hda_codec ttm snd_hda_core drm_kms_helper snd_hwdep snd_seq syscopyarea sysfillrect sysimgblt snd_seq_device fb_sys_fops snd_pcm drm snd_timer crc32_pclmul snd soundcore dax_pmem_compat i2c_piix4 device_dax virtio_balloon pcspkr joydev dax_pmem_core ip_tables xfs libcrc32c crct10dif_pclmul crc32c_intel sd_mod sg ata_generic 8139too ata_piix libata ghash_clmulni_intel 8139cp virtio_console serio_raw nd_pmem mii dm_mirror dm_region_hash dm_log dm_mod
[   97.382176] CPU: 0 PID: 2486 Comm: t_mmap_cow_race Tainted: G        W         5.5.0-rc3-v5.5-rc3 #1
[   97.387804] Hardware name: Red Hat KVM, BIOS 0.5.1 01/01/2011
[   97.392228] RIP: 0010:wp_page_copy+0x687/0x6e0
[   97.396572] Code: 95 f5 00 48 81 e6 00 f0 ff ff ba 00 10 00 00 49 c1 ff 06 49 c1 e7 0c 4c 03 3d 35 95 f5 00 4c 89 ff e8 8d 85 6a 00 85 c0 74 0a <0f> 0b 4c 89 ff e8 8f 80 6a 00 65 48 8b 04 25 40 7f 01 00 83 a8 d8
[   97.413487] RSP: 0000:ffffb882493afd28 EFLAGS: 00010206
[   97.417520] RAX: 0000000000001000 RBX: ffffb882493afdf8 RCX: 0000000000001000
[   97.422295] RDX: 0000000000001000 RSI: 00007f1d20c00000 RDI: ffff976384d1f000
[   97.426914] RBP: 0000000000000000 R08: 0000000000000000 R09: 00000000000ca308
[   97.431746] R10: 0000000000000000 R11: ffffe0cd4c1347c0 R12: ffffe0cd4c1347c0
[   97.436371] R13: ffff9763b46ba190 R14: ffff9763a963d0c0 R15: ffff976384d1f000
[   97.441085] FS:  00007f1d203fe700(0000) GS:ffff9763b8a00000(0000) knlGS:0000000000000000
[   97.445500] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[   97.448393] CR2: 00007f1d20c00000 CR3: 0000000333dfc000 CR4: 00000000000006f0
[   97.452346] Call Trace:
[   97.453681]  ? __switch_to_asm+0x34/0x70
[   97.455566]  ? __switch_to_asm+0x40/0x70
[   97.457418]  ? __switch_to_asm+0x34/0x70
[   97.459197]  ? __switch_to_asm+0x40/0x70
[   97.460971]  ? __switch_to_asm+0x34/0x70
[   97.462746]  ? __switch_to_asm+0x40/0x70
[   97.464561]  ? __switch_to_asm+0x34/0x70
[   97.466342]  ? __switch_to_asm+0x40/0x70
[   97.468141]  do_wp_page+0x8c/0x640
[   97.469818]  ? finish_task_switch+0x77/0x2a0
[   97.471631]  __handle_mm_fault+0xa06/0x1420
[   97.473517]  handle_mm_fault+0xae/0x1d0
[   97.475168]  __do_page_fault+0x27f/0x4e0
[   97.476947]  do_page_fault+0x30/0x110
[   97.478490]  async_page_fault+0x39/0x40
[   97.480275] RIP: 0033:0x400d68
[   97.481587] Code: 53 48 89 fb 48 83 ec 10 66 2e 0f 1f 84 00 00 00 00 00 0f b6 03 ba 04 00 00 00 be 00 00 20 00 48 89 df 89 44 24 0c 8b 44 24 0c <88> 03 e8 71 fc ff ff 85 c0 78 30 e8 b8 fc ff ff 89 c7 41 f7 ec 89
[   97.489326] RSP: 002b:00007f1d203fded0 EFLAGS: 00010202
[   97.491336] RAX: 0000000000000001 RBX: 00007f1d20c00000 RCX: 0000000000000000
[   97.494080] RDX: 0000000000000004 RSI: 0000000000200000 RDI: 00007f1d20c00000
[   97.497244] RBP: 0000000000000002 R08: 00007f1d2138d230 R09: 00007f1d2138d260
[   97.500028] R10: 00007f1d203fe9d0 R11: 0000000000000000 R12: 0000000051eb851f
[   97.502785] R13: 00007fff01d5607f R14: 0000000000000000 R15: 00007f1d203fdfc0
[   97.505546] ---[ end trace 18f1c94bd7c3d1e1 ]---

Signed-off-by: Murphy Zhou <jencce.kernel@gmail.com>
---
 mm/memory.c | 14 ++++----------
 1 file changed, 4 insertions(+), 10 deletions(-)

Comments

Jia He Dec. 25, 2019, 8:17 a.m. UTC | #1 
Hi Murphy

> -----Original Message-----
> From: Murphy Zhou <jencce.kernel@gmail.com>
> Sent: Wednesday, December 25, 2019 1:42 PM
> To: linux-nvdimm@lists.01.org; Justin He <Justin.He@arm.com>; Ross Zwisler
> <ross.zwisler@linux.intel.com>
> Subject: [PATCH] mm: get rid of WARN if failed to cow user pages
>
> By running xfstests with fsdax enabled, generic/437 always hits this
> warning[1] since this commit:
>
> commit 83d116c53058d505ddef051e90ab27f57015b025
> Author: Jia He <justin.he@arm.com>
> Date:   Fri Oct 11 22:09:39 2019 +0800
>
>     mm: fix double page fault on arm64 if PTE_AF is cleared
>
> Looking at the test program[2] generic/437 uses, it's pretty easy
> to hit this warning. Remove this WARN as it seems not necessary.
>
> [2] https://git.kernel.org/pub/scm/fs/xfs/xfstests-
> dev.git/tree/src/t_mmap_cow_race.c
> [1] warning message:
> -----------------------------------------------------------------------
> [   97.344077] WARNING: CPU: 0 PID: 2486 at mm/memory.c:2281
It is hard for me to reproduce it in my x86 desktop when running that generic/437

Could you please share your test env? E.g. guest or host? What is the test_dev of
your xfstests? Is it a 100% reproducible issue?

Besides, a few days ago, syzbot reported a similar issue, Kirill relied his concerns
at [1]

[1] https://www.spinics.net/lists/linux-mm/msg199008.html

--
Cheers,
Justin (Jia He)


> wp_page_copy+0x687/0x6e0
> [   97.348354] Modules linked in: nf_tables nfnetlink rfkill sunrpc
> snd_hda_codec_generic ledtrig_audio qxl snd_hda_intel snd_intel_dspcfg
> drm_ttm_helper snd_hda_codec ttm snd_hda_core drm_kms_helper
> snd_hwdep snd_seq syscopyarea sysfillrect sysimgblt snd_seq_device
> fb_sys_fops snd_pcm drm snd_timer crc32_pclmul snd soundcore
> dax_pmem_compat i2c_piix4 device_dax virtio_balloon pcspkr joydev
> dax_pmem_core ip_tables xfs libcrc32c crct10dif_pclmul crc32c_intel sd_mod
> sg ata_generic 8139too ata_piix libata ghash_clmulni_intel 8139cp
> virtio_console serio_raw nd_pmem mii dm_mirror dm_region_hash dm_log
> dm_mod
> [   97.382176] CPU: 0 PID: 2486 Comm: t_mmap_cow_race Tainted: G        W
> 5.5.0-rc3-v5.5-rc3 #1
> [   97.387804] Hardware name: Red Hat KVM, BIOS 0.5.1 01/01/2011
> [   97.392228] RIP: 0010:wp_page_copy+0x687/0x6e0
> [   97.396572] Code: 95 f5 00 48 81 e6 00 f0 ff ff ba 00 10 00 00 49 c1 ff 06 49
> c1 e7 0c 4c 03 3d 35 95 f5 00 4c 89 ff e8 8d 85 6a 00 85 c0 74 0a <0f> 0b 4c 89
> ff e8 8f 80 6a 00 65 48 8b 04 25 40 7f 01 00 83 a8 d8
> [   97.413487] RSP: 0000:ffffb882493afd28 EFLAGS: 00010206
> [   97.417520] RAX: 0000000000001000 RBX: ffffb882493afdf8 RCX:
> 0000000000001000
> [   97.422295] RDX: 0000000000001000 RSI: 00007f1d20c00000 RDI:
> ffff976384d1f000
> [   97.426914] RBP: 0000000000000000 R08: 0000000000000000 R09:
> 00000000000ca308
> [   97.431746] R10: 0000000000000000 R11: ffffe0cd4c1347c0 R12:
> ffffe0cd4c1347c0
> [   97.436371] R13: ffff9763b46ba190 R14: ffff9763a963d0c0 R15:
> ffff976384d1f000
> [   97.441085] FS:  00007f1d203fe700(0000) GS:ffff9763b8a00000(0000)
> knlGS:0000000000000000
> [   97.445500] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> [   97.448393] CR2: 00007f1d20c00000 CR3: 0000000333dfc000 CR4:
> 00000000000006f0
> [   97.452346] Call Trace:
> [   97.453681]  ? __switch_to_asm+0x34/0x70
> [   97.455566]  ? __switch_to_asm+0x40/0x70
> [   97.457418]  ? __switch_to_asm+0x34/0x70
> [   97.459197]  ? __switch_to_asm+0x40/0x70
> [   97.460971]  ? __switch_to_asm+0x34/0x70
> [   97.462746]  ? __switch_to_asm+0x40/0x70
> [   97.464561]  ? __switch_to_asm+0x34/0x70
> [   97.466342]  ? __switch_to_asm+0x40/0x70
> [   97.468141]  do_wp_page+0x8c/0x640
> [   97.469818]  ? finish_task_switch+0x77/0x2a0
> [   97.471631]  __handle_mm_fault+0xa06/0x1420
> [   97.473517]  handle_mm_fault+0xae/0x1d0
> [   97.475168]  __do_page_fault+0x27f/0x4e0
> [   97.476947]  do_page_fault+0x30/0x110
> [   97.478490]  async_page_fault+0x39/0x40
> [   97.480275] RIP: 0033:0x400d68
> [   97.481587] Code: 53 48 89 fb 48 83 ec 10 66 2e 0f 1f 84 00 00 00 00 00 0f
> b6 03 ba 04 00 00 00 be 00 00 20 00 48 89 df 89 44 24 0c 8b 44 24 0c <88> 03
> e8 71 fc ff ff 85 c0 78 30 e8 b8 fc ff ff 89 c7 41 f7 ec 89
> [   97.489326] RSP: 002b:00007f1d203fded0 EFLAGS: 00010202
> [   97.491336] RAX: 0000000000000001 RBX: 00007f1d20c00000 RCX:
> 0000000000000000
> [   97.494080] RDX: 0000000000000004 RSI: 0000000000200000 RDI:
> 00007f1d20c00000
> [   97.497244] RBP: 0000000000000002 R08: 00007f1d2138d230 R09:
> 00007f1d2138d260
> [   97.500028] R10: 00007f1d203fe9d0 R11: 0000000000000000 R12:
> 0000000051eb851f
> [   97.502785] R13: 00007fff01d5607f R14: 0000000000000000 R15:
> 00007f1d203fdfc0
> [   97.505546] ---[ end trace 18f1c94bd7c3d1e1 ]---
>
> Signed-off-by: Murphy Zhou <jencce.kernel@gmail.com>
> ---
>  mm/memory.c | 14 ++++----------
>  1 file changed, 4 insertions(+), 10 deletions(-)
>
> diff --git a/mm/memory.c b/mm/memory.c
> index 45442d9..e3a1dce 100644
> --- a/mm/memory.c
> +++ b/mm/memory.c
> @@ -2269,18 +2269,12 @@ static inline bool cow_user_page(struct page
> *dst, struct page *src,
>
>       /*
>        * This really shouldn't fail, because the page is there
> -      * in the page tables. But it might just be unreadable,
> -      * in which case we just give up and fill the result with
> -      * zeroes.
> +      * in the page tables. But it could happen during races,
> +      * or it might just be unreadable, in which cases we
> +      * just give up and fill the result with zeroes.
>        */
> -     if (__copy_from_user_inatomic(kaddr, uaddr, PAGE_SIZE)) {
> -             /*
> -              * Give a warn in case there can be some obscure
> -              * use-case
> -              */
> -             WARN_ON_ONCE(1);
> +     if (__copy_from_user_inatomic(kaddr, uaddr, PAGE_SIZE))
>               clear_page(kaddr);
> -     }
>
>       ret = true;
>
> --
> 1.8.3.1

IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.
Murphy Zhou Dec. 26, 2019, 10:53 a.m. UTC | #2 
On Wed, Dec 25, 2019 at 08:17:29AM +0000, Justin He wrote:
> Hi Murphy
> 
> > -----Original Message-----
> > From: Murphy Zhou <jencce.kernel@gmail.com>
> > Sent: Wednesday, December 25, 2019 1:42 PM
> > To: linux-nvdimm@lists.01.org; Justin He <Justin.He@arm.com>; Ross Zwisler
> > <ross.zwisler@linux.intel.com>
> > Subject: [PATCH] mm: get rid of WARN if failed to cow user pages
> >
> > By running xfstests with fsdax enabled, generic/437 always hits this
> > warning[1] since this commit:
> >
> > commit 83d116c53058d505ddef051e90ab27f57015b025
> > Author: Jia He <justin.he@arm.com>
> > Date:   Fri Oct 11 22:09:39 2019 +0800
> >
> >     mm: fix double page fault on arm64 if PTE_AF is cleared
> >
> > Looking at the test program[2] generic/437 uses, it's pretty easy
> > to hit this warning. Remove this WARN as it seems not necessary.
> >
> > [2] https://git.kernel.org/pub/scm/fs/xfs/xfstests-
> > dev.git/tree/src/t_mmap_cow_race.c
> > [1] warning message:
> > -----------------------------------------------------------------------
> > [   97.344077] WARNING: CPU: 0 PID: 2486 at mm/memory.c:2281
> It is hard for me to reproduce it in my x86 desktop when running that generic/437
> 
> Could you please share your test env? E.g. guest or host? What is the test_dev of
> your xfstests? Is it a 100% reproducible issue?

It's a kvm guest, with pmem ramdisks. 100% reproducible.

[root@8u ~]# uname -r
5.5.0-rc3-v5.5-rc3
[root@8u ~]# cat /proc/cmdline
BOOT_IMAGE=(hd0,msdos2)/boot/vmlinuz-5.5.0-rc3-v5.5-rc3 root=UUID=62b76d7e-e314-4400-9ea7-ded0de2f9fca ro crashkernel=auto resume=/dev/mapper/rhel-swap rd.lvm.lv=rhel/swap console=ttyS0,115200 memmap=5G!6G memmap=5G!13G
[root@8u ~]# lspci
00:00.0 Host bridge: Intel Corporation 440FX - 82441FX PMC [Natoma] (rev 02)
00:01.0 ISA bridge: Intel Corporation 82371SB PIIX3 ISA [Natoma/Triton II]
00:01.1 IDE interface: Intel Corporation 82371SB PIIX3 IDE [Natoma/Triton II]
00:01.3 Bridge: Intel Corporation 82371AB/EB/MB PIIX4 ACPI (rev 03)
00:02.0 VGA compatible controller: Red Hat, Inc. QXL paravirtual graphic card (rev 04)
00:03.0 Ethernet controller: Realtek Semiconductor Co., Ltd.  RTL-8100/8101L/8139 PCI Fast Ethernet Adapter (rev 20)
00:04.0 Audio device: Intel Corporation 82801FB/FBM/FR/FW/FRW (ICH6 Family) High Definition Audio Controller (rev 01)
00:05.0 USB controller: Intel Corporation 82801I (ICH9 Family) USB UHCI Controller #1 (rev 03)
00:05.1 USB controller: Intel Corporation 82801I (ICH9 Family) USB UHCI Controller #2 (rev 03)
00:05.2 USB controller: Intel Corporation 82801I (ICH9 Family) USB UHCI Controller #3 (rev 03)
00:05.7 USB controller: Intel Corporation 82801I (ICH9 Family) USB2 EHCI Controller #1 (rev 03)
00:06.0 Communication controller: Red Hat, Inc. Virtio console
00:07.0 Unclassified device [00ff]: Red Hat, Inc. Virtio memory balloon
00:08.0 Ethernet controller: Realtek Semiconductor Co., Ltd.  RTL-8100/8101L/8139 PCI Fast Ethernet Adapter (rev 20)
[root@8u ~]#
[root@8u ~]# ndctl list
[
  {
    "dev":"namespace0.0",
    "mode":"fsdax",
    "map":"mem",
    "size":5368709120,
    "sector_size":512,
    "blockdev":"pmem0"
  }
]
[root@8u ~]# lsblk
NAME          MAJ:MIN RM  SIZE RO TYPE MOUNTPOINT
sda             8:0    0  620G  0 disk
├─sda1          8:1    0    2G  0 part
│ └─rhel-swap 253:0    0    2G  0 lvm  [SWAP]
└─sda2          8:2    0  618G  0 part /
sdb             8:16   0  200G  0 disk /home
sdc             8:32   0   20G  0 disk
├─sdc1          8:33   0   10G  0 part
└─sdc2          8:34   0   10G  0 part
pmem0         259:0    0    5G  0 disk
├─pmem0p1     259:1    0  2.4G  0 part
└─pmem0p2     259:2    0  2.6G  0 part
[root@8u ~]#

[root@8u xfstests-dev (master)]# cat local.config
TEST_DEV=/dev/pmem0p1
TEST_DIR=/test2
SCRATCH_DEV=/dev/pmem0p2
SCRATCH_MNT=/test1
FSTYP=xfs
MOUNT_OPTIONS="-o dax"
TEST_FS_MOUNT_OPTS="-o dax"
MKFS_OPTIONS="-f -b size=4096 -m reflink=0"
LOGWRITES_DEV=""
[root@8u xfstests-dev (master)]#
> 
> Besides, a few days ago, syzbot reported a similar issue, Kirill relied his concerns
> at [1]
> 
> [1] https://www.spinics.net/lists/linux-mm/msg199008.html
> 
> --
> Cheers,
> Justin (Jia He)
> 
> 
> > wp_page_copy+0x687/0x6e0
> > [   97.348354] Modules linked in: nf_tables nfnetlink rfkill sunrpc
> > snd_hda_codec_generic ledtrig_audio qxl snd_hda_intel snd_intel_dspcfg
> > drm_ttm_helper snd_hda_codec ttm snd_hda_core drm_kms_helper
> > snd_hwdep snd_seq syscopyarea sysfillrect sysimgblt snd_seq_device
> > fb_sys_fops snd_pcm drm snd_timer crc32_pclmul snd soundcore
> > dax_pmem_compat i2c_piix4 device_dax virtio_balloon pcspkr joydev
> > dax_pmem_core ip_tables xfs libcrc32c crct10dif_pclmul crc32c_intel sd_mod
> > sg ata_generic 8139too ata_piix libata ghash_clmulni_intel 8139cp
> > virtio_console serio_raw nd_pmem mii dm_mirror dm_region_hash dm_log
> > dm_mod
> > [   97.382176] CPU: 0 PID: 2486 Comm: t_mmap_cow_race Tainted: G        W
> > 5.5.0-rc3-v5.5-rc3 #1
> > [   97.387804] Hardware name: Red Hat KVM, BIOS 0.5.1 01/01/2011
> > [   97.392228] RIP: 0010:wp_page_copy+0x687/0x6e0
> > [   97.396572] Code: 95 f5 00 48 81 e6 00 f0 ff ff ba 00 10 00 00 49 c1 ff 06 49
> > c1 e7 0c 4c 03 3d 35 95 f5 00 4c 89 ff e8 8d 85 6a 00 85 c0 74 0a <0f> 0b 4c 89
> > ff e8 8f 80 6a 00 65 48 8b 04 25 40 7f 01 00 83 a8 d8
> > [   97.413487] RSP: 0000:ffffb882493afd28 EFLAGS: 00010206
> > [   97.417520] RAX: 0000000000001000 RBX: ffffb882493afdf8 RCX:
> > 0000000000001000
> > [   97.422295] RDX: 0000000000001000 RSI: 00007f1d20c00000 RDI:
> > ffff976384d1f000
> > [   97.426914] RBP: 0000000000000000 R08: 0000000000000000 R09:
> > 00000000000ca308
> > [   97.431746] R10: 0000000000000000 R11: ffffe0cd4c1347c0 R12:
> > ffffe0cd4c1347c0
> > [   97.436371] R13: ffff9763b46ba190 R14: ffff9763a963d0c0 R15:
> > ffff976384d1f000
> > [   97.441085] FS:  00007f1d203fe700(0000) GS:ffff9763b8a00000(0000)
> > knlGS:0000000000000000
> > [   97.445500] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> > [   97.448393] CR2: 00007f1d20c00000 CR3: 0000000333dfc000 CR4:
> > 00000000000006f0
> > [   97.452346] Call Trace:
> > [   97.453681]  ? __switch_to_asm+0x34/0x70
> > [   97.455566]  ? __switch_to_asm+0x40/0x70
> > [   97.457418]  ? __switch_to_asm+0x34/0x70
> > [   97.459197]  ? __switch_to_asm+0x40/0x70
> > [   97.460971]  ? __switch_to_asm+0x34/0x70
> > [   97.462746]  ? __switch_to_asm+0x40/0x70
> > [   97.464561]  ? __switch_to_asm+0x34/0x70
> > [   97.466342]  ? __switch_to_asm+0x40/0x70
> > [   97.468141]  do_wp_page+0x8c/0x640
> > [   97.469818]  ? finish_task_switch+0x77/0x2a0
> > [   97.471631]  __handle_mm_fault+0xa06/0x1420
> > [   97.473517]  handle_mm_fault+0xae/0x1d0
> > [   97.475168]  __do_page_fault+0x27f/0x4e0
> > [   97.476947]  do_page_fault+0x30/0x110
> > [   97.478490]  async_page_fault+0x39/0x40
> > [   97.480275] RIP: 0033:0x400d68
> > [   97.481587] Code: 53 48 89 fb 48 83 ec 10 66 2e 0f 1f 84 00 00 00 00 00 0f
> > b6 03 ba 04 00 00 00 be 00 00 20 00 48 89 df 89 44 24 0c 8b 44 24 0c <88> 03
> > e8 71 fc ff ff 85 c0 78 30 e8 b8 fc ff ff 89 c7 41 f7 ec 89
> > [   97.489326] RSP: 002b:00007f1d203fded0 EFLAGS: 00010202
> > [   97.491336] RAX: 0000000000000001 RBX: 00007f1d20c00000 RCX:
> > 0000000000000000
> > [   97.494080] RDX: 0000000000000004 RSI: 0000000000200000 RDI:
> > 00007f1d20c00000
> > [   97.497244] RBP: 0000000000000002 R08: 00007f1d2138d230 R09:
> > 00007f1d2138d260
> > [   97.500028] R10: 00007f1d203fe9d0 R11: 0000000000000000 R12:
> > 0000000051eb851f
> > [   97.502785] R13: 00007fff01d5607f R14: 0000000000000000 R15:
> > 00007f1d203fdfc0
> > [   97.505546] ---[ end trace 18f1c94bd7c3d1e1 ]---
> >
> > Signed-off-by: Murphy Zhou <jencce.kernel@gmail.com>
> > ---
> >  mm/memory.c | 14 ++++----------
> >  1 file changed, 4 insertions(+), 10 deletions(-)
> >
> > diff --git a/mm/memory.c b/mm/memory.c
> > index 45442d9..e3a1dce 100644
> > --- a/mm/memory.c
> > +++ b/mm/memory.c
> > @@ -2269,18 +2269,12 @@ static inline bool cow_user_page(struct page
> > *dst, struct page *src,
> >
> >       /*
> >        * This really shouldn't fail, because the page is there
> > -      * in the page tables. But it might just be unreadable,
> > -      * in which case we just give up and fill the result with
> > -      * zeroes.
> > +      * in the page tables. But it could happen during races,
> > +      * or it might just be unreadable, in which cases we
> > +      * just give up and fill the result with zeroes.
> >        */
> > -     if (__copy_from_user_inatomic(kaddr, uaddr, PAGE_SIZE)) {
> > -             /*
> > -              * Give a warn in case there can be some obscure
> > -              * use-case
> > -              */
> > -             WARN_ON_ONCE(1);
> > +     if (__copy_from_user_inatomic(kaddr, uaddr, PAGE_SIZE))
> >               clear_page(kaddr);
> > -     }
> >
> >       ret = true;
> >
> > --
> > 1.8.3.1
> 
> IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.
Murphy Zhou Jan. 15, 2020, 4:48 a.m. UTC | #3 
Ping ?

On Wed, Dec 25, 2019 at 01:42:27PM +0800, Murphy Zhou wrote:
> By running xfstests with fsdax enabled, generic/437 always hits this
> warning[1] since this commit:
> 
> commit 83d116c53058d505ddef051e90ab27f57015b025
> Author: Jia He <justin.he@arm.com>
> Date:   Fri Oct 11 22:09:39 2019 +0800
> 
>     mm: fix double page fault on arm64 if PTE_AF is cleared
> 
> Looking at the test program[2] generic/437 uses, it's pretty easy
> to hit this warning. Remove this WARN as it seems not necessary.
> 
> [2] https://git.kernel.org/pub/scm/fs/xfs/xfstests-dev.git/tree/src/t_mmap_cow_race.c
> [1] warning message:
> -----------------------------------------------------------------------
> [   97.344077] WARNING: CPU: 0 PID: 2486 at mm/memory.c:2281 wp_page_copy+0x687/0x6e0
> [   97.348354] Modules linked in: nf_tables nfnetlink rfkill sunrpc snd_hda_codec_generic ledtrig_audio qxl snd_hda_intel snd_intel_dspcfg drm_ttm_helper snd_hda_codec ttm snd_hda_core drm_kms_helper snd_hwdep snd_seq syscopyarea sysfillrect sysimgblt snd_seq_device fb_sys_fops snd_pcm drm snd_timer crc32_pclmul snd soundcore dax_pmem_compat i2c_piix4 device_dax virtio_balloon pcspkr joydev dax_pmem_core ip_tables xfs libcrc32c crct10dif_pclmul crc32c_intel sd_mod sg ata_generic 8139too ata_piix libata ghash_clmulni_intel 8139cp virtio_console serio_raw nd_pmem mii dm_mirror dm_region_hash dm_log dm_mod
> [   97.382176] CPU: 0 PID: 2486 Comm: t_mmap_cow_race Tainted: G        W         5.5.0-rc3-v5.5-rc3 #1
> [   97.387804] Hardware name: Red Hat KVM, BIOS 0.5.1 01/01/2011
> [   97.392228] RIP: 0010:wp_page_copy+0x687/0x6e0
> [   97.396572] Code: 95 f5 00 48 81 e6 00 f0 ff ff ba 00 10 00 00 49 c1 ff 06 49 c1 e7 0c 4c 03 3d 35 95 f5 00 4c 89 ff e8 8d 85 6a 00 85 c0 74 0a <0f> 0b 4c 89 ff e8 8f 80 6a 00 65 48 8b 04 25 40 7f 01 00 83 a8 d8
> [   97.413487] RSP: 0000:ffffb882493afd28 EFLAGS: 00010206
> [   97.417520] RAX: 0000000000001000 RBX: ffffb882493afdf8 RCX: 0000000000001000
> [   97.422295] RDX: 0000000000001000 RSI: 00007f1d20c00000 RDI: ffff976384d1f000
> [   97.426914] RBP: 0000000000000000 R08: 0000000000000000 R09: 00000000000ca308
> [   97.431746] R10: 0000000000000000 R11: ffffe0cd4c1347c0 R12: ffffe0cd4c1347c0
> [   97.436371] R13: ffff9763b46ba190 R14: ffff9763a963d0c0 R15: ffff976384d1f000
> [   97.441085] FS:  00007f1d203fe700(0000) GS:ffff9763b8a00000(0000) knlGS:0000000000000000
> [   97.445500] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> [   97.448393] CR2: 00007f1d20c00000 CR3: 0000000333dfc000 CR4: 00000000000006f0
> [   97.452346] Call Trace:
> [   97.453681]  ? __switch_to_asm+0x34/0x70
> [   97.455566]  ? __switch_to_asm+0x40/0x70
> [   97.457418]  ? __switch_to_asm+0x34/0x70
> [   97.459197]  ? __switch_to_asm+0x40/0x70
> [   97.460971]  ? __switch_to_asm+0x34/0x70
> [   97.462746]  ? __switch_to_asm+0x40/0x70
> [   97.464561]  ? __switch_to_asm+0x34/0x70
> [   97.466342]  ? __switch_to_asm+0x40/0x70
> [   97.468141]  do_wp_page+0x8c/0x640
> [   97.469818]  ? finish_task_switch+0x77/0x2a0
> [   97.471631]  __handle_mm_fault+0xa06/0x1420
> [   97.473517]  handle_mm_fault+0xae/0x1d0
> [   97.475168]  __do_page_fault+0x27f/0x4e0
> [   97.476947]  do_page_fault+0x30/0x110
> [   97.478490]  async_page_fault+0x39/0x40
> [   97.480275] RIP: 0033:0x400d68
> [   97.481587] Code: 53 48 89 fb 48 83 ec 10 66 2e 0f 1f 84 00 00 00 00 00 0f b6 03 ba 04 00 00 00 be 00 00 20 00 48 89 df 89 44 24 0c 8b 44 24 0c <88> 03 e8 71 fc ff ff 85 c0 78 30 e8 b8 fc ff ff 89 c7 41 f7 ec 89
> [   97.489326] RSP: 002b:00007f1d203fded0 EFLAGS: 00010202
> [   97.491336] RAX: 0000000000000001 RBX: 00007f1d20c00000 RCX: 0000000000000000
> [   97.494080] RDX: 0000000000000004 RSI: 0000000000200000 RDI: 00007f1d20c00000
> [   97.497244] RBP: 0000000000000002 R08: 00007f1d2138d230 R09: 00007f1d2138d260
> [   97.500028] R10: 00007f1d203fe9d0 R11: 0000000000000000 R12: 0000000051eb851f
> [   97.502785] R13: 00007fff01d5607f R14: 0000000000000000 R15: 00007f1d203fdfc0
> [   97.505546] ---[ end trace 18f1c94bd7c3d1e1 ]---
> 
> Signed-off-by: Murphy Zhou <jencce.kernel@gmail.com>
> ---
>  mm/memory.c | 14 ++++----------
>  1 file changed, 4 insertions(+), 10 deletions(-)
> 
> diff --git a/mm/memory.c b/mm/memory.c
> index 45442d9..e3a1dce 100644
> --- a/mm/memory.c
> +++ b/mm/memory.c
> @@ -2269,18 +2269,12 @@ static inline bool cow_user_page(struct page *dst, struct page *src,
>  
>  	/*
>  	 * This really shouldn't fail, because the page is there
> -	 * in the page tables. But it might just be unreadable,
> -	 * in which case we just give up and fill the result with
> -	 * zeroes.
> +	 * in the page tables. But it could happen during races,
> +	 * or it might just be unreadable, in which cases we
> +	 * just give up and fill the result with zeroes.
>  	 */
> -	if (__copy_from_user_inatomic(kaddr, uaddr, PAGE_SIZE)) {
> -		/*
> -		 * Give a warn in case there can be some obscure
> -		 * use-case
> -		 */
> -		WARN_ON_ONCE(1);
> +	if (__copy_from_user_inatomic(kaddr, uaddr, PAGE_SIZE))
>  		clear_page(kaddr);
> -	}
>  
>  	ret = true;
>  
> -- 
> 1.8.3.1
>
Dan Williams Jan. 15, 2020, 6:02 a.m. UTC | #4 
[ drop Ross, add Kirill, linux-mm, and lkml ]

On Tue, Dec 24, 2019 at 9:42 PM Murphy Zhou <jencce.kernel@gmail.com> wrote:
>
> By running xfstests with fsdax enabled, generic/437 always hits this
> warning[1] since this commit:
>
> commit 83d116c53058d505ddef051e90ab27f57015b025
> Author: Jia He <justin.he@arm.com>
> Date:   Fri Oct 11 22:09:39 2019 +0800
>
>     mm: fix double page fault on arm64 if PTE_AF is cleared
>
> Looking at the test program[2] generic/437 uses, it's pretty easy
> to hit this warning. Remove this WARN as it seems not necessary.

This is not sufficient justification. Does this same test fail without
DAX? If not, why not? At a minimum you need to explain why this is not
indicating a problem.
Jeff Moyer Feb. 18, 2020, 9:09 p.m. UTC | #5 
Dan Williams <dan.j.williams@intel.com> writes:

> [ drop Ross, add Kirill, linux-mm, and lkml ]
>
> On Tue, Dec 24, 2019 at 9:42 PM Murphy Zhou <jencce.kernel@gmail.com> wrote:
>>
>> By running xfstests with fsdax enabled, generic/437 always hits this
>> warning[1] since this commit:
>>
>> commit 83d116c53058d505ddef051e90ab27f57015b025
>> Author: Jia He <justin.he@arm.com>
>> Date:   Fri Oct 11 22:09:39 2019 +0800
>>
>>     mm: fix double page fault on arm64 if PTE_AF is cleared
>>
>> Looking at the test program[2] generic/437 uses, it's pretty easy
>> to hit this warning. Remove this WARN as it seems not necessary.
>
> This is not sufficient justification. Does this same test fail without
> DAX? If not, why not? At a minimum you need to explain why this is not
> indicating a problem.

I ran into this, too, and Kirill has posted a patch[1] to fix the issue.
Note that it's a potential data corrupter, so just removing the warning
is NOT the right approach.  :)

-Jeff

[1] https://lore.kernel.org/linux-mm/20200218154151.13349-1-kirill.shutemov@linux.intel.com/T/#u
Murphy Zhou Feb. 19, 2020, 1:58 a.m. UTC | #6 
On Tue, Feb 18, 2020 at 04:09:30PM -0500, Jeff Moyer wrote:
> Dan Williams <dan.j.williams@intel.com> writes:
> 
> > [ drop Ross, add Kirill, linux-mm, and lkml ]
> >
> > On Tue, Dec 24, 2019 at 9:42 PM Murphy Zhou <jencce.kernel@gmail.com> wrote:
> >>
> >> By running xfstests with fsdax enabled, generic/437 always hits this
> >> warning[1] since this commit:
> >>
> >> commit 83d116c53058d505ddef051e90ab27f57015b025
> >> Author: Jia He <justin.he@arm.com>
> >> Date:   Fri Oct 11 22:09:39 2019 +0800
> >>
> >>     mm: fix double page fault on arm64 if PTE_AF is cleared
> >>
> >> Looking at the test program[2] generic/437 uses, it's pretty easy
> >> to hit this warning. Remove this WARN as it seems not necessary.
> >
> > This is not sufficient justification. Does this same test fail without
> > DAX? If not, why not? At a minimum you need to explain why this is not
> > indicating a problem.
> 
> I ran into this, too, and Kirill has posted a patch[1] to fix the issue.
> Note that it's a potential data corrupter, so just removing the warning
> is NOT the right approach.  :)

Agree :) Thanks!

> 
> -Jeff
> 
> [1] https://lore.kernel.org/linux-mm/20200218154151.13349-1-kirill.shutemov@linux.intel.com/T/#u
>
diff mbox series

Patch

diff --git a/mm/memory.c b/mm/memory.c
index 45442d9..e3a1dce 100644
--- a/mm/memory.c
+++ b/mm/memory.c
@@ -2269,18 +2269,12 @@  static inline bool cow_user_page(struct page *dst, struct page *src,
 
 	/*
 	 * This really shouldn't fail, because the page is there
-	 * in the page tables. But it might just be unreadable,
-	 * in which case we just give up and fill the result with
-	 * zeroes.
+	 * in the page tables. But it could happen during races,
+	 * or it might just be unreadable, in which cases we
+	 * just give up and fill the result with zeroes.
 	 */
-	if (__copy_from_user_inatomic(kaddr, uaddr, PAGE_SIZE)) {
-		/*
-		 * Give a warn in case there can be some obscure
-		 * use-case
-		 */
-		WARN_ON_ONCE(1);
+	if (__copy_from_user_inatomic(kaddr, uaddr, PAGE_SIZE))
 		clear_page(kaddr);
-	}
 
 	ret = true;