>  
>  /*
> - * Returns approximate total of the readers' ->srcu_lock_count[] values
> - * for the rank of per-CPU counters specified by idx.
> + * Computes approximate total of the readers' ->srcu_lock_count[] values
> + * for the rank of per-CPU counters specified by idx, and returns true if
> + * the caller did the proper barrier (gp), and if the count of the locks
> + * matches that of the unlocks passed in.
>   */
> -static unsigned long srcu_readers_lock_idx(struct srcu_struct *ssp, int idx)
> +static bool srcu_readers_lock_idx(struct srcu_struct *ssp, int idx, bool gp, 
> unsigned long unlocks)
>  {
>       int cpu;
> +     unsigned long mask = 0;
>       unsigned long sum = 0;
>  
>       for_each_possible_cpu(cpu) {
>               struct srcu_data *sdp = per_cpu_ptr(ssp->sda, cpu);
>  
>               sum += atomic_long_read(&sdp->srcu_lock_count[idx]);
> +             if (IS_ENABLED(CONFIG_PROVE_RCU))
> +                     mask = mask | READ_ONCE(sdp->srcu_reader_flavor);
>       }
> -     return sum;
> +     WARN_ONCE(IS_ENABLED(CONFIG_PROVE_RCU) && (mask & (mask - 1)),
> +               "Mixed reader flavors for srcu_struct at %ps.\n", ssp);

I am trying to understand the (unlikely) case where synchronize_srcu() is done 
before any
srcu reader lock/unlock lite call is done. Can new SRCU readers fail to observe 
the
updates?


> +     if (mask & SRCU_READ_FLAVOR_LITE && !gp)
> +             return false;

So, srcu_readers_active_idx_check() can potentially return false for very long
time, until the CPU executing srcu_readers_active_idx_check() does
at least one read lock/unlock lite call?

> +     return sum == unlocks;
>  }
>  
>  /*
> @@ -473,6 +482,7 @@ static unsigned long srcu_readers_unlock_idx(struct 
> srcu_struct *ssp, int idx)
>   */
>  static bool srcu_readers_active_idx_check(struct srcu_struct *ssp, int idx)
>  {
> +     bool did_gp = !!(raw_cpu_read(ssp->sda->srcu_reader_flavor) & 
> SRCU_READ_FLAVOR_LITE);

sda->srcu_reader_flavor is only set when CONFIG_PROVE_RCU is enabled. But we
need the reader flavor information for srcu lite variant to work. So, lite
variant does not work when CONFIG_PROVE_RCU is disabled. Am I missing something
obvious here?



- Neeraj

>       unsigned long unlocks;
>  
>       unlocks = srcu_readers_unlock_idx(ssp, idx);
> @@ -482,13 +492,16 @@ static bool srcu_readers_active_idx_check(struct 
> srcu_struct *ssp, int idx)
>        * unlock is counted. Needs to be a smp_mb() as the read side may
>        * contain a read from a variable that is written to before the
>        * synchronize_srcu() in the write side. In this case smp_mb()s
> -      * A and B act like the store buffering pattern.
> +      * A and B (or X and Y) act like the store buffering pattern.
>        *
> -      * This smp_mb() also pairs with smp_mb() C to prevent accesses
> -      * after the synchronize_srcu() from being executed before the
> -      * grace period ends.
> +      * This smp_mb() also pairs with smp_mb() C (or, in the case of X,
> +      * Z) to prevent accesses after the synchronize_srcu() from being
> +      * executed before the grace period ends.
>        */
> -     smp_mb(); /* A */
> +     if (!did_gp)
> +             smp_mb(); /* A */
> +     else
> +             synchronize_rcu(); /* X */
>  
>       /*
>        * If the locks are the same as the unlocks, then there must have
> @@ -546,7 +559,7 @@ static bool srcu_readers_active_idx_check(struct 
> srcu_struct *ssp, int idx)
>        * which are unlikely to be configured with an address space fully
>        * populated with memory, at least not anytime soon.
>        */
> -     return srcu_readers_lock_idx(ssp, idx) == unlocks;
> +     return srcu_readers_lock_idx(ssp, idx, did_gp, unlocks);
>  }
>  


Reply via email to