[PATCH RT 6/7] fs/dcache: incremental fixup of the retry routine

[PATCH RT 6/7] fs/dcache: incremental fixup of the retry routine

[Steven Rostedt]

[-- Attachment #1: 0006-fs-dcache-incremental-fixup-of-the-retry-routine.patch --]
[-- Type: text/plain, Size: 1216 bytes --]

3.4.112-rt144-rc1 stable review patch.
If anyone has any objections, please let me know.

------------------

From: Sebastian Andrzej Siewior <bigeasy@linutronix.de>

It has been pointed out by tglx that on UP the non-RT task could spin
its entire time slice because the lock owner is preempted. This won't
happen on !RT. So we back to "chill" if we can't cond_resched() did not
work.

Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
---
 fs/dcache.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/fs/dcache.c b/fs/dcache.c
index 408551d483b7..e0a439c13318 100644
--- a/fs/dcache.c
+++ b/fs/dcache.c
@@ -38,8 +38,6 @@
 #include <linux/prefetch.h>
 #include <linux/ratelimit.h>
 #include <linux/delay.h>
-#include <linux/sched/rt.h>
-#include <linux/sched/deadline.h>
 #include "internal.h"
 #include "mount.h"
 
@@ -575,10 +573,11 @@ kill_it:
 		if (parent == dentry) {
 			/* the task with the highest priority won't schedule */
 			r = cond_resched();
-			if (!r && (rt_task(current) || dl_task(current)))
+			if (!r)
 				cpu_chill();
-		} else
+		} else {
 			dentry = parent;
+		}
 		goto repeat;
 	}
 }
-- 
2.8.1

[PATCH RT 0/7] Linux 3.2.82-rt119-rc1

[Steven Rostedt]

Dear RT Folks,

This is the RT stable review cycle of patch 3.2.82-rt119-rc1.

Please scream at me if I messed something up. Please test the patches too.

The -rc release will be uploaded to kernel.org and will be deleted when
the final release is out. This is just a review release (or release candidate).

The pre-releases will not be pushed to the git repository, only the
final release is.

If all goes well, this patch will be converted to the next main release
on 9/25/2016.

Enjoy,

-- Steve


To build 3.2.82-rt119-rc1 directly, the following patches should be applied:

  http://www.kernel.org/pub/linux/kernel/v3.x/linux-3.2.tar.xz

  http://www.kernel.org/pub/linux/kernel/v3.x/patch-3.2.82.xz

  http://www.kernel.org/pub/linux/kernel/projects/rt/3.2/patch-3.2.82-rt119-rc1.patch.xz

You can also build from 3.2.82-rt118 by applying the incremental patch:

http://www.kernel.org/pub/linux/kernel/projects/rt/3.2/incr/patch-3.2.82-rt118-rt119-rc1.patch.xz


Changes from 3.2.82-rt118:

---


Sebastian Andrzej Siewior (6):
      timers: wakeup all timer waiters
      timers: wakeup all timer waiters without holding the base lock
      net: add back the missing serialization in ip_send_unicast_reply()
      net: add a lock around icmp_sk()
      fs/dcache: resched/chill only if we make no progress
      fs/dcache: incremental fixup of the retry routine

Steven Rostedt (Red Hat) (1):
      Linux 3.2.82-rt119-rc1

----
 fs/dcache.c         | 17 +++++++++++++++--
 kernel/timer.c      |  4 ++--
 localversion-rt     |  2 +-
 net/ipv4/icmp.c     |  8 ++++++++
 net/ipv4/tcp_ipv4.c |  7 +++++++
 5 files changed, 33 insertions(+), 5 deletions(-)

[PATCH RT 1/7] timers: wakeup all timer waiters

[Steven Rostedt]

[-- Attachment #1: 0001-timers-wakeup-all-timer-waiters.patch --]
[-- Type: text/plain, Size: 1364 bytes --]

3.2.82-rt119-rc1 stable review patch.
If anyone has any objections, please let me know.

------------------

From: Sebastian Andrzej Siewior <bigeasy@linutronix.de>

The base lock is dropped during the invocation if the timer. That means
it is possible that we have one waiter while timer1 is running and once
this one finished, we get another waiter while timer2 is running. Since
we wake up only one waiter it is possible that we miss the other one.
This will probably heal itself over time because most of the time we
complete timers without an active wake up.
To avoid the scenario where we don't wake up all waiters at once,
wake_up_all() is used.

Cc: stable-rt@vger.kernel.org
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
---
 kernel/timer.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/timer.c b/kernel/timer.c
index badd2d2066dc..3beac326b447 100644
--- a/kernel/timer.c
+++ b/kernel/timer.c
@@ -932,7 +932,7 @@ static void wait_for_running_timer(struct timer_list *timer)
 			   base->running_timer != timer);
 }
 
-# define wakeup_timer_waiters(b)	wake_up(&(b)->wait_for_running_timer)
+# define wakeup_timer_waiters(b)	wake_up_all(&(b)->wait_for_running_timer)
 #else
 static inline void wait_for_running_timer(struct timer_list *timer)
 {
-- 
2.8.1

[PATCH RT 2/7] timers: wakeup all timer waiters without holding the base lock

[Steven Rostedt]

[-- Attachment #1: 0002-timers-wakeup-all-timer-waiters-without-holding-the-.patch --]
[-- Type: text/plain, Size: 951 bytes --]

3.2.82-rt119-rc1 stable review patch.
If anyone has any objections, please let me know.

------------------

From: Sebastian Andrzej Siewior <bigeasy@linutronix.de>

There should be no need to hold the base lock during the wakeup. There
should be no boosting involved, the wakeup list has its own lock so it
should be safe to do this without the lock.

Cc: stable-rt@vger.kernel.org
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
---
 kernel/timer.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/timer.c b/kernel/timer.c
index 3beac326b447..9e1b9db4c8b8 100644
--- a/kernel/timer.c
+++ b/kernel/timer.c
@@ -1185,8 +1185,8 @@ static inline void __run_timers(struct tvec_base *base)
 			spin_lock_irq(&base->lock);
 		}
 	}
-	wakeup_timer_waiters(base);
 	spin_unlock_irq(&base->lock);
+	wakeup_timer_waiters(base);
 }
 
 #ifdef CONFIG_NO_HZ
-- 
2.8.1

[PATCH RT 4/7] net: add a lock around icmp_sk()

[Steven Rostedt]

[-- Attachment #1: 0004-net-add-a-lock-around-icmp_sk.patch --]
[-- Type: text/plain, Size: 2321 bytes --]

3.2.82-rt119-rc1 stable review patch.
If anyone has any objections, please let me know.

------------------

From: Sebastian Andrzej Siewior <bigeasy@linutronix.de>

It looks like the this_cpu_ptr() access in icmp_sk() is protected with
local_bh_disable(). To avoid missing serialization in -RT I am adding
here a local lock. No crash has been observed, this is just precaution.

Cc: stable-rt@vger.kernel.org
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
---
 net/ipv4/icmp.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/net/ipv4/icmp.c b/net/ipv4/icmp.c
index 028eb47226e8..e232225f7446 100644
--- a/net/ipv4/icmp.c
+++ b/net/ipv4/icmp.c
@@ -76,6 +76,7 @@
 #include <linux/string.h>
 #include <linux/netfilter_ipv4.h>
 #include <linux/slab.h>
+#include <linux/locallock.h>
 #include <net/snmp.h>
 #include <net/ip.h>
 #include <net/route.h>
@@ -202,6 +203,8 @@ static const struct icmp_control icmp_pointers[NR_ICMP_TYPES+1];
  *
  *	On SMP we have one ICMP socket per-cpu.
  */
+static DEFINE_LOCAL_IRQ_LOCK(icmp_sk_lock);
+
 static struct sock *icmp_sk(struct net *net)
 {
 	return net->ipv4.icmp_sk[smp_processor_id()];
@@ -213,12 +216,14 @@ static inline struct sock *icmp_xmit_lock(struct net *net)
 
 	local_bh_disable();
 
+	local_lock(icmp_sk_lock);
 	sk = icmp_sk(net);
 
 	if (unlikely(!spin_trylock(&sk->sk_lock.slock))) {
 		/* This can happen if the output path signals a
 		 * dst_link_failure() for an outgoing ICMP packet.
 		 */
+		local_unlock(icmp_sk_lock);
 		local_bh_enable();
 		return NULL;
 	}
@@ -228,6 +233,7 @@ static inline struct sock *icmp_xmit_lock(struct net *net)
 static inline void icmp_xmit_unlock(struct sock *sk)
 {
 	spin_unlock_bh(&sk->sk_lock.slock);
+	local_unlock(icmp_sk_lock);
 }
 
 /*
@@ -298,6 +304,7 @@ static void icmp_push_reply(struct icmp_bxm *icmp_param,
 	struct sock *sk;
 	struct sk_buff *skb;
 
+	local_lock(icmp_sk_lock);
 	sk = icmp_sk(dev_net((*rt)->dst.dev));
 	if (ip_append_data(sk, fl4, icmp_glue_bits, icmp_param,
 			   icmp_param->data_len+icmp_param->head_len,
@@ -320,6 +327,7 @@ static void icmp_push_reply(struct icmp_bxm *icmp_param,
 		skb->ip_summed = CHECKSUM_NONE;
 		ip_push_pending_frames(sk, fl4);
 	}
+	local_unlock(icmp_sk_lock);
 }
 
 /*
-- 
2.8.1

[PATCH RT 5/7] fs/dcache: resched/chill only if we make no progress

[Steven Rostedt]

[-- Attachment #1: 0005-fs-dcache-resched-chill-only-if-we-make-no-progress.patch --]
[-- Type: text/plain, Size: 2283 bytes --]

3.2.82-rt119-rc1 stable review patch.
If anyone has any objections, please let me know.

------------------

From: Sebastian Andrzej Siewior <bigeasy@linutronix.de>

Upstream commit 47be61845c77 ("fs/dcache.c: avoid soft-lockup in
dput()") changed the condition _when_ cpu_relax() / cond_resched() was
invoked. This change was adapted in -RT into mostly the same thing
except that if cond_resched() did nothing we had to do cpu_chill() to
force the task off CPU for a tiny little bit in case the task had RT
priority and did not want to leave the CPU.
This change resulted in a performance regression (in my testcase the
build time on /dev/shm increased from 19min to 24min). The reason is
that with this change cpu_chill() was invoked even dput() made progress
(dentry_kill() returned a different dentry) instead only if we were
trying this operation on the same dentry over and over again.

This patch brings back to the old behavior back to cond_resched() &
chill if we make no progress. A little improvement is to invoke
cpu_chill() only if we are a RT task (and avoid the sleep otherwise).
Otherwise the scheduler should remove us from the CPU if we make no
progress.

Cc: stable-rt@vger.kernel.org
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
---
 fs/dcache.c | 18 ++++++++++++++++--
 1 file changed, 16 insertions(+), 2 deletions(-)

diff --git a/fs/dcache.c b/fs/dcache.c
index 0089bd3c86ce..bd9bd649c390 100644
--- a/fs/dcache.c
+++ b/fs/dcache.c
@@ -38,6 +38,8 @@
 #include <linux/prefetch.h>
 #include <linux/ratelimit.h>
 #include <linux/delay.h>
+#include <linux/sched/rt.h>
+#include <linux/sched/deadline.h>
 #include "internal.h"
 
 /*
@@ -465,6 +467,8 @@ relock:
  */
 void dput(struct dentry *dentry)
 {
+	struct dentry *parent;
+
 	if (!dentry)
 		return;
 
@@ -502,9 +506,19 @@ repeat:
 	return;
 
 kill_it:
-	dentry = dentry_kill(dentry, 1);
-	if (dentry)
+	parent = dentry_kill(dentry, 1);
+	if (parent) {
+		int r;
+
+		if (parent == dentry) {
+			/* the task with the highest priority won't schedule */
+			r = cond_resched();
+			if (!r && (rt_task(current) || dl_task(current)))
+				cpu_chill();
+		} else
+			dentry = parent;
 		goto repeat;
+	}
 }
 EXPORT_SYMBOL(dput);
 
-- 
2.8.1

[PATCH RT 6/7] fs/dcache: incremental fixup of the retry routine

[Steven Rostedt]

[-- Attachment #1: 0006-fs-dcache-incremental-fixup-of-the-retry-routine.patch --]
[-- Type: text/plain, Size: 1199 bytes --]

3.2.82-rt119-rc1 stable review patch.
If anyone has any objections, please let me know.

------------------

From: Sebastian Andrzej Siewior <bigeasy@linutronix.de>

It has been pointed out by tglx that on UP the non-RT task could spin
its entire time slice because the lock owner is preempted. This won't
happen on !RT. So we back to "chill" if we can't cond_resched() did not
work.

Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
---
 fs/dcache.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/fs/dcache.c b/fs/dcache.c
index bd9bd649c390..bea5589bc957 100644
--- a/fs/dcache.c
+++ b/fs/dcache.c
@@ -38,8 +38,6 @@
 #include <linux/prefetch.h>
 #include <linux/ratelimit.h>
 #include <linux/delay.h>
-#include <linux/sched/rt.h>
-#include <linux/sched/deadline.h>
 #include "internal.h"
 
 /*
@@ -513,10 +511,11 @@ kill_it:
 		if (parent == dentry) {
 			/* the task with the highest priority won't schedule */
 			r = cond_resched();
-			if (!r && (rt_task(current) || dl_task(current)))
+			if (!r)
 				cpu_chill();
-		} else
+		} else {
 			dentry = parent;
+		}
 		goto repeat;
 	}
 }
-- 
2.8.1

[PATCH RT 7/7] Linux 3.2.82-rt119-rc1

[Steven Rostedt]

[-- Attachment #1: 0007-Linux-3.2.82-rt119-rc1.patch --]
[-- Type: text/plain, Size: 414 bytes --]

3.2.82-rt119-rc1 stable review patch.
If anyone has any objections, please let me know.

------------------

From: "Steven Rostedt (Red Hat)" <rostedt@goodmis.org>

---
 localversion-rt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/localversion-rt b/localversion-rt
index 4e32122c6b30..bd54f56d15eb 100644
--- a/localversion-rt
+++ b/localversion-rt
@@ -1 +1 @@
--rt118
+-rt119-rc1
-- 
2.8.1

Re: [PATCH RT 3/7] net: add back the missing serialization in ip_send_unicast_reply()

[Steven Rostedt]

On Fri, 23 Sep 2016 15:50:11 -0400
Steven Rostedt <rostedt@goodmis.org> wrote:

> 3.2.82-rt119-rc1 stable review patch.
> If anyone has any objections, please let me know.

I once again forgot to fix the umlaut before sending it via quilt. I'll
have to try to fix quilt when I get a chance.

-- Steve

> 
> ------------------
> 
> From: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
> 
> Some time ago Sami PietikÀinen reported a crash on -RT in
> ip_send_unicast_reply() which was later fixed by Nicholas Mc Guire
> (v3.12.8-rt11). Later (v3.18.8) the code was reworked and I dropped the
> patch. As it turns out it was mistake.
> I have reports that the same crash is possible with a similar backtrace.
> It seems that vanilla protects access to this_cpu_ptr() via
> local_bh_disable(). This does not work the on -RT since we can have
> NET_RX and NET_TX running in parallel on the same CPU.
> This is brings back the old locks.
> 
> |Unable to handle kernel NULL pointer dereference at virtual address 00000010
> |PC is at __ip_make_skb+0x198/0x3e8
> |[<c04e39d8>] (__ip_make_skb) from [<c04e3ca8>] (ip_push_pending_frames+0x20/0x40)
> |[<c04e3ca8>] (ip_push_pending_frames) from [<c04e3ff0>] (ip_send_unicast_reply+0x210/0x22c)
> |[<c04e3ff0>] (ip_send_unicast_reply) from [<c04fbb54>] (tcp_v4_send_reset+0x190/0x1c0)
> |[<c04fbb54>] (tcp_v4_send_reset) from [<c04fcc1c>] (tcp_v4_do_rcv+0x22c/0x288)
> |[<c04fcc1c>] (tcp_v4_do_rcv) from [<c0474364>] (release_sock+0xb4/0x150)
> |[<c0474364>] (release_sock) from [<c04ed904>] (tcp_close+0x240/0x454)
> |[<c04ed904>] (tcp_close) from [<c0511408>] (inet_release+0x74/0x7c)
> |[<c0511408>] (inet_release) from [<c0470728>] (sock_release+0x30/0xb0)
> |[<c0470728>] (sock_release) from [<c0470abc>] (sock_close+0x1c/0x24)
> |[<c0470abc>] (sock_close) from [<c0115ec4>] (__fput+0xe8/0x20c)
> |[<c0115ec4>] (__fput) from [<c0116050>] (____fput+0x18/0x1c)
> |[<c0116050>] (____fput) from [<c0058138>] (task_work_run+0xa4/0xb8)
> |[<c0058138>] (task_work_run) from [<c0011478>] (do_work_pending+0xd0/0xe4)
> |[<c0011478>] (do_work_pending) from [<c000e740>] (work_pending+0xc/0x20)
> |Code: e3530001 8a000001 e3a00040 ea000011 (e5973010)
> 
> Cc: stable-rt@vger.kernel.org
> Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
> Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
> ---
>  net/ipv4/tcp_ipv4.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/net/ipv4/tcp_ipv4.c b/net/ipv4/tcp_ipv4.c
> index b4e0eb49f56d..e6345b547922 100644
> --- a/net/ipv4/tcp_ipv4.c
> +++ b/net/ipv4/tcp_ipv4.c
> @@ -61,6 +61,7 @@
>  #include <linux/init.h>
>  #include <linux/times.h>
>  #include <linux/slab.h>
> +#include <linux/locallock.h>
>  
>  #include <net/net_namespace.h>
>  #include <net/icmp.h>
> @@ -575,6 +576,7 @@ int tcp_v4_gso_send_check(struct sk_buff *skb)
>  	return 0;
>  }
>  
> +static DEFINE_LOCAL_IRQ_LOCK(tcp_sk_lock);
>  /*
>   *	This routine will send an RST to the other tcp.
>   *
> @@ -659,8 +661,11 @@ static void tcp_v4_send_reset(struct sock *sk, struct sk_buff *skb)
>  
>  	net = dev_net(skb_dst(skb)->dev);
>  	arg.tos = ip_hdr(skb)->tos;
> +
> +	local_lock(tcp_sk_lock);
>  	ip_send_reply(net->ipv4.tcp_sock, skb, ip_hdr(skb)->saddr,
>  		      &arg, arg.iov[0].iov_len);
> +	local_unlock(tcp_sk_lock);
>  
>  	TCP_INC_STATS_BH(net, TCP_MIB_OUTSEGS);
>  	TCP_INC_STATS_BH(net, TCP_MIB_OUTRSTS);
> @@ -734,8 +739,10 @@ static void tcp_v4_send_ack(struct sk_buff *skb, u32 seq, u32 ack,
>  	if (oif)
>  		arg.bound_dev_if = oif;
>  	arg.tos = tos;
> +	local_lock(tcp_sk_lock);
>  	ip_send_reply(net->ipv4.tcp_sock, skb, ip_hdr(skb)->saddr,
>  		      &arg, arg.iov[0].iov_len);
> +	local_unlock(tcp_sk_lock);
>  
>  	TCP_INC_STATS_BH(net, TCP_MIB_OUTSEGS);
>  }