Re: [PATCH v6 00/16] locking/lockdep: Add support for dynamic keys
From: Bart Van Assche
Date: Sun Feb 03 2019 - 12:36:48 EST
On 2/1/19 4:15 AM, Peter Zijlstra wrote:
On Fri, Jan 18, 2019 at 06:34:20PM -0800, Bart Van Assche wrote:
I agree with what you wrote. The only code I know of that accesses list
entries using RCU is the __bfs() function. In that function I found the
following loop:
list_for_each_entry_rcu(entry, head, entry) { [ ... ] }
Thing is; I can't seem to find any __bfs() usage outside of graph_lock.
count_{fwd,bwd}_deps() - takes graph lock
check_{noncircular,redudant}() - called from check_prev_add() <-
check_prevs_add() <- validate_chain() which takes graph lock
find_usage{,_fwd,_bwd}
<- check_usage() <- check_irq_usage() <- check_prev_add_irq() <-
check_prev_add <- check_prevs_add() <- validate_chain() which takes
graph lock
<- check_usage_{fwd,bdw}() <- mark_lock_irq() <- mark_lock() which
takes graph lock
Or did I miss something? If there are no __bfs() users outside of graph
lock, then we can simply remove that _rcu from the iteration, and
simplify all that.
Every time I make a single change to the lockdep code I have to rerun my
test case for a week to make sure that no regressions have been
introduced. In other words, I can make further changes but that could
take some time. Do you want me to look into this simplification now or
after this patch series went upstream?
Since zap_class() calls list_del_rcu(&entry->entry), since a grace period
occurs between the call_rcu() invocation and the RCU callback function,
since at least an RCU reader lock must be held around RCU loops and since
sleeping is not allowed while holding an RCU read lock I think there is
no risk that __bfs() will examine a list entry after it has been freed.
So you agree that list_entry_being_freed() should only check the current
pf?
Sorry if I wasn't clear enough. In a previous e-mail I tried to explain
that both pf's have to be checked. Another way to explain that is as
follows:
- Each list entry has one of the following states: free, in use or being
freed.
- "Free" means that the corresponding bit in the list_entries_in_use
bitmap has not been set.
- "In use" means that the corresponding bit in the list_entries_in_use
bitmap has been set and that none of the corresponding bits in the
list_entries_being_freed bitmaps have been set.
- "Being freed" means that the corresponding bit in one of the
list_entries_being_freed bitmaps has been set.
Since it can happen that multiple elements of the pending_free[] array
are in the state where call_rcu() has been called but the RCU callback
function has not yet been called, I think that zap_class() must check
the list_entries_being_freed bitmaps in all pending_free[] array elements.
Thanks,
Bart.