On Thu, Jul 11, 2019 at 07:43:56PM -0400, Joel Fernandes (Google) wrote:
> +int rcu_read_lock_any_held(void)
> +{
> +     int lockdep_opinion = 0;
> +
> +     if (!debug_lockdep_rcu_enabled())
> +             return 1;
> +     if (!rcu_is_watching())
> +             return 0;
> +     if (!rcu_lockdep_current_cpu_online())
> +             return 0;
> +
> +     /* Preemptible RCU flavor */
> +     if (lock_is_held(&rcu_lock_map))

you forgot debug_locks here.

> +             return 1;
> +
> +     /* BH flavor */
> +     if (in_softirq() || irqs_disabled())

I'm not sure I'd put irqs_disabled() under BH, also this entire
condition is superfluous, see below.

> +             return 1;
> +
> +     /* Sched flavor */
> +     if (debug_locks)
> +             lockdep_opinion = lock_is_held(&rcu_sched_lock_map);
> +     return lockdep_opinion || !preemptible();

that !preemptible() turns into:

  !(preempt_count()==0 && !irqs_disabled())

which is:

  preempt_count() != 0 || irqs_disabled()

and already includes irqs_disabled() and in_softirq().

> +}

So maybe something lke:

        if (debug_locks && (lock_is_held(&rcu_lock_map) ||
                            lock_is_held(&rcu_sched_lock_map)))
                return true;

        return !preemptible();


Reply via email to