Re: [PATCH AUTOSEL 4.20 028/304] srcu: Prevent __call_srcu() counter wrap with read-side critical section

Paul E. McKenney
2019-01-28T16:23:25Z

On Mon, Jan 28, 2019 at 10:39:05AM -0500, Sasha Levin wrote:
> From: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
>
> [ Upstream commit 0607ba8403c4cdb253f8c5200ecf654dfb7790cc ]
>
> Ever since cdf7abc4610a ("srcu: Allow use of Tiny/Tree SRCU from
> both process and interrupt context"), it has been permissible
> to use SRCU read-side critical sections in interrupt context.
> This allows __call_srcu() to use SRCU read-side critical sections to
> prevent a new SRCU grace period from ending before the call to either
> srcu_funnel_gp_start() or srcu_funnel_exp_start completes, thus preventing
> SRCU grace-period counter overflow during that time.
>
> Note that this does not permit removal of the counter-wrap checks in
> srcu_gp_end(). These check are necessary to handle the case where
> a given CPU does not interact at all with SRCU for an extended time
> period.
>
> This commit therefore adds an SRCU read-side critical section to
> __call_srcu() in order to prevent grace period counter wrap during
> the funnel-locking process.
>
> Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
> Signed-off-by: Sasha Levin <sashal@kernel.org>

Hello, Sasha,

I recommend -against- backporting this one. It is a theoretical bug
that requires rather long preemption, so the risk of backporting likely
greatly exceeeds the risk of the bug actually happening, especially on
64-bit systems.

Thanx, Paul

> ---
> kernel/rcu/srcutree.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/kernel/rcu/srcutree.c b/kernel/rcu/srcutree.c
> index a180abc8c925..697a2d7e8e8a 100644
> --- a/kernel/rcu/srcutree.c
> +++ b/kernel/rcu/srcutree.c
> @@ -860,6 +860,7 @@ void __call_srcu(struct srcu_struct *sp, struct rcu_head *rhp,
> rcu_callback_t func, bool do_norm)
> {
> unsigned long flags;
> + int idx;
> bool needexp = false;
> bool needgp = false;
> unsigned long s;
> @@ -873,6 +874,7 @@ void __call_srcu(struct srcu_struct *sp, struct rcu_head *rhp,
> return;
> }
> rhp->func = func;
> + idx = srcu_read_lock(sp);
> local_irq_save(flags);
> sdp = this_cpu_ptr(sp->sda);
> spin_lock_rcu_node(sdp);
> @@ -894,6 +896,7 @@ void __call_srcu(struct srcu_struct *sp, struct rcu_head *rhp,
> srcu_funnel_gp_start(sp, sdp, s, do_norm);
> else if (needexp)
> srcu_funnel_exp_start(sp, sdp->mynode, s);
> + srcu_read_unlock(sp, idx);
> }
>
> /**
> --
> 2.19.1
>