Thread (65 messages) 65 messages, 6 authors, 2026-08-30

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

flat view

From: NeilBrown <hidden>
Date: 2026-08-24 23:38:31
Also in: linux-arch, linux-block, linux-doc, linux-ext4, linux-fsdevel, linux-i2c, linux-ide, linux-media, linux-mm, linux-modules, linux-nfs, linux-rt-devel, lkml, rcu, rust-for-linux

On Tue, 25 Aug 2026, David Hildenbrand (Arm) wrote:
Hi!
quoted
quoted
quoted
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).
[...]
quoted
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.
quoted
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.
quoted
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).
quoted
 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
  
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help