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

["David Hildenbrand (Arm)" <david@kernel.org>]

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.

[...]

> 
> 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.

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);

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.

> 
> 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.

> 
> 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.


>  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.

-- 
Cheers,

David