Re: [PATCH v19 00/40] DEPT(DEPendency Tracker)

On Tue, 25 Aug 2026, David Hildenbrand (Arm) wrote:
> Hi!
> 
> >>>
> >>> 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.
> 
> That would be even better.

A little bit more on this:
 lockdep *does* store per-lock information - a "struct lockdep_map".
 I don't fully understand how it is used, but it seems to be mostly
 for convenience and performance.
 When you lock something, lockdep need to know what class the lock
 belongs to.  The lockdep_map maps from the lock to the class so the
 caller doesn't have to explicitly identify the class every time.
 The lockdep_map also contains a "name" and some wait_types which I
 *think* are only used for more descriptive error messages.
 There is also a "class_cache" which makes it easier (faster) for lockdep to
 find the target class for different subclasses.

 rhashtables "bucket_table" is an existence proof there there doesn't
 need to be a lockdep_map for each lock.  Every bucket has its own
 bit-spin-lock, but there is only one lockdep_map that they all shared.

 You could conceivably put a lockdep_map in each "struct address_space"
 for folio locks (assuming all lockable folios still belong to some
 address_space).

> 
> [...]
> 
> > 
> > maybe you put the folio which is locked on a queue or an lru or
> > whatever.
> 
> For readahead, I think that queues / lru are not involved. We submit the I/O,
> and once the I/O is done, we unlock the folio from interrupt context.

"submit the I/O" means "attach the page to a "struct bio" (or similar)
and attach the struct bio to a transmit queue for the device (or
similar).  So there really is a queue.

> 
> end_buffer_async_read() / iomap_finish_folio_read() end up calling
> folio_end_read(), where we do the magic
> 
> 	folio_wake_bit(folio, PG_locked);

Exactly where the lock ownership should be reclaimed is not immediately
clear to me.  bio_endio() might be early enough but there are probably
better points.

A small difficulty here is that a bio has multiple folios and they are
all locked.  I cannot see that lockdep has a concept of holding an
arbitrarily large set of related locks.
We could just tell lockdep 
  "I have some folios locked" 
or maybe enhance lockdep to allow
  "I have N folios locked"
or even
  "I have N folios in address-space A with the highest offset being O".

This would allow lockdep to check the validity of locking another folio
- only allowed if the address space is the same and the offset is larger
than the previous largest.

> 
> It's interesting, that for writeback we never unlock the folio on the I/O path,
> but instead have the dedicated page flag to tell us when writeback is complete.

PG_locked and PG_writeback are two different lock-bits with two
different meanings.
When a folio is PG_locked the content is invalid and shouldn't be accessed.
When a folio is in PG_writeback its data is not safe even though
PG_dirty has been cleared (if I remember correctly).

So these are two separate, though related, lock bits that would both
need to be tracked.  lockdep would detect how they are related.

> 
> > 
> > There is no way to say "that queue owns this lock".  Maybe that could
> > usefully be added - assuming coherent semantics can be designed.
> > 
> > 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)
> 
> I'd assume that's what the unlock path would do from interrupt context.

exactly.

> 
> > 
> > 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.
> 
> I guess on the interrupt path, we primarily unlock the folio lock only. But we
> might take some other spinlocks temporarily indeed, like
> iomap_finish_folio_read() does.

Yes.  It is hard to see there being locking problems in interrupt
handlers as the folio locks are sleeping locks and an interrupt handler
can only take atomic locks, so getting the ordering wrong is likely
impossible.

You would be more likely to see problems in a workqueue which takes the
completely IO and does something with it, maybe verify a checksum or
whatever (it is a long time since I've worked in the block layer).

> 
> 
> >  But I'd like to see a coherent
> > explanation of how the functionality offered by DEPT is clearly better.
> 
> Yes, that's also what I am missing.
> 
> Stating that lockdep is stable and should not be destabilized is not really a
> good argument.

No it is not.  We change "stable" things all the time.  Replacing one
big thing with another big thing is almost always rejected.
Incremental development is the preferred approach.
So developing something like DEPT to explore and learn and demonstrate
a cool idea is great.  Making it completely separate can give a
developer freedom to innovate.
But the path from there to upstream acceptance is to take the lessons
learned and to apply them one by one to the existing tools.
Yes, that takes longer.  Yes it can be frustrating.  Yes you need to
be able to work with people - not just code - to bring the community
along with you.
But the result is generally much better.

Thanks,
NeilBrown


> 
> -- 
> Cheers,
> 
> David
>