Softirq -rt Optimizations
v2 cover Softirq -rt Optimizations
Hey all,
This series is a set of patches that optimize scheduler decisions around realtime tasks and softirqs. This series is a rebased and reworked set of changes that have been shipping on Android devices for a number of years, originally created to resolve audio glitches seen on devices caused by network or storage driver softirqs.
Long running softirqs cause issues because they aren’t currently taken into account when a realtime task is woken up, but they will delay realtime tasks from running if the realtime tasks are placed on a cpu currently running a softirq.
This can easily be seen on some devices by running cyclictest* along with some heavy background filesystems noise:
Without the patches:
T: 0 ( 7596) P:99 I:1000 C: 59980 Min: 7 Act: 13 Avg: 29 Max: 4107
T: 1 ( 7597) P:99 I:1500 C: 39990 Min: 14 Act: 26 Avg: 36 Max: 8994
T: 2 ( 7598) P:99 I:2000 C: 29995 Min: 7 Act: 22 Avg: 35 Max: 3616
T: 3 ( 7599) P:99 I:2500 C: 23915 Min: 7 Act: 25 Avg: 49 Max: 40273
T: 4 ( 7600) P:99 I:3000 C: 19995 Min: 8 Act: 22 Avg: 38 Max: 10510
T: 5 ( 7601) P:99 I:3500 C: 17135 Min: 7 Act: 26 Avg: 39 Max: 13194
T: 6 ( 7602) P:99 I:4000 C: 14990 Min: 7 Act: 26 Avg: 40 Max: 9470
T: 7 ( 7603) P:99 I:4500 C: 13318 Min: 8 Act: 29 Avg: 44 Max: 20101
Which you can visually see in the image here:
https://github.com/johnstultz-work/misc/raw/main/images/2022-08-09-softirq-rt-big-latency.png
Which is from the perfetto trace captured here:
https://ui.perfetto.dev/#!/?s=33661aec8ec82c2da0a59263f36f7d72b4a2f4e7a99b28b222bd12ad872f
The first patch in the series adds logic to account for when softirqs are running, and then conditionally based on CONFIG_RT_SOFTIRQ_OPTIMIZATION allows rt-task placement to be done in a way that’s aware if a current softirq might be a long-running one, to potentially place the rt task on another free core.
The second patch in the series adds logic in __do_softirq(), also under CONFIG_RT_SOFTIRQ_OPTIMIZATION, to defer some of the potentially long running softirqs to ksoftirqd if a -rt task is currently running on the cpu. This patch also includes a folded down fix that stubbs out ksoftirqd_running() based on CONFIG_RT_SOFTIRQ_OPTIMIZATION, since in changing to more frequently defer long running softirqs, the logic using ksoftirqd_running will end up being too conservative and needlessly delay shorter-running softirqs.
With these patches we see dramatic improvements in the worst case latencies in the cyclictest* + filesystem noise test above:
With the patches
T: 0 ( 7527) P:99 I:1000 C: 59998 Min: 6 Act: 29 Avg: 35 Max: 1734
T: 1 ( 7528) P:99 I:1500 C: 40000 Min: 7 Act: 39 Avg: 35 Max: 1181
T: 2 ( 7529) P:99 I:2000 C: 30000 Min: 7 Act: 25 Avg: 25 Max: 444
T: 3 ( 7530) P:99 I:2500 C: 24000 Min: 7 Act: 34 Avg: 36 Max: 1729
T: 4 ( 7531) P:99 I:3000 C: 20000 Min: 7 Act: 36 Avg: 25 Max: 406
T: 5 ( 7532) P:99 I:3500 C: 17143 Min: 7 Act: 38 Avg: 34 Max: 1264
T: 6 ( 7533) P:99 I:4000 C: 15000 Min: 7 Act: 27 Avg: 33 Max: 2351
T: 7 ( 7534) P:99 I:4500 C: 13334 Min: 7 Act: 41 Avg: 29 Max: 2285
Since these patches have been carried along for years, and have at times badly collided with upstream, I wanted to submit them for some initial review, discussion and feedback so we could hopefully eventually find a reasonable solution that might land upstream.
* Unfortunately cyclictest had a bug that causes it to always affine threads to cpus preventing them from being migrated. So you’ll need to update to the latest version (which includes a fix) to reproduce.
Let me know what you think!
thanks
-john
Changes in v2:
* Reformatted Kconfig entry to match coding style (Reported-by: Randy Dunlap <[email protected]>)
* Made rt_task_fits_capacity_and_may_preempt static to avoid warnings (Reported-by: kernel test robot <[email protected]>)
* Rework to use preempt_count and drop kconfig dependency on ARM64
Connor O'Brien (1):
sched: Avoid placing RT threads on cores handling long softirqs
Pavankumar Kondeti (1):
softirq: defer softirq processing to ksoftirqd if CPU is busy with RT
include/linux/interrupt.h | 7 +++++ include/linux/sched.h | 10 ++++++ init/Kconfig | 10 ++++++ kernel/sched/cpupri.c | 13 ++++++++ kernel/sched/rt.c | 65 +++++++++++++++++++++++++++++++++------ kernel/softirq.c | 34 ++++++++++++++++++-- 6 files changed, 128 insertions(+), 11 deletions(-)
Signature · 1 line
2.37.1.595.g718a3a8f04-goog
RFC PATCH v2 1/2 sched: Avoid placing RT threads on cores handling long softirqs
From: Connor O'Brien <[email protected]>
In certain audio use cases, scheduling RT threads on cores that are handling softirqs can lead to glitches. Prevent this behavior in cases where the softirq is likely to take a long time. To avoid unnecessary migrations, the old behavior is preserved for RCU, SCHED and TIMER irqs which are expected to be relatively quick.
This patch reworks and combines two related changes originally by John Dias <[email protected]>
[jstultz: Further simplified and fixed issues, reworded commit
message, removed arm64-isms]
---
v2:
* Reformatted Kconfig entry to match coding style (Reported-by: Randy Dunlap <[email protected]>)
* Made rt_task_fits_capacity_and_may_preempt static to avoid warnings (Reported-by: kernel test robot <[email protected]>)
* Rework to use preempt_count and drop kconfig dependency on ARM64
--- include/linux/interrupt.h | 7 +++++ init/Kconfig | 10 ++++++ kernel/sched/rt.c | 65 +++++++++++++++++++++++++++++++++------ kernel/softirq.c | 9 ++++++ 4 files changed, 82 insertions(+), 9 deletions(-)
include/linux/interrupt.h +7 -0
@@ -571,6 +571,12 @@ enum* _ IRQ_POLL: irq_poll_cpu_dead() migrates the queue */ #define SOFTIRQ_HOTPLUG_SAFE_MASK (BIT(RCU_SOFTIRQ) | BIT(IRQ_POLL_SOFTIRQ)) +/* Softirq's where the handling might be long: */ +#define LONG_SOFTIRQ_MASK ((1 << NET_TX_SOFTIRQ) | \ + (1 << NET_RX_SOFTIRQ) | \ + (1 << BLOCK_SOFTIRQ) | \ + (1 << IRQ_POLL_SOFTIRQ) | \ + (1 << TASKLET_SOFTIRQ))/* map softirq index to softirq name. update 'softirq_to_name' in * kernel/softirq.c when adding a new softirq.
@@ -606,6 +612,7 @@ extern void raise_softirq_irqoff(unsigned int nr);extern void raise_softirq (unsigned int nr); DECLARE_PER_CPU(struct task_struct *, ksoftirqd); +DECLARE_PER_CPU (u32, active_softirqs);static inline struct task_struct *this_cpu_ksoftirqd (void ) {
init/Kconfig +10 -0
@@ -1284,6 +1284,16 @@ config SCHED_AUTOGROUP desktop applications. Task group autogeneration is currently based upon task session. +config RT_SOFTIRQ_OPTIMIZATION + bool "Improve RT scheduling during long softirq execution" + depends on SMP + default n + help + Enable an optimization which tries to avoid placing RT tasks on CPUs + occupied by nonpreemptible tasks, such as a long softirq or CPUs + which may soon block preemptions, such as a CPU running a ksoftirq + thread which handles slow softirqs. + config SYSFS_DEPRECATED bool "Enable deprecated sysfs features to support old userspace tools" depends on SYSFS
kernel/sched/rt.c +56 -9
106 lines not on this page · raw message
kernel/softirq.c +9 -0
@@ -60,6 +60,13 @@ static struct softirq_action softirq_vec[NR_SOFTIRQS] __cacheline_aligned_in_smp DEFINE_PER_CPU(struct task_struct *, ksoftirqd); +/* +* active_softirqs -- per cpu, a mask of softirqs that are being handled, +* with the expectation that approximate answers are acceptable and therefore +* no synchronization. +*/ +DEFINE_PER_CPU (u32, active_softirqs); +const char *const softirq_to_name[NR_SOFTIRQS ] = {"HI" ,"TIMER" ,"NET_TX" ,"NET_RX" ,"BLOCK" ,"IRQ_POLL" ,"TASKLET" ,"SCHED" ,"HRTIMER" ,"RCU"
@@ -551,6 +558,7 @@ asmlinkage __visible void __softirq_entry __do_softirq(void) restart:/* Reset the pending bitmask before enabling irqs */ set_softirq_pending (0 ); +__this_cpu_write (active_softirqs, pending);local_irq_enable ();
@@ -580,6 +588,7 @@ asmlinkage __visible void __softirq_entry __do_softirq(void) pending >>= softirq_bit; } +__this_cpu_write (active_softirqs,0 );if (!IS_ENABLED (CONFIG_PREEMPT_RT ) &&__this_cpu_read (ksoftirqd) == current)rcu_softirq_qs ();
Signature · 1 line
2.37.1.595.g718a3a8f04-goog
kernel test robot
build botRe: [RFC][PATCH v2 1/2] sched: Avoid placing RT threads on cores handling long softirqs
[If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in…
Hi John,
[FYI, it's a private test report for your RFC patch.]
[auto build test ERROR on linus/master]
[also build test ERROR on v6.0-rc2 next-20220824]
[cannot apply to tip/sched/core]
[If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/John-Stultz/Softirq-rt-Optimizations/20220823-030642 base: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 072e51356cd5a4a1c12c1020bc054c99b98333df config: s390-randconfig-r003-20220824 (https://download.01.org/0day-ci/archive/20220824/[email protected]/config) compiler: clang version 16.0.0 (https://github.com/llvm/llvm-project d00e97df0fe8c67f694c4d027297f4382ce72b38)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross
# install s390 cross compiling tool for clang build
# apt-get install binutils-s390x-linux-gnu
# https://github.com/intel-lab-lkp/linux/commit/1d1c0ba7867315d6327259cbf7a561248330038b
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review John-Stultz/Softirq-rt-Optimizations/20220823-030642 git checkout 1d1c0ba7867315d6327259cbf7a561248330038b
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=s390 SHELL=/bin/bash
If you fix the issue, kindly add following tag where applicable
All errors (new ones prefixed by >>):
In file included from kernel/sched/build_policy.c:34:
In file included from kernel/sched/sched.h:63:
In file included from include/linux/tick.h:8:
In file included from include/linux/clockchips.h:14:
In file included from include/linux/clocksource.h:22:
In file included from arch/s390/include/asm/io.h:75:
include/asm-generic/io.h:547:31: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
val = __raw_readb(PCI_IOBASE + addr);
~~~~~~~~~~ ^
include/asm-generic/io.h:560:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
val = __le16_to_cpu((__le16 __force)__raw_readw(PCI_IOBASE + addr));
~~~~~~~~~~ ^
include/uapi/linux/byteorder/big_endian.h:37:59: note: expanded from macro '__le16_to_cpu'
#define __le16_to_cpu(x) __swab16((__force __u16)(__le16)(x))
^
include/uapi/linux/swab.h:102:54: note: expanded from macro '__swab16'
#define __swab16(x) (__u16)__builtin_bswap16((__u16)(x))
^
In file included from kernel/sched/build_policy.c:34:
In file included from kernel/sched/sched.h:63:
In file included from include/linux/tick.h:8:
In file included from include/linux/clockchips.h:14:
In file included from include/linux/clocksource.h:22:
In file included from arch/s390/include/asm/io.h:75:
include/asm-generic/io.h:573:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
val = __le32_to_cpu((__le32 __force)__raw_readl(PCI_IOBASE + addr));
~~~~~~~~~~ ^
include/uapi/linux/byteorder/big_endian.h:35:59: note: expanded from macro '__le32_to_cpu'
#define __le32_to_cpu(x) __swab32((__force __u32)(__le32)(x))
^
include/uapi/linux/swab.h:115:54: note: expanded from macro '__swab32'
#define __swab32(x) (__u32)__builtin_bswap32((__u32)(x))
^
In file included from kernel/sched/build_policy.c:34:
In file included from kernel/sched/sched.h:63:
In file included from include/linux/tick.h:8:
In file included from include/linux/clockchips.h:14:
In file included from include/linux/clocksource.h:22:
In file included from arch/s390/include/asm/io.h:75:
include/asm-generic/io.h:584:33: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
__raw_writeb(value, PCI_IOBASE + addr);
~~~~~~~~~~ ^
include/asm-generic/io.h:594:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
__raw_writew((u16 __force)cpu_to_le16(value), PCI_IOBASE + addr);
~~~~~~~~~~ ^
include/asm-generic/io.h:604:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
__raw_writel((u32 __force)cpu_to_le32(value), PCI_IOBASE + addr);
~~~~~~~~~~ ^
include/asm-generic/io.h:692:20: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
readsb(PCI_IOBASE + addr, buffer, count);
~~~~~~~~~~ ^
include/asm-generic/io.h:700:20: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
readsw(PCI_IOBASE + addr, buffer, count);
~~~~~~~~~~ ^
include/asm-generic/io.h:708:20: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
readsl(PCI_IOBASE + addr, buffer, count);
~~~~~~~~~~ ^
include/asm-generic/io.h:717:21: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
writesb(PCI_IOBASE + addr, buffer, count);
~~~~~~~~~~ ^
include/asm-generic/io.h:726:21: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
writesw(PCI_IOBASE + addr, buffer, count);
~~~~~~~~~~ ^
include/asm-generic/io.h:735:21: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
writesl(PCI_IOBASE + addr, buffer, count);
~~~~~~~~~~ ^
In file included from kernel/sched/build_policy.c:45:
>> kernel/sched/rt.c:1612:33: error: no member named '__softirq_pending' in 'struct irq_stat'
per_cpu(irq_stat, cpu).__softirq_pending;
~~~~~~~~~~~~~~~~~~~~~~ ^12 warnings and 1 error generated.
vim +1612 kernel/sched/rt.c
1601
1602 #ifdef CONFIG_RT_SOFTIRQ_OPTIMIZATION
1603 /*
1604 * Return whether the task on the given cpu is currently non-preemptible
1605 * while handling a potentially long softirq, or if the task is likely
1606 * to block preemptions soon because it is a ksoftirq thread that is
1607 * handling slow softirq.
1608 */
1609 static bool task_may_preempt(struct task_struct *task, int cpu)
1610 {
1611 u32 softirqs = per_cpu(active_softirqs, cpu) |
> 1612 per_cpu(irq_stat, cpu).__softirq_pending;
1613
1614 struct task_struct *cpu_ksoftirqd = per_cpu(ksoftirqd, cpu);
1615 struct task_struct *curr;
1616 struct rq *rq = cpu_rq(cpu);
1617 int ret;
1618
1619 rcu_read_lock();
1620 curr = READ_ONCE(rq->curr); /* unlocked access */
1621 ret = !((softirqs & LONG_SOFTIRQ_MASK) &&
1622 (curr == cpu_ksoftirqd ||
1623 preempt_count() & SOFTIRQ_MASK));
1624 rcu_read_unlock();
1625 return ret;
1626 }
1627 #else
1628 static bool task_may_preempt(struct task_struct *task, int cpu)
1629 {
1630 return true;
1631 }
1632 #endif /* CONFIG_RT_SOFTIRQ_OPTIMIZATION */
1633
Signature · 2 lines
0-DAY CI Kernel Test Service https://01.org/lkp
RFC PATCH v2 2/2 softirq: defer softirq processing to ksoftirqd if CPU is busy with RT
In reply to John StultzFrom: Pavankumar Kondeti <[email protected]>
Defer the softirq processing to ksoftirqd if a RT task is running or queued on the current CPU. This complements the RT task placement algorithm which tries to find a CPU that is not currently busy with softirqs.
Currently NET_TX, NET_RX, BLOCK and TASKLET softirqs are only deferred as they can potentially run for long time.
Additionally, this patch stubs out ksoftirqd_running() logic, in the CONFIG_RT_SOFTIRQ_OPTIMIZATION case, as deferring potentially long-running softirqs will cause the logic to not process shorter-running softirqs immediately. By stubbing it out the potentially long running softirqs are deferred, but the shorter running ones can still run immediately.
This patch includes folded-in fixes by:
Lingutla Chandrasekhar <[email protected]> Satya Durga Srinivasu Prabhala <[email protected]> J. Avila <[email protected]>
[jstultz: Rebase to linus/HEAD, minor rearranging of code,
included bug fix Reported-by: Qais Yousef <[email protected]> ]
--- include/linux/sched.h | 10 ++++++++++ kernel/sched/cpupri.c | 13 +++++++++++++ kernel/softirq.c | 25 +++++++++++++++++++++++-- 3 files changed, 46 insertions(+), 2 deletions(-)
include/linux/sched.h +10 -0
@@ -1826,6 +1826,16 @@ current_restore_flags(unsigned long orig_flags, unsigned long flags)extern int cpuset_cpumask_can_shrink (const struct cpumask *cur,const struct cpumask *trial);extern int task_can_attach (struct task_struct *p,const struct cpumask *cs_effective_cpus); + +#ifdef CONFIG_RT_SOFTIRQ_OPTIMIZATION +extern bool cpupri_check_rt (void ); +#else +static inline bool cpupri_check_rt (void ) +{ +return false; +} +#endif +#ifdef CONFIG_SMP extern void do_set_cpus_allowed (struct task_struct *p,const struct cpumask *new_mask);extern int set_cpus_allowed_ptr (struct task_struct *p,const struct cpumask *new_mask);
kernel/sched/cpupri.c +13 -0
@@ -64,6 +64,19 @@ static int convert_prio(int prio)return cpupri; } +#ifdef CONFIG_RT_SOFTIRQ_OPTIMIZATION +/* +* cpupri_check_rt - check if CPU has a RT task +* should be called from rcu-sched read section. +*/ +bool cpupri_check_rt (void ) +{ +int cpu =raw_smp_processor_id (); + +return cpu_rq (cpu)->rd->cpupri.cpu_to_pri[cpu] >CPUPRI_NORMAL ; +} +#endif +static inline int __cpupri_find (struct cpupri *cp,struct task_struct *p,struct cpumask *lowest_mask,int idx) {
kernel/softirq.c +23 -2
@@ -87,6 +87,7 @@ static void wakeup_softirqd(void)wake_up_process (tsk); } +#ifndef CONFIG_RT_SOFTIRQ_OPTIMIZATION /* * If ksoftirqd is scheduled, we do not want to process pending softirqs * right now. Let ksoftirqd handle this at its own rate, to get fairness,
@@ -101,6 +102,9 @@ static bool ksoftirqd_running(unsigned long pending)return false; return tsk && task_is_running(tsk) && !__kthread_should_park(tsk); } +#else +#define ksoftirqd_running (pending) (false) +#endif /* CONFIG_RT_SOFTIRQ_OPTIMIZATION */ #ifdef CONFIG_TRACE_IRQFLAGS DEFINE_PER_CPU (int, hardirqs_enabled);
@@ -532,6 +536,17 @@ static inline bool lockdep_softirq_start(void) { return false; }static inline void lockdep_softirq_end (bool in_hardirq) { }#endif +static __u32 softirq_deferred_for_rt (__u32 *pending) +{ +__u32 deferred =0 ; + +if (cpupri_check_rt ()) { + deferred = *pending &LONG_SOFTIRQ_MASK ; + *pending &= ~LONG_SOFTIRQ_MASK ; + } +return deferred; +} + asmlinkage __visible void __softirq_entry __do_softirq(void) {unsigned long end = jiffies +MAX_SOFTIRQ_TIME ;
@@ -539,6 +554,7 @@ asmlinkage __visible void __softirq_entry __do_softirq(void)int max_restart =MAX_SOFTIRQ_RESTART ;struct softirq_action *h;bool in_hardirq; +__u32 deferred;__u32 pending;int softirq_bit;
@@ -551,13 +567,15 @@ asmlinkage __visible void __softirq_entry __do_softirq(void) pending =local_softirq_pending (); + deferred =softirq_deferred_for_rt (&pending);softirq_handle_begin (); + in_hardirq =lockdep_softirq_start ();account_softirq_enter (current); restart:/* Reset the pending bitmask before enabling irqs */ -set_softirq_pending (0 ); +set_softirq_pending (deferred);__this_cpu_write (active_softirqs, pending);local_irq_enable ();
@@ -596,13 +614,16 @@ asmlinkage __visible void __softirq_entry __do_softirq(void)local_irq_disable (); pending =local_softirq_pending (); + deferred =softirq_deferred_for_rt (&pending); +if (pending) {if (time_before (jiffies, end) && !need_resched () && --max_restart) goto restart; + } +if (pending | deferred)wakeup_softirqd (); - }account_softirq_exit (current);lockdep_softirq_end (in_hardirq);
Signature · 1 line
2.37.1.595.g718a3a8f04-goog