On 8/20/26 19:51, Matthew Wilcox wrote: > On Thu, Aug 20, 2026 at 07:16:05PM +0200, David Hildenbrand (Arm) wrote: >>> Consider this real deadlock pattern that lockdep cannot detect: >>> >>> context X context Y context Z >>> >>> mutex_lock A >>> folio_lock B >>> folio_lock B <- DEADLOCK >>> mutex_lock A <- DEADLOCK >>> folio_unlock B >>> folio_unlock B >>> mutex_unlock A >>> mutex_unlock A >> >> But that really just boils down to folio lock being implemented as a PG_lock >> + >> some advanced wait mechanism. And we must do that because of lack of bits in >> struct page. >> >> Willy mentioned in a previous version [1]: "I don't think it makes sense to >> track lock state in the page (nor folio). Partly because there's just so >> many >> of them, but also because the locking rules don't really apply to individual >> folios so much as they do to the mappings (or anon_vmas) that contain >> folios." >> >> Given that lockdep is a debug feature, and we will at some point allocate >> struct >> folio separately, I assume we could just squeeze a "struct lockdep_map" in >> there >> in such debug configs and the world would not collapse. >> >> Doing that today (one "struct lockdep_map" in each "struct page") wouldn't >> work >> as mm_zero_struct_page() would not expect such large "struct page". But >> conceptually, for a debug kernel with a special CONFIG_LOCKDEP_PAGE_LOCK, >> maybe >> that would already be ok and we could just do that (and optimize it as we >> allocate folios separately). >> >> Not that it's ideal, but for a debug feature to at least check PG_lock, >> probably >> an easier way to achieve it than some completely new infrastructure. >> >> Now, Willy said "locking rules don't really apply to individual folios", I >> wonder if that could just help to also let lockdep check PG_lock with less >> metadata? (didn't fully wrap my head around the implications) >> >> [1] >> https://lore.kernel.org/all/[email protected]/?utm_source=chatgpt.com > > There are a few things going on that make PG_lock special. Let me try > to explain again, only better this time. >
Thanks for that information :) > 1. The current lifetime of a struct page is the lifetime of the system. > But the semantics of its PG_lock bit change each time it is freed and > allocated. Well, yeah, but that will effectively change once we dynamically allocate struct folio. Until then, don't we have the following two events: (a) Allocator handing out a page (b) Page getting returned to page allocator And while in the allocator, the PG_locked is essentially unused/unusable (and so would be lockdep data)? I'd argue there is a well defined lifetime for PG_locked and any additional lockdep metadata. At least theoretically ;) > > 2. The position of PG_lock in the locking hierarchy only depend on > what the folio is currently being used for. That is, all folios in > a given xfs inode behave exactly the same from a locking perspective. > There's no need to build up state about how each PG_lock is used; > they can all share. Arguably all xfs file inodes are the same as > each other (directory inodes might be different from file inodes), > so we might want to go further than telling DEPT that "this folio > belongs to this inode" and go to "this folio belongs to this xfs file > inode". "There is no need", I am not entirely sure when it comes to anonymous folios. Even if there is actually no need, given that we are talking about debugging mechanisms, I am not sure if optimizing for less memory consumption is our highest priority. Debug kernels already consume plenty of memory on other things (page_ext, for example), so consuming some more to support the folio lock with lockdep does not sound too crazy for me. (maybe lockdep data could even life in page_ext) I understand that Dept applies to more things than just PG_locked, though. > > 3. PG_lock can be taken in task context then released in interrupt > context. For full points, we need to mark the exact point at which > we submit the folio for read. Otherwise we can get into the situation > alluded to by f2c817bed58d and better discussed at > https://lore.kernel.org/linux-mm/[email protected]/ > where we have the folio locked but haven't yet submitted it for I/O > so it doesn't matter how long we wait, it will never come unlocked. Yeah, it seems hard to squeeze different locking contexts into existing lockdep. I cannot tell whether lockdep could be adjusted to allow for that, or how much work it would involve. So yeah, Dept might be a more natural fit for this scenario. -- Cheers, David
