HORIGUCHI NAOYA(堀口 直也) wrote: > On Wed, Aug 24, 2022 at 02:52:51PM -0700, Dan Williams wrote: > > Shiyang Ruan wrote: > > > This new function is a variant of mf_generic_kill_procs that accepts a > > > file, offset pair instead of a struct to support multiple files sharing > > > a DAX mapping. It is intended to be called by the file systems as part > > > of the memory_failure handler after the file system performed a reverse > > > mapping from the storage address to the file and file offset. > > > > > > Signed-off-by: Shiyang Ruan <ruansy.f...@fujitsu.com> > > > Reviewed-by: Dan Williams <dan.j.willi...@intel.com> > > > Reviewed-by: Christoph Hellwig <h...@lst.de> > > > Reviewed-by: Darrick J. Wong <djw...@kernel.org> > > > Reviewed-by: Miaohe Lin <linmia...@huawei.com> > > > --- > > > include/linux/mm.h | 2 + > > > mm/memory-failure.c | 96 ++++++++++++++++++++++++++++++++++++++++----- > > > 2 files changed, 88 insertions(+), 10 deletions(-) > > > > Unfortunately my test suite was only running the "non-destructive" set > > of 'ndctl' tests which skipped some of the complex memory-failure cases. > > Upon fixing that, bisect flags this commit as the source of the following > > crash regression: > > Thank you for testing/reporting. > > > > > kernel BUG at mm/memory-failure.c:310! > > invalid opcode: 0000 [#1] PREEMPT SMP PTI > > CPU: 26 PID: 1252 Comm: dax-pmd Tainted: G OE 5.19.0-rc4+ #58 > > Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 0.0.0 02/06/2015 > > RIP: 0010:add_to_kill+0x304/0x400 > > [..] > > Call Trace: > > <TASK> > > collect_procs.part.0+0x2c8/0x470 > > memory_failure+0x979/0xf30 > > do_madvise.part.0.cold+0x9c/0xd3 > > ? lock_is_held_type+0xe3/0x140 > > ? find_held_lock+0x2b/0x80 > > ? lock_release+0x145/0x2f0 > > ? lock_is_held_type+0xe3/0x140 > > ? syscall_enter_from_user_mode+0x20/0x70 > > __x64_sys_madvise+0x56/0x70 > > do_syscall_64+0x3a/0x80 > > entry_SYSCALL_64_after_hwframe+0x46/0xb0 > > This stacktrace shows that VM_BUG_ON_VMA() in dev_pagemap_mapping_shift() > was triggered. I think that BUG_ON is too harsh here because address == > -EFAULT means that there's no mapping for the address. The subsequent > code considers "tk->size_shift == 0" as "no mapping" cases, so > dev_pagemap_mapping_shift() can return 0 in such a case? > > Could the following diff work for the issue?
This passes the "dax-ext4.sh" and "dax-xfs.sh" tests from the ndctl suite. It then fails on the "device-dax" test with this signature: BUG: kernel NULL pointer dereference, address: 0000000000000010 #PF: supervisor read access in kernel mode #PF: error_code(0x0000) - not-present page PGD 8000000205073067 P4D 8000000205073067 PUD 2062b3067 PMD 0 Oops: 0000 [#1] PREEMPT SMP PTI CPU: 22 PID: 4535 Comm: device-dax Tainted: G OE N 6.0.0-rc2+ #59 Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 0.0.0 02/06/2015 RIP: 0010:memory_failure+0x667/0xba0 [..] Call Trace: <TASK> ? _printk+0x58/0x73 do_madvise.part.0.cold+0xaf/0xc5 Which is: (gdb) li *(memory_failure+0x667) 0xffffffff813b7f17 is in memory_failure (mm/memory-failure.c:1933). 1928 1929 /* 1930 * Call driver's implementation to handle the memory failure, otherwise 1931 * fall back to generic handler. 1932 */ 1933 if (pgmap->ops->memory_failure) { 1934 rc = pgmap->ops->memory_failure(pgmap, pfn, 1, flags); ...I think this is just a simple matter of: @@ -1928,7 +1930,7 @@ static int memory_failure_dev_pagemap(unsigned long pfn, int flags, * Call driver's implementation to handle the memory failure, otherwise * fall back to generic handler. */ - if (pgmap->ops->memory_failure) { + if (pgmap->ops && pgmap->ops->memory_failure) { rc = pgmap->ops->memory_failure(pgmap, pfn, 1, flags); /* * Fall back to generic handler too if operation is not ...since device-dax does not implement pagemap ops. I will see what else pops up and make sure that this regression always runs going forward.