| Message ID | 20190801143658.074833024@linutronix.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | posix-cpu-timers: Move expiry into task work context | expand |
On Thu, Aug 01, 2019 at 04:32:54PM +0200, Thomas Gleixner wrote: > --- a/kernel/time/Kconfig > +++ b/kernel/time/Kconfig > @@ -52,6 +52,11 @@ config GENERIC_CLOCKEVENTS_MIN_ADJUST > config GENERIC_CMOS_UPDATE > bool > > +# Select to handle posix CPU timers from task_work > +# and not from the timer interrupt context > +config POSIX_CPU_TIMERS_TASK_WORK > + bool > + > if GENERIC_CLOCKEVENTS > menu "Timers subsystem" > diff --git a/kernel/Kconfig.preempt b/kernel/Kconfig.preempt index deff97217496..76e37ad5bc31 100644 --- a/kernel/Kconfig.preempt +++ b/kernel/Kconfig.preempt @@ -58,6 +58,7 @@ config PREEMPT config PREEMPT_RT bool "Fully Preemptible Kernel (Real-Time)" depends on EXPERT && ARCH_SUPPORTS_RT + depends on POSIX_CPU_TIMERS_TASK_WORK select PREEMPTION help This option turns the kernel into a real-time kernel by replacing
On Thu, 1 Aug 2019, Peter Zijlstra wrote: > On Thu, Aug 01, 2019 at 04:32:54PM +0200, Thomas Gleixner wrote: > > --- a/kernel/time/Kconfig > > +++ b/kernel/time/Kconfig > > @@ -52,6 +52,11 @@ config GENERIC_CLOCKEVENTS_MIN_ADJUST > > config GENERIC_CMOS_UPDATE > > bool > > > > +# Select to handle posix CPU timers from task_work > > +# and not from the timer interrupt context > > +config POSIX_CPU_TIMERS_TASK_WORK > > + bool > > + > > if GENERIC_CLOCKEVENTS > > menu "Timers subsystem" > > > > > diff --git a/kernel/Kconfig.preempt b/kernel/Kconfig.preempt > index deff97217496..76e37ad5bc31 100644 > --- a/kernel/Kconfig.preempt > +++ b/kernel/Kconfig.preempt > @@ -58,6 +58,7 @@ config PREEMPT > config PREEMPT_RT > bool "Fully Preemptible Kernel (Real-Time)" > depends on EXPERT && ARCH_SUPPORTS_RT > + depends on POSIX_CPU_TIMERS_TASK_WORK Indeed.
On 08/01, Thomas Gleixner wrote: > > +static void __run_posix_cpu_timers(struct task_struct *tsk) > +{ > + /* FIXME: Init it proper in fork or such */ > + init_task_work(&tsk->cpu_timer_work, posix_cpu_timers_work); > + task_work_add(tsk, &tsk->cpu_timer_work, true); > +} What if update_process_times/run_posix_cpu_timers is called again before this task does task_work_run() ? somehow it should check that ->cpu_timer_work is not already queued... Or suppose that this is called when task_work_run() executes this cpu_timer_work. Looks like you need another flag checked by __run_posix_cpu_timers() and cleare in posix_cpu_timers_work() ? Oleg.
On Thu, 1 Aug 2019, Oleg Nesterov wrote: > On 08/01, Thomas Gleixner wrote: > > > > +static void __run_posix_cpu_timers(struct task_struct *tsk) > > +{ > > + /* FIXME: Init it proper in fork or such */ > > + init_task_work(&tsk->cpu_timer_work, posix_cpu_timers_work); > > + task_work_add(tsk, &tsk->cpu_timer_work, true); > > +} > > What if update_process_times/run_posix_cpu_timers is called again before > this task does task_work_run() ? > > somehow it should check that ->cpu_timer_work is not already queued... Right. > Or suppose that this is called when task_work_run() executes this > cpu_timer_work. Looks like you need another flag checked by > __run_posix_cpu_timers() and cleare in posix_cpu_timers_work() ? That's a non issue. The only thing which can happen is that it runs through the task_work once more to figure out there is nothing to do. Thanks, tglx
--- a/include/linux/sched.h +++ b/include/linux/sched.h @@ -880,6 +880,9 @@ struct task_struct { struct task_cputime cputime_expires; struct list_head cpu_timers[3]; #endif +#ifdef CONFIG_POSIX_CPU_TIMERS_TASK_WORK + struct callback_head cpu_timer_work; +#endif /* Process credentials: */ --- a/kernel/time/Kconfig +++ b/kernel/time/Kconfig @@ -52,6 +52,11 @@ config GENERIC_CLOCKEVENTS_MIN_ADJUST config GENERIC_CMOS_UPDATE bool +# Select to handle posix CPU timers from task_work +# and not from the timer interrupt context +config POSIX_CPU_TIMERS_TASK_WORK + bool + if GENERIC_CLOCKEVENTS menu "Timers subsystem" --- a/kernel/time/posix-cpu-timers.c +++ b/kernel/time/posix-cpu-timers.c @@ -14,6 +14,7 @@ #include <linux/tick.h> #include <linux/workqueue.h> #include <linux/compat.h> +#include <linux/task_work.h> #include <linux/sched/deadline.h> #include "posix-timers.h" @@ -1127,7 +1128,7 @@ static inline int fastpath_timer_check(s return 0; } -static void __run_posix_cpu_timers(struct task_struct *tsk) +static void handle_posix_cpu_timers(struct task_struct *tsk) { struct k_itimer *timer, *next; unsigned long flags; @@ -1178,6 +1179,29 @@ static void __run_posix_cpu_timers(struc } } +#ifdef CONFIG_POSIX_CPU_TIMERS_TASK_WORK + +static void posix_cpu_timers_work(struct callback_head *work) +{ + handle_posix_cpu_timers(current); +} + +static void __run_posix_cpu_timers(struct task_struct *tsk) +{ + /* FIXME: Init it proper in fork or such */ + init_task_work(&tsk->cpu_timer_work, posix_cpu_timers_work); + task_work_add(tsk, &tsk->cpu_timer_work, true); +} + +#else + +static void __run_posix_cpu_timers(struct task_struct *tsk) +{ + handle_posix_cpu_timers(tsk); +} + +#endif + /* * This is called from the timer interrupt handler. The irq handler has * already updated our counts. We need to check if any timers fire now.
Running posix cpu timers in hard interrupt context has a few downsides: - For PREEMPT_RT it cannot work as the expiry code needs to take sighand lock, which is a 'sleeping spinlock' in RT - For fine grained accounting it's just wrong to run this in context of the timer interrupt because that way a process specific cpu time is accounted to the timer interrupt. There is no real hard requirement to run the expiry code in hard interrupt context. The posix CPU timers are an approximation anyway, so having them expired and evaluated in task work context does not really make them worse. Make it conditional on a selectable config switch as this requires that task work is handled in KVM. The available tests pass and no problematic difference has been observed. Signed-off-by: Thomas Gleixner <tglx@linutronix.de> Cc: John Stultz <john.stultz@linaro.org> Cc: Andy Lutomirski <luto@kernel.org> Cc: "Paul E. McKenney" <paulmck@linux.ibm.com> --- include/linux/sched.h | 3 +++ kernel/time/Kconfig | 5 +++++ kernel/time/posix-cpu-timers.c | 26 +++++++++++++++++++++++++- 3 files changed, 33 insertions(+), 1 deletion(-)