FAILED: patch "[PATCH] xfrm: policy: Read seqcount outside of rcu-read side in" failed to apply to 4.14-stable tree
Subsystems (3)
[email protected]
maintainerThe patch below does not apply to the 4.14-stable tree.
If someone wants it applied there, or to any other stable or longterm tree, then please email the backport, including the original git commit id to <[email protected]>.
thanks,
greg k-h
------------------ original commit in Linus's tree ------------------
From d7b0408934c749f546b01f2b33d07421a49b6f3e Mon Sep 17 00:00:00 2001
From: Varad Gautam <[email protected]>
Date: Fri, 28 May 2021 18:04:06 +0200
Subject: [PATCH] xfrm: policy: Read seqcount outside of rcu-read side in
xfrm_policy_lookup_bytype
xfrm_policy_lookup_bytype loops on seqcount mutex xfrm_policy_hash_generation within an RCU read side critical section. Although ill advised, this is fine if the loop is bounded.
xfrm_policy_hash_generation wraps mutex hash_resize_mutex, which is used to serialize writers (xfrm_hash_resize, xfrm_hash_rebuild). This is fine too.
On PREEMPT_RT=y, the read_seqcount_begin call within xfrm_policy_lookup_bytype emits a mutex lock/unlock for hash_resize_mutex. Mutex locking is fine, since RCU read side critical sections are allowed to sleep with PREEMPT_RT.
xfrm_hash_resize can, however, block on synchronize_rcu while holding hash_resize_mutex.
This leads to the following situation on PREEMPT_RT, where the writer is blocked on RCU grace period expiry, while the reader is blocked on a lock held by the writer:
Thead 1 (xfrm_hash_resize) Thread 2 (xfrm_policy_lookup_bytype)
rcu_read_lock(); mutex_lock(&hash_resize_mutex); read_seqcount_begin(&xfrm_policy_hash_generation); mutex_lock(&hash_resize_mutex); // block
xfrm_bydst_resize();synchronize_rcu(); // block
<RCU stalls in xfrm_policy_lookup_bytype>
Move the read_seqcount_begin call outside of the RCU read side critical section, and do an rcu_read_unlock/retry if we got stale data within the critical section.
On non-PREEMPT_RT, this shortens the time spent within RCU read side critical section in case the seqcount needs a retry, and avoids unbounded looping.
net/xfrm/xfrm_policy.c +14 -7
@@ -2092,12 +2092,15 @@ static struct xfrm_policy *xfrm_policy_lookup_bytype(struct net *net, u8 type,if (unlikely (!daddr || !saddr))return NULL ; -rcu_read_lock (); retry: -do { - sequence =read_seqcount_begin (&xfrm_policy_hash_generation); - chain =policy_hash_direct (net, daddr, saddr, family, dir); - }while (read_seqcount_retry (&xfrm_policy_hash_generation, sequence)); + sequence =read_seqcount_begin (&xfrm_policy_hash_generation); +rcu_read_lock (); + + chain =policy_hash_direct (net, daddr, saddr, family, dir); +if (read_seqcount_retry (&xfrm_policy_hash_generation, sequence)) { +rcu_read_unlock (); + goto retry; + } ret =NULL ;hlist_for_each_entry_rcu (pol, chain, bydst) {
@@ -2128,11 +2131,15 @@ static struct xfrm_policy *xfrm_policy_lookup_bytype(struct net *net, u8 type, } skip_inexact: -if (read_seqcount_retry (&xfrm_policy_hash_generation, sequence)) +if (read_seqcount_retry (&xfrm_policy_hash_generation, sequence)) { +rcu_read_unlock (); goto retry; + } -if (ret && !xfrm_pol_hold_rcu (ret)) +if (ret && !xfrm_pol_hold_rcu (ret)) { +rcu_read_unlock (); goto retry; + } fail:rcu_read_unlock ();