Re: [RFC 00/10] fs/dax: Fix FS DAX page reference counts

Alistair Popple
2024-04-12T07:03:03Z

Alistair Popple <apopple@nvidia.com> writes:

> Dan Williams <dan.j.williams@intel.com> writes:
>
>> Alistair Popple wrote:
>>> FS DAX pages have always maintained their own page reference counts
>>> without following the normal rules for page reference counting. In
>>> particular pages are considered free when the refcount hits one rather
>>> than zero and refcounts are not added when mapping the page.
>>
>>> Tracking this requires special PTE bits (PTE_DEVMAP) and a secondary
>>> mechanism for allowing GUP to hold references on the page (see
>>> get_dev_pagemap). However there doesn't seem to be any reason why FS
>>> DAX pages need their own reference counting scheme.
>>
>> This is fair. However, for anyone coming in fresh to this situation
>> maybe some more "how we get here" history helps. That longer story is
>> here:
>>
>> http://lore.kernel.org/all/166579181584.2236710.17813547487183983273.stgit@dwillia2-xfh.jf.intel.com/
>
> Good idea.
>
>>> This RFC is an initial attempt at removing the special reference
>>> counting and instead refcount FS DAX pages the same as normal pages.
>>>
>>> There are still a couple of rough edges - in particular I haven't
>>> completely removed the devmap PTE bit references from arch specific
>>> code and there is probably some more cleanup of dev_pagemap reference
>>> counting that could be done, particular in mm/gup.c. I also haven't
>>> yet compiled on anything other than x86_64.
>>>
>>> Before continuing further with this clean-up though I would appreciate
>>> some feedback on the viability of this approach and any issues I may
>>> have overlooked, as I am not intimately familiar with FS DAX code (or
>>> for that matter the FS layer in general).
>>>
>>> I have of course run some basic testing which didn't reveal any
>>> problems.
>>
>> FWIW I see the following with the ndctl/dax test-suite (double-checked
>> that vanilla v6.6 passes). I will take a look at the patches, but in the
>> meantime...
>
> Hmmm...
>
>> # meson test -C build --suite ndctl:dax
>> ninja: no work to do.
>> ninja: Entering directory `/root/git/ndctl/build'
>> [1/70] Generating version.h with a custom command
>> 1/13 ndctl:dax / daxdev-errors.sh OK 14.46s
>> 2/13 ndctl:dax / multi-dax.sh OK 2.70s
>> 3/13 ndctl:dax / sub-section.sh OK 7.21s
>> 4/13 ndctl:dax / dax-dev OK 0.08s
>> [5/13] 🌖 ndctl:dax / dax-ext4.sh 0/600s
>
> ...thanks for pasting that output. Turns out I didn't have destructive
> testing enabled during the build so hadn't noticed these tests were not
> running. It would be nice if these were reported as skipped when not
> enabled rather than hidden.
>
> With that fixed I'm seeing a couple of kernel warnings (and I think I
> know why), so it might be worth holding off looking at this too closely
> until I've fixed these.

Ok, I think I found the dragons you were talking about earlier for
device-dax. I completely broke that because as you've already pointed
out pmd_trans_huge() won't filter out DAX pages. That's fine for FS DAX
(because the pages are essentially normal pages now anyway), but we
don't have a PMD equivalent of vm_normal_page() which leads to all sorts
of issues for DEVDAX.

So I will probably have to add something like that unless we only need
to support large (pmd/pud) mappings of DEVDAX pages on systems with
CONFIG_ARCH_HAS_PTE_SPECIAL in which case I guess we could just filter
based on pte_special().

>> ...that last test crashed with:
>>
>> EXT4-fs (pmem0): mounted filesystem 2adea02a-a791-4714-be40-125afd16634b r/w with ordered
>> ota mode: none.
>> page:ffffea0005f00000 refcount:0 mapcount:0 mapping:ffff8882a8a6be10 index:0x5800 pfn:0x1
>>
>> head:ffffea0005f00000 order:9 entire_mapcount:0 nr_pages_mapped:0 pincount:0
>> aops:ext4_dax_aops ino:c dentry name:"image"
>> flags: 0x4ffff800004040(reserved|head|node=0|zone=4|lastcpupid=0x1ffff)
>> page_type: 0xffffffff()
>> raw: 004ffff800004040 ffff888202681520 0000000000000000 ffff8882a8a6be10
>> raw: 0000000000005800 0000000000000000 00000000ffffffff 0000000000000000
>> page dumped because: VM_BUG_ON_FOLIO(((unsigned int) folio_ref_count(folio) + 127u <= 127
>>
>> ------------[ cut here ]------------
>> kernel BUG at include/linux/mm.h:1419!
>> invalid opcode: 0000 [#1] PREEMPT SMP PTI
>> CPU: 0 PID: 1415 Comm: dax-pmd Tainted: G OE N 6.6.0+ #209
>> Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS edk2-20230524-3.fc38 05/24/2023
>> RIP: 0010:dax_insert_pfn_pmd+0x41c/0x430
>> Code: 89 c1 41 b8 01 00 00 00 48 89 ea 4c 89 e6 4c 89 f7 e8 18 8a c7 ff e9 e0 fc ff ff 48
>> c b3 48 89 c7 e8 a4 53 f7 ff <0f> 0b e8 0d ba a8 00 48 8b 15 86 8a 62 01 e9 89 fc ff ff 90
>>
>> RSP: 0000:ffffc90001d57b68 EFLAGS: 00010246
>> RAX: 000000000000005c RBX: ffffea0005f00000 RCX: 0000000000000000
>> RDX: 0000000000000000 RSI: ffffffffb3749a15 RDI: 00000000ffffffff
>> RBP: ffff8882982c07e0 R08: 00000000ffffdfff R09: 0000000000000001
>> R10: 00000000ffffdfff R11: ffffffffb3a771c0 R12: 800000017c0008e7
>> R13: 8000000000000025 R14: ffff888202a395f8 R15: ffffea0005f00000
>> FS: 00007fdaa00e3d80(0000) GS:ffff888477000000(0000) knlGS:0000000000000000
>> CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
>> CR2: 00007fda9f800000 CR3: 0000000296224000 CR4: 00000000000006f0
>> DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
>> DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
>> Call Trace:
>> <TASK>
>> ? die+0x32/0x80
>> ? do_trap+0xd6/0x100
>> ? dax_insert_pfn_pmd+0x41c/0x430
>> ? dax_insert_pfn_pmd+0x41c/0x430
>> ? do_error_trap+0x81/0x110
>> ? dax_insert_pfn_pmd+0x41c/0x430
>> ? exc_invalid_op+0x4c/0x60
>> ? dax_insert_pfn_pmd+0x41c/0x430
>> ? asm_exc_invalid_op+0x16/0x20
>> ? dax_insert_pfn_pmd+0x41c/0x430
>> ? dax_insert_pfn_pmd+0x41c/0x430
>> dax_fault_iter+0x5d0/0x700
>> dax_iomap_pmd_fault+0x212/0x450
>> ext4_dax_huge_fault+0x1dc/0x470
>> __handle_mm_fault+0x808/0x13e0
>> handle_mm_fault+0x178/0x3e0
>> do_user_addr_fault+0x186/0x830
>> exc_page_fault+0x6f/0x1d0
>> asm_exc_page_fault+0x22/0x30
>> RIP: 0033:0x7fdaa072d009