linux-kernel - Re: [PATCH 0/5] Misc rework on hugetlb

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 for Android: free password hash cracker in your pocket

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

Message-ID: <aEvnbO0n8lkYF9yI@localhost.localdomain>
Date: Fri, 13 Jun 2025 10:55:08 +0200
From: Oscar Salvador <osalvador@...e.de>
To: Andrew Morton <akpm@...ux-foundation.org>
Cc: David Hildenbrand <david@...hat.com>,
	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 0/5] Misc rework on hugetlb_fault

On Thu, Jun 12, 2025 at 03:46:56PM +0200, Oscar Salvador wrote:
> This is a new version of the RFC I sent a couple of weeks ago [1].
> It's the conclusion of the discussions that ocurred in that thread.
> 
> Patch#1 is the fix for the deadlock.
> Patch#2 is to document why we take the lock, so none of us have to spend
> time again in figuring that out.
> Patch#3-5 are a bit of cleanup (removing obsolete comments etc.)

Andrew told me that this is very vague for a coverletter, and while this patchset
is sort of a refactor/clenaup/fixup mix, I acknowledge that it is far
from optimal to get people some context.
So let me try again, and I apologyze.

"
 This patchset aims to give some love to the hugetlb faulting path, doing so
 by removing obsolete comments that are no longer true, documenting in a clear
 way the reason we take certain locks, and changing the mechanism we use to
 determine whether we are COWing a private mapping already.

 The most important patch of the series is #1, as it fixes a deadlock that
 was described in [1], where two processes were holding the same lock
 for the folio in the pagecache, and then deadlocked in the mutex.
 Looking up and locking the folio in the pagecache was done to check whether
 that folio was the same folio we had mapped in our pagetables, meaning that if it
 was different we knew that we already mapped that folio privately, so any
 further CoW would be made on a private mapping, which lead us to the  question:
  __Was the reservation for that address consumed?__
 That is all we care about, because if it was indeed consumed and we are the
 owner and we cannot allocate more folios, we need to unmap the folio from the
 processes pagetables and make it exclusive for us.

 We figured we do not need to look up the folio at all, and it is just enough to
 check whether the folio we have mapped is anonymous, which means we mapped it
 privately, so the reservation was indeed consumed.

 Patch#2 follows a bit more to the trend on why we need to lock the
 folio from the pagetables, whether it is anonymous or not.
 For anonymous folios we need the lock in order to check whether we can re-use
 exclusively for us, while for file folios we need to hold the lock to make the
 folio stable throughout the copy.
 We also hold it even if we are read-faulting, just to make sure it does not go
 away. For this last case, we are already covered by the hugetlb_mutex, but
 in order to bring hugetlb closer to the general faulting path, let us rely on
 the lock instead, as do_read_fault() does.

 Patch#3 is a merely correctness issue if you will, while Patch#4 and
 Patch#5 remove obsolete comments and assumptions that are no longer
 true.

 [1] https://lore.kernel.org/lkml/20250513093448.592150-1-gavinguo@igalia.com/
"

Shame on me, this should have been the cover letter.

-- 
Oscar Salvador
SUSE Labs