linux-ext4 - Re: [RFC 04/10] fs/dax: Don't track page mapping/index

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <87h6g2b1qs.fsf@nvdebian.thelocal>
Date: Tue, 16 Apr 2024 10:07:16 +1000
From: Alistair Popple <apopple@...dia.com>
To: Dan Williams <dan.j.williams@...el.com>
Cc: linux-mm@...ck.org, david@...morbit.com, jhubbard@...dia.com,
 rcampbell@...dia.com, willy@...radead.org, jgg@...dia.com,
 linux-fsdevel@...r.kernel.org, jack@...e.cz, djwong@...nel.org,
 hch@....de, david@...hat.com, ruansy.fnst@...itsu.com,
 nvdimm@...ts.linux.dev, linux-xfs@...r.kernel.org,
 linux-ext4@...r.kernel.org, jglisse@...hat.com
Subject: Re: [RFC 04/10] fs/dax: Don't track page mapping/index


Dan Williams <dan.j.williams@...el.com> writes:

> Alistair Popple wrote:
>> I was initially concerned about these cases because I was wondering if
>> folio subpages could ever get different mappings and the shared case
>> implied they could. But it seems that's xfs specific and there is a
>> separate mechanism to deal with looking up ->mapping/index for that. So
>> I guess we should still be able to safely store this on the folio
>> head. I will double check and update this change.
>> 
>
> I think there is path to store this information only on the folio head.
> However, ugh, I think this is potentially another "head" of the
> pmd_devmap() hydra.
>
> pmd_devmap() taught the core-mm to treat dax_pmds indentically to
> thp_pmds *except* for the __split_huge_pmd() case:
>
>    5c7fb56e5e3f mm, dax: dax-pmd vs thp-pmd vs hugetlbfs-pmd
>
> Later on pmd migration entries joined pmd_devmap() in skipping splits:
>
>    84c3fc4e9c56 mm: thp: check pmd migration entry in common path
>
> Unfortunately, pmd_devmap() stopped being considered for skipping
> splits here:
>
>    7f7609175ff2 mm/huge_memory: remove stale locking logic from __split_huge_pmd()
>
> Likely __split_huge_pmd_locked() grew support for pmd migration handling
> and forgot about the pmd_devmap() case.
>
> So now Linux has been allowing FSDAX pmd splits since v5.18...

>From what I see we currently (in v6.6) have this in
__split_huge_pmd_locked():

        if (!vma_is_anonymous(vma)) {
                old_pmd = pmdp_huge_clear_flush_notify(vma, haddr, pmd);
                /*
                 * We are going to unmap this huge page. So
                 * just go ahead and zap it
                 */
                if (arch_needs_pgtable_deposit())
                        zap_deposited_table(mm, pmd);
                if (vma_is_special_huge(vma))
                        return;

Where vma_is_special_huge(vma) returns true for vma_is_dax(). So AFAICT
we're still skipping the split right? In all versions we just zap the
PMD and continue. What am I missing?

> but with
> no reports of any issues. Likely this is benefiting from the fact that
> the preconditions for splitting are rarely if ever satisfied because
> FSDAX mappings are never anon, and establishing the mapping in the first
> place requires a 2MB aligned file extent and that is likely never
> fractured.
>
> Same for device-dax where the fracturing *should* not be allowed, but I
> will feel better with focus tests to go after mremap() cases that would
> attempt to split the page.