linux-kernel - Re: [PATCH 2/5] mm,hugetlb: Document the reason to lock the folio in the faulting path

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: <aFFknpsmfb-Sh7xT@localhost.localdomain>
Date: Tue, 17 Jun 2025 14:50:38 +0200
From: Oscar Salvador <osalvador@...e.de>
To: David Hildenbrand <david@...hat.com>
Cc: Andrew Morton <akpm@...ux-foundation.org>,
	Muchun Song <muchun.song@...ux.dev>,
	James Houghton <jthoughton@...gle.com>,
	Peter Xu <peterx@...hat.com>, Gavin Guo <gavinguo@...lia.com>,
	linux-mm@...ck.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 2/5] mm,hugetlb: Document the reason to lock the folio in
 the faulting path

On Tue, Jun 17, 2025 at 02:10:09PM +0200, Oscar Salvador wrote:
> On Tue, Jun 17, 2025 at 02:08:16PM +0200, David Hildenbrand wrote:
> > folio lock is a sleeping lock, PTL is a spinlock. :)
> 
> Lol yes, overlooked that totally.
> And I also saw the comment from mm/rmap.c about lockin order.

So, we could do something like this:

        if (folio_test_anon(old_folio)) {
                spin_unlock(vmf->ptl);
                folio_lock(old_folio);
                spin_lock(vmf->ptl);
                vmf->pte = hugetlb_walk(vma, vmf->address, huge_page_size(h));
                if (unlikely(!vmf->pte ||
                    !pte_same(huge_ptep_get(mm, vmf->address, vmf->pte), pte))) {
                        spin_unlock(vmf->ptl);
                        folio_unlock(old_folio);
                        goto out_take_lock;
                }

                if (folio_mapcount(old_folio == 1)) {
                        if (!PageAnonExclusive(&old_folio->page)) {
                                folio_move_anon_rmap(old_folio, vma);
                                SetPageAnonExclusive(&old_folio->page);
                        }
                        if (likely(!unshare))
                                set_huge_ptep_maybe_writable(vma, vmf->address,
                                                             vmf->pte);

                        spin_unlock(vmf->ptl);
                        folio_unlock(old_folio);
                        goto out_take_lock;
                }
                VM_BUG_ON_PAGE(folio_test_anon(old_folio) &&
                               PageAnonExclusive(&old_folio->page), &old_folio->page);
                spin_unlock(vmf->ptl);
                folio_unlock(old_folio);
                spin_lock(vmf->ptl);
                vmf->pte = hugetlb_walk(vma, vmf->address, huge_page_size(h));
                if (unlikely(!vmf->pte ||
                   !pte_same(huge_ptep_get(mm, vmf->address, vmf->pte), pte)))
                        return 0;
        }

Hopefully we can do some refactor here, because I quite dislike the
unlock-lock-retake-unlock-blah cycle.



-- 
Oscar Salvador
SUSE Labs