On Tue, 25 Aug 2026, Byungchul Park wrote:
> On Fri, Aug 21, 2026 at 07:48:40PM +1000, NeilBrown wrote:
> > On Fri, 21 Aug 2026, David Hildenbrand (Arm) wrote:
> > > [...]
> > >
> > > > Exactly.  That's why we use classification e.g. lock class - DEPT also
> > > > makes use of the concept.
> > > >
> > > > DEPT doesn't use a full map in each page but uses a minimum space for a
> > > > timestamp in each to track when each starts to wait so as to use the
> > > > recorded timestamp when the event occurs e.g. folio_unlock().
> > >
> > > Thanks for that information!
> > >
> > > >
> > > >> 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.
> > > >
> > > > That's a good news for lockdep.  (And even for DEPT :)
> > >
> > > He :) Where do you currently store the additional per-page information?
> > 
> > lockdep doesn't need to store per-page information.  Possibly DEPT
> > doesn't either.
> 
> Class can be stored in global map, but DEPT needs to keep a timestamp
> in each page to track when a potential-wait e.g. folio_lock() has been
> started and to refer to the information on its event e.g. folio_unlock().

A timestamp?  What does that do?
There is a lot of detail in your cover letter, but I think there is a
lot missing too.  I would like to understand how DEPT works, but I don't
want to dig too hard.  The document in patch 25/40 helps a bit, but it
is still a bit vague.
Where is the document which would explain at a level that would help me
understand this timestamp?

> 
> > lockdep needs one lockdep_map for each lock class.  It would make sense
> > for all folio locks to use the same global lockdep_map.
> 
> Exactly.  Only considering classes, you are right.
> 
> > Each specific lock is known to lockdep as a task which holds the lock, a
> > lockdep_map which represents the class of locks, and subclass number
> > which allows a given task to hold multiple locks of the same class
> > providing it declare (e.g. with spin_lock_nested() etc).
> 
> Right, lockdep works that way.
> 
> > > >> 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).
> > > >
> > > > Sounds great.
> > > >
> > > >> 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.
> > > >
> > > > I understand what you are going to tell.
> > > >
> > > > However, it's worth noting that lockdep tracks dependencies basically
> > > > based on **lock acqusition orders** in the system.  To make it track
> > > > even rwlock and general synchronization mechanism as well, lockdep has
> > > > no choice but to get more complicated.
> > >
> > > Well, yes, sure :)
> > >
> > > >
> > > > Focusing on only the dependency checking, the most parts of lockdep are
> > > > for the tricky things, so the reusable parts are not that big.
> > > >
> > > >> 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)
> > > >
> > > > That's what DEPT did and what brought external wgen introduced in DEPT.
> > > > I was considering the exactly same thing :)
> > > >
> > > > Again, lockdep that tracks lock acquisition orders can't do that.
> > > >
> > > >> [1]
> > > >> https://lore.kernel.org/all/[email protected]/?utm_source=chatgpt.com
> > > >>
> > > >>
> > > >> It's your guiding example, that's why I mention it. You do mention 
> > > >> other wait
> > > >> cases here, I don't know anything about them, but for folios it's 
> > > >> really just
> > > >> "we used a single bit so far" AFAIKs.
> > > >
> > > > It doesn't matter whether it's implemented using bit or not.  folio lock
> > > > is quite special since it's allowed to be released other than the
> > > > acquisition context that makes lockdep impossible to track them.
> > >
> > > Does that really make lockdep *impossible* to track them? IOW, there is 
> > > no way
> > > to extend lockdep to support lock release in different context?
> > 
> > Yes and no....
> > 
> > lockdep has no knowledge of control flows moving across threads in the
> > way that I assume DEPT does.  But it should be possible to tell it.
> > 
> > If you have some code that takes a lock and then hands it off to
> > another thread, at the hand-off point you call
> >    lock_map_release(&the_lock_map)
> > 
> > This says "no task owns this lock any more".
> > 
> > maybe you put the folio which is locked on a queue or an lru or
> > whatever.
> > 
> > There is no way to say "that queue owns this lock".  Maybe that could
> > usefully be added - assuming coherent semantics can be designed.
> 
> It's certainly useful to track who owns the lock, but the essence, when
> it comes to deadlock detection, lies elsewhere.  DEPT is based on the
> essence, that is, a deadlock comes from waits that are never awakened.

I disagree.
The "essence" of deadlock detection involves the interplay of ownership
and waiting.
In your cover letter "THE DEPT APPROACH" talks about [S] where and
event context begins, and [E] where the even occurs.  This S-E section
is exactly ownership.  During that time something "owns" the event.
Maybe the terminology is different, but the concept is the same.

> 
> > Somewhere else some other task takes responsibility for that folio and
> > the lock.  maybe it dequeues a page, or maybe an lru callback gives the
> > locked page to some code.
> > That code then calls
> >    lock_map_acquire_try(&the_lock_map)
> > 
> > This says "this task is now holding this lock" (or more accurately "now
> > holding a lock of this class").
> > Note the "_try" - that says that the task didn't have to wait for the
> > lock, it just got it for free, which in fact it did.
> > 
> > Now if that task takes some other lock, lockdep will see a dependency
> > between the page lock and the new lock, and will accept or reject it as
> > you would expect.
> 
> It's an interesting approach if your goal is to track the owenership,
> but if the goal is for tracking dependencies.. well.. I'm not sure.

I am sure.  Dependencies are between waiters and owners.

A strong theme in your cover letter is the need to add comprehensive
annotation for DEPT to work.
My position is that most of what we need for LOCKDEP to be able to
detect exactly the same things is appropriate annotation.
We need to annotate when ownership is handed off to another thread, and
when the other thread takes on ownership.

So what does your scheme provide that is clearly better?  Is the
annotation easier?  Is some of it automatic?  Does if give better error
reports? 

I had a look through dept.rst and it seems that if a lock is released by
a different thread than the one which took the lock, then DEPT can
detect that and do something useful.  That might be interesting.
But how does it know when it first took ownership of that lock?  Or you
use your terminology: how does it know when that lock first entered the
context in that thread?

Maybe when you see an "unlock", you assume the lock has been owned since
the last time the context didn't hold hold any locks.  Would that be
right? 
Surely it wouldn't the too hard to teach lockdep about that.
i.e.  when a lock, which is allowed to migrate across threads, is
released though it wasn't held - insert this lock at the top of the
current context and re-assess all the wait points.

How do you decide that a lock that was taken is no longer part of the
current context?


NeilBrown

Reply via email to