On Fri, Dec 30, 2022 at 11:38:21AM +0000, Matthew Wilcox wrote:
>
> <dwmw2> why doesn't lockdep catch us calling synchronize_srcu() with
> a lock held, and elsewhere obtaining that lock within an srcu critical
> region ("read lock") ?
>
> Because synchronize_srcu() doesn't acquire the lock, merely checks that
> it isn't held.
>
> Would this work? Not even compile tested.
>
> You can put my SoB on this if it works.
>
> diff --git a/kernel/rcu/srcutree.c b/kernel/rcu/srcutree.c
> index ca4b5dcec675..e9c2ab8369c0 100644
> --- a/kernel/rcu/srcutree.c
> +++ b/kernel/rcu/srcutree.c
> @@ -1267,11 +1267,11 @@ static void __synchronize_srcu(struct srcu_struct *ssp, bool do_norm)
> {
> struct rcu_synchronize rcu;
>
> - RCU_LOCKDEP_WARN(lockdep_is_held(ssp) ||
> - lock_is_held(&rcu_bh_lock_map) ||
> + rcu_lock_acquire(&ssp->dep_map);
If this does do anything, why doesn't lockdep complain about things
like this?
i1 = srcu_read_lock(&my_srcu);
i2 = srcu_read_lock(&my_srcu);
srcu_read_unlock(&my_srcu, i2);
srcu_read_unlock(&my_srcu, i1);
Or does it in fact complain and I have somehow managed to confuse things
such that rcutorture does not detect such complaints? I would expect
rcutorture_extend_mask_max() to make this happen, but then again, RCU
has confused me in the past.
Thanx, Paul
> + RCU_LOCKDEP_WARN(lock_is_held(&rcu_bh_lock_map) ||
> lock_is_held(&rcu_lock_map) ||
> lock_is_held(&rcu_sched_lock_map),
> - "Illegal synchronize_srcu() in same-type SRCU (or in RCU) read-side critical section");
> + "Illegal synchronize_srcu() in RCU read-side critical section");
>
> if (rcu_scheduler_active == RCU_SCHEDULER_INACTIVE)
> return;
> @@ -1282,6 +1282,7 @@ static void __synchronize_srcu(struct srcu_struct *ssp, bool do_norm)
> __call_srcu(ssp, &rcu.head, wakeme_after_rcu, do_norm);
> wait_for_completion(&rcu.completion);
> destroy_rcu_head_on_stack(&rcu.head);
> + rcu_lock_release(&ssp->dep_map);
>
> /*
> * Make sure that later code is ordered after the SRCU grace
