On Mon, Jul 06, 2026 at 07:05:44PM +0100, Matthew Wilcox wrote:
On Mon, Jul 06, 2026 at 03:19:10PM +0900, Byungchul Park wrote:
Makes dept able to track PG_locked waits and events, which will be useful in practice. See the following link that shows dept worked with PG_locked and detected real issues in practice:
https://lore.kernel.org/lkml/1674268856-31807-1-git-send-email-byungchul.par...
@@ -219,6 +220,7 @@ struct page { struct page *kmsan_shadow; struct page *kmsan_origin; #endif
struct dept_ext_wgen pg_locked_wgen;} _struct_page_alignment;
I may not understand this quite correctly, but I think that tracking PG_locked dependencies in the struct page has both false positive and false negative problems.
Imagine we have a file mapping M1 containing folio F1 at index 0 and F2 at index 1. It is correct locking order to lock F1 before locking F2 (for example when doing writeback). Later, M1 has its folios reclaimed and returned to the free pool. Then each is added to mapping M2, this time with folio F2 at index 8 and F1 at index 9. Now the correct order to lock these folios in the order F2 followed by F1.
First of all, I appreciate your feedback. Thanks!
That case doesn't generate any dependency unless any other waits are involved in. That should be handled in xxx_nested manner e.g. folio_lock_nested() that I need to introduce. The work is in progress.
I don't see a part of this patch where we clear pg_locked_wgen when the page is returned to the page allocator. Maybe I missed that.
You are right. pg_locked_wgen doesn't get cleared. However, DEPT works this way:
folio_lock() wait_for_pg_locked_cleared() set_pg_locked() // (1) update pg_locked_wgen to the current wgen
... // there might be other waits
folio_unlock() clear_pg_locked() // (2) check if there have been any waits since (1)
In other words, it's guranteed that pg_locked_wgen has been updated e.i. (1) when DEPT refers to pg_locked_wgen e.i. (2). So I don't think it's a problem.
I think we should be tracking PG_locked dependencies in the owner of the folio. For files, that would be in the struct address_space. For anon memory, I think that's in the anon_vma, but if somebody told me it was in some other structure, I wouldn't argue with them.
I think it's a good point but it's a classification issue. folios owned by struct address_space should be classified to e.g. address_space_class and ones owned by struct anon_vma should be classified to e.g. anon_vma_class. I will work on it to apply the insight you just gave but better do it as follow-up patches since the initial patchset is already too big to get reviewed.
This requires slightly more complexity than lockdep currently has. We don't want to use a lockdep class for each folio, obviously. So we need something to say "I already have folio F1 locked, is it OK to lock
From DEPT's perspective, folio_lock(F1) and folio_lock(F2) are waits and folio_unlock(F1) and folio_unlock(F2) are events. Since DEPT tracks dependencies with specified classes between waits and events, DEPT's interest in the following example is to detect a situation like:
< context X >
folio_lock(address_space_class'ed F1) ... folio_lock(anon_vma_class'ed F2) ... folio_unlock(anon_vma_class'ed F2) ... folio_unlock(address_space_class'ed F1)
< context Y >
folio_lock(anon_vma_class'ed any folio) ... folio_lock(address_space_class'ed any folio) ... folio_unlock(address_space_class'ed any folio) ... folio_unlock(anon_vma_class'ed any folio)
However, the following pattern should be manually annotated by developers like using folio_lock_nested() or something. DEPT cannot work with it automatically:
folio_lock(address_space_class'ed F1) ... folio_lock(address_space_class'ed F2) ... folio_unlock(address_space_class'ed F2) ... folio_unlock(address_space_class'ed F1)
or
folio_lock(anon_vma_class'ed F1) ... folio_lock(anon_vma_class'ed F2) ... folio_unlock(anon_vma_class'ed F2) ... folio_unlock(anon_vma_class'ed F1)
These should be explicitly annotated by developers if it's intended:
folio_lock(address_space_class'ed F1) ... folio_lock_nested(address_space_class'ed F2) ... folio_unlock(address_space_class'ed F2) ... folio_unlock(address_space_class'ed F1)
or
folio_lock(anon_vma_class'ed F1) ... folio_lock_nested(anon_vma_class'ed F2) ... folio_unlock(anon_vma_class'ed F2) ... folio_unlock(anon_vma_class'ed F1)
folio F2?". Essentially figuring out how we can track all folios in a given mapping the same way, and making sure that we don't deadlock on folios in the same mapping.
At the moment, as I told you, DEPT cannot work with dependencies between the same class'ed folios. However, it'd be much better if DEPT can work with even those cases. Could you provide a scenario where a deadlock happens between the same class'ed ones? Any idea how to detect for the cases?
If F1 and F2 are in different mappings, it's not a deadlock if F1 is in a filesystem mapping and F2 is in its backing dev. It's also not a deadlock if F1 and F2 are both filesystem folios and the inodes are both locked. See vfs_lock_two_folios() in fs/remap_range.c.
Yeah.. DEPT is a tracker to track dependencies between waits and events even across different contexts, but not a magic unfortunately. That lock ordering issue - with the same class'ed ones - should be resolved in the manual manner as vfs_lock_two_folios() does.
I have much less knowledge about anonymous memory locking order. Maybe it doesn't happen. Or about locking one anon and one file folio. For slab memory, we don't sleep on PG_locked (it's used as a spinlock bit). For other kinds of memory ... I don't know. Page migration is fun.
Anyway, the sophisticated classification you mentioned is necessary for DEPT to be better especially for folio locking mechanism.
Thanks again!
Byungchul