Thread Migration Preemption
RFC 1/1 Thread Migration Preemption
Thread Migration Preemption
This patch adds the ability to protect critical sections from migration to another CPU without disabling preemption.
This will be useful to minimize the amount of preemption disabling for the -rt patch. It will help leveraging improvements brought by the local_t types in asm/local.h (see Documentation/local_ops.txt). Note that the updates done to variables protected by migration_disable must be either atomic or protected from concurrent updates done by other threads.
Typical use:
migration_disable();
local_inc(&__get_cpu_var(&my_local_t_var));migration_enable();
Which will increment the variable atomically wrt the local CPU.
Comments (such as how to integrate this in the already almost full preempt_count) are welcome.
It applies on 2.6.22-rc6-mm1.
--- include/asm-alpha/thread_info.h | 1 + include/asm-arm/thread_info.h | 2 ++ include/asm-arm26/thread_info.h | 2 ++ include/asm-avr32/thread_info.h | 2 ++ include/asm-blackfin/thread_info.h | 2 ++ include/asm-cris/thread_info.h | 2 ++ include/asm-frv/thread_info.h | 2 ++ include/asm-h8300/thread_info.h | 2 ++ include/asm-i386/thread_info.h | 4 ++-- include/asm-ia64/thread_info.h | 2 ++ include/asm-m32r/thread_info.h | 2 ++ include/asm-m68k/thread_info.h | 1 + include/asm-m68knommu/thread_info.h | 1 + include/asm-mips/thread_info.h | 2 ++ include/asm-parisc/thread_info.h | 2 ++ include/asm-powerpc/thread_info.h | 2 ++ include/asm-s390/thread_info.h | 2 ++ include/asm-sh/thread_info.h | 2 ++ include/asm-sh64/thread_info.h | 2 ++ include/asm-sparc/thread_info.h | 2 ++ include/asm-sparc64/thread_info.h | 2 ++ include/asm-um/thread_info.h | 2 ++ include/asm-v850/thread_info.h | 2 ++ include/asm-x86_64/thread_info.h | 2 ++ include/asm-xtensa/thread_info.h | 2 ++ include/linux/preempt.h | 26 +++++++++++++++++++++++++- kernel/sched.c | 26 ++++++++++++++++++++++++++ 27 files changed, 98 insertions(+), 3 deletions(-)
include/asm-i386/thread_info.h +2 -2
@@ -31,8 +31,7 @@unsigned long status;/* thread-synchronous flags */ __u32 cpu;/* current CPU */ int preempt_count;/* 0 => preemptable, <0 => BUG */ - - +int migration_count;/* 0: can migrate, <0: BUG */ mm_segment_t addr_limit;/* thread address space: 0-0xBFFFFFFF for user-thead 0-0xFFFFFFFF for kernel-thread
@@ -74,6 +73,7 @@ .flags = 0, \ .cpu = 0, \ .preempt_count = 1, \ + .migration_count = 0, \ .addr_limit = KERNEL_DS, \ .restart_block = { \ .fn = do_no_restart_syscall, \
include/linux/preempt.h +25 -1
@@ -12,15 +12,24 @@#ifdef CONFIG_DEBUG_PREEMPT extern void fastcall add_preempt_count(int val); extern void fastcall sub_preempt_count(int val); + extern void fastcall add_migration_count(int val); + extern void fastcall sub_migration_count(int val);#else # define add_preempt_count (val) do { preempt_count() += (val); } while (0)# define sub_preempt_count (val) do { preempt_count() -= (val); } while (0) +# define add_migration_count (val) do { migration_count() += (val); } while (0) +# define sub_migration_count (val) do { migration_count() -= (val); } while (0)#endif #define inc_preempt_count () add_preempt_count(1)#define dec_preempt_count () sub_preempt_count(1) -#define preempt_count () (current_thread_info()->preempt_count) +#define preempt_count () (current_thread_info()->preempt_count) + +#define inc_migration_count () add_migration_count(1) +#define dec_migration_count () sub_migration_count(1) + +#define migration_count () (current_thread_info()->migration_count)#ifdef CONFIG_PREEMPT
@@ -51,6 +60,18 @@preempt_check_resched (); \ } while (0) +#define migration_disable () \ +do { \ + inc_migration_count(); \ + barrier(); \ +} while (0) + +#define migration_enable () \ +do { \ + barrier(); \ + dec_migration_count(); \ +} while (0) +#else #define preempt_disable () do { } while (0)
@@ -58,6 +79,9 @@#define preempt_enable () do { } while (0)#define preempt_check_resched () do { } while (0) +#define migration_disable () do { } while (0) +#define migration_enable () do { } while (0) +#endif #endif /* __LINUX_PREEMPT_H */
kernel/sched.c +26 -0
@@ -1996,6 +1996,7 @@* 1) running (obviously), or * 2) cannot be migrated to this CPU due to cpus_allowed, or * 3) are cache-hot on their current CPU. +* 4) migration preemption is non 0 for this non running task. */ if (!cpu_isset (this_cpu, p->cpus_allowed))return 0 ;
@@ -2003,6 +2004,8 @@if (task_running (rq, p))return 0 ; +if (task_thread_info (p)->migration_count) +return 0 ;/* * Aggressive migration if too many balance attempts have failed:
@@ -3220,6 +3223,29 @@ }EXPORT_SYMBOL (sub_preempt_count); +void fastcall add_migration_count(int val) +{ +/* +* Underflow? +*/ +if (DEBUG_LOCKS_WARN_ON ((migration_count () <0 ))) +return ; +migration_count () += val; +} +EXPORT_SYMBOL (add_migration_count); + +void fastcall sub_migration_count(int val) +{ +/* +* Underflow? +*/ +if (DEBUG_LOCKS_WARN_ON (val >migration_count ())) +return ; + +migration_count () -= val; +} +EXPORT_SYMBOL (sub_migration_count); +#endif /*
include/asm-alpha/thread_info.h +1 -0
@@ -21,6 +21,7 @@mm_segment_t addr_limit;/* thread address space */ unsigned cpu;/* current CPU */ int preempt_count;/* 0 => preemptable, <0 => BUG */ +int migration_count;/* 0: can migrate, <0 => BUG */ int bpt_nsaved;unsigned long bpt_addr[2 ];/* breakpoint handling */
include/asm-arm/thread_info.h +2 -0
@@ -51,6 +51,7 @@struct thread_info {unsigned long flags;/* low level flags */ int preempt_count;/* 0 => preemptable, <0 => bug */ +int migration_count;/* 0: can migrate, <0 => BUG */ mm_segment_t addr_limit;/* address limit */ struct task_struct *task;/* main task structure */ struct exec_domain *exec_domain;/* execution domain */
@@ -72,6 +73,7 @@ .exec_domain = &default_exec_domain, \ .flags = 0, \ .preempt_count = 1, \ + .migration_count = 0, \ .addr_limit = KERNEL_DS, \ .cpu_domain =domain_val (DOMAIN_USER ,DOMAIN_MANAGER ) | \ domain_val(DOMAIN_KERNEL, DOMAIN_MANAGER) | \
include/asm-arm26/thread_info.h +2 -0
@@ -45,6 +45,7 @@struct thread_info {unsigned long flags;/* low level flags */ int preempt_count;/* 0 => preemptable, <0 => bug */ +int migration_count;/* 0: can migrate, <0 => BUG */ mm_segment_t addr_limit;/* address limit */ struct task_struct *task;/* main task structure */ struct exec_domain *exec_domain;/* execution domain */
@@ -60,6 +61,7 @@ .exec_domain &default_exec_domain, \ .flags 0, \ .preempt_count 0, \ + .migration_count 0, \ .addr_limit KERNEL_DS, \ .restart_block = { \ .fn = do_no_restart_syscall, \
include/asm-avr32/thread_info.h +2 -0
@@ -25,6 +25,7 @@unsigned long flags;/* low level flags */ __u32 cpu;__s32 preempt_count;/* 0 => preemptable, <0 => BUG */ +int migration_count;/* 0: can migrate, <0 => BUG */ struct restart_block restart_block;__u8 supervisor_stack[0 ]; };
@@ -36,6 +37,7 @@ .flags = 0, \ .cpu = 0, \ .preempt_count = 1, \ + .migration_count = 0, \ .restart_block = { \ .fn = do_no_restart_syscall \ } \
include/asm-blackfin/thread_info.h +2 -0
@@ -54,6 +54,7 @@unsigned long flags;/* low level flags */ int cpu;/* cpu we're on */ int preempt_count;/* 0 => preemptable, <0 => BUG */ +int migration_count;/* 0: can migrate, <0 => BUG */ mm_segment_t addr_limit;/* address limit */ struct restart_block restart_block;struct l1_scratch_task_info l1_task_info;
@@ -69,6 +70,7 @@ .flags = 0, \ .cpu = 0, \ .preempt_count = 1, \ + .migration_count = 0, \ .restart_block = { \ .fn = do_no_restart_syscall, \ }, \
include/asm-cris/thread_info.h +2 -0
@@ -32,6 +32,7 @@unsigned long flags;/* low level flags */ __u32 cpu;/* current CPU */ int preempt_count;/* 0 => preemptable, <0 => BUG */ +int migration_count;/* 0: can migrate, <0 => BUG */ mm_segment_t addr_limit;/* thread address space: 0-0xBFFFFFFF for user-thead
@@ -58,6 +59,7 @@ .flags = 0, \ .cpu = 0, \ .preempt_count = 1, \ + .migration_count = 0, \ .addr_limit = KERNEL_DS, \ .restart_block = { \ .fn = do_no_restart_syscall, \
include/asm-frv/thread_info.h +2 -0
@@ -36,6 +36,7 @@unsigned long status;/* thread-synchronous flags */ __u32 cpu;/* current CPU */ int preempt_count;/* 0 => preemptable, <0 => BUG */ +int migration_count;/* 0: can migrate, <0 => BUG */ mm_segment_t addr_limit;/* thread address space: 0-0xBFFFFFFF for user-thead
@@ -68,6 +69,7 @@ .flags = 0, \ .cpu = 0, \ .preempt_count = 1, \ + .migration_count = 0, \ .addr_limit = KERNEL_DS, \ .restart_block = { \ .fn = do_no_restart_syscall, \
include/asm-h8300/thread_info.h +2 -0
@@ -24,6 +24,7 @@unsigned long flags;/* low level flags */ int cpu;/* cpu we're on */ int preempt_count;/* 0 => preemptable, <0 => BUG */ +int migration_count;/* 0: can migrate, <0 => BUG */ struct restart_block restart_block; };
@@ -37,6 +38,7 @@ .flags = 0, \ .cpu = 0, \ .preempt_count = 1, \ + .migration_count = 0, \ .restart_block = { \ .fn = do_no_restart_syscall, \ }, \
include/asm-ia64/thread_info.h +2 -0
@@ -30,6 +30,7 @@__u32 status;/* Thread synchronous flags */ mm_segment_t addr_limit;/* user-level address space limit */ int preempt_count;/* 0=premptable, <0=BUG; will also serve as bh-counter */ +int migration_count;/* 0: can migrate, <0 => BUG */ struct restart_block restart_block; };
@@ -43,6 +44,7 @@ .cpu = 0, \ .addr_limit = KERNEL_DS, \ .preempt_count = 0, \ + .migration_count = 0, \ .restart_block = { \ .fn = do_no_restart_syscall, \ }, \
include/asm-m32r/thread_info.h +2 -0
@@ -29,6 +29,7 @@unsigned long status;/* thread-synchronous flags */ __u32 cpu;/* current CPU */ int preempt_count;/* 0 => preemptable, <0 => BUG */ +int migration_count;/* 0: can migrate, <0 => BUG */ mm_segment_t addr_limit;/* thread address space: 0-0xBFFFFFFF for user-thread
@@ -69,6 +70,7 @@ .flags = 0, \ .cpu = 0, \ .preempt_count = 1, \ + .migration_count = 0, \ .addr_limit = KERNEL_DS, \ .restart_block = { \ .fn = do_no_restart_syscall, \
include/asm-m68k/thread_info.h +1 -0
@@ -9,6 +9,7 @@unsigned long flags;struct exec_domain *exec_domain;/* execution domain */ int preempt_count;/* 0 => preemptable, <0 => BUG */ +int migration_count;/* 0: can migrate, <0 => BUG */ __u32 cpu;/* should always be 0 on m68k */ struct restart_block restart_block; };
include/asm-m68knommu/thread_info.h +1 -0
@@ -37,6 +37,7 @@unsigned long flags;/* low level flags */ int cpu;/* cpu we're on */ int preempt_count;/* 0 => preemptable, <0 => BUG */ +int migration_count;/* 0: can migrate, <0 => BUG */ struct restart_block restart_block; };
include/asm-mips/thread_info.h +2 -0
@@ -28,6 +28,7 @@unsigned long tp_value;/* thread pointer */ __u32 cpu;/* current CPU */ int preempt_count;/* 0 => preemptable, <0 => BUG */ +int migration_count;/* 0: can migrate, <0 => BUG */ mm_segment_t addr_limit;/* thread address space: 0-0xBFFFFFFF for user-thead
@@ -49,6 +50,7 @@ .flags = 0, \ .cpu = 0, \ .preempt_count = 1, \ + .migration_count = 0, \ .addr_limit = KERNEL_DS, \ .restart_block = { \ .fn = do_no_restart_syscall, \
include/asm-parisc/thread_info.h +2 -0
@@ -13,6 +13,7 @@mm_segment_t addr_limit;/* user-level address space limit */ __u32 cpu;/* current CPU */ int preempt_count;/* 0=premptable, <0=BUG; will also serve as bh-counter */ +int migration_count;/* 0: can migrate, <0 => BUG */ struct restart_block restart_block; };
@@ -24,6 +25,7 @@ .cpu = 0, \ .addr_limit = KERNEL_DS, \ .preempt_count = 1, \ + .migration_count = 0, \ .restart_block = { \ .fn = do_no_restart_syscall \ } \
include/asm-powerpc/thread_info.h +2 -0
@@ -35,6 +35,7 @@int cpu;/* cpu we're on */ int preempt_count;/* 0 => preemptable, <0 => BUG */ +int migration_count;/* 0: can migrate, <0 => BUG */ struct restart_block restart_block;unsigned long local_flags;/* private flags for thread */
@@ -53,6 +54,7 @@ .exec_domain = &default_exec_domain, \ .cpu = 0, \ .preempt_count = 1, \ + .migration_count = 0, \ .restart_block = { \ .fn = do_no_restart_syscall, \ }, \
include/asm-s390/thread_info.h +2 -0
@@ -51,6 +51,7 @@unsigned long flags;/* low level flags */ unsigned int cpu;/* current CPU */ int preempt_count;/* 0 => preemptable, <0 => BUG */ +int migration_count;/* 0: can migrate, <0 => BUG */ struct restart_block restart_block; };
@@ -64,6 +65,7 @@ .flags = 0, \ .cpu = 0, \ .preempt_count = 1, \ + .migration_count = 0, \ .restart_block = { \ .fn = do_no_restart_syscall, \ }, \
include/asm-sh/thread_info.h +2 -0
@@ -21,6 +21,7 @@unsigned long flags;/* low level flags */ __u32 cpu;int preempt_count;/* 0 => preemptable, <0 => BUG */ +int migration_count;/* 0: can migrate, <0 => BUG */ mm_segment_t addr_limit;/* thread address space */ struct restart_block restart_block;unsigned long previous_sp;/* sp of previous stack in case
@@ -58,6 +59,7 @@ .flags = 0, \ .cpu = 0, \ .preempt_count = 1, \ + .migration_count = 0, \ .addr_limit = KERNEL_DS, \ .restart_block = { \ .fn = do_no_restart_syscall, \
include/asm-sh64/thread_info.h +2 -0
@@ -23,6 +23,7 @@unsigned long flags;/* low level flags */ /* Put the 4 32-bit fields together to make asm offsetting easier. */ int preempt_count;/* 0 => preemptable, <0 => BUG */ +int migration_count;/* 0: can migrate, <0 => BUG */ __u16 cpu;mm_segment_t addr_limit;
@@ -41,6 +42,7 @@ .flags = 0, \ .cpu = 0, \ .preempt_count = 1, \ + .migration_count = 0, \ .addr_limit = KERNEL_DS, \ .restart_block = { \ .fn = do_no_restart_syscall, \
include/asm-sparc/thread_info.h +2 -0
@@ -33,6 +33,7 @@int cpu;/* cpu we're on */ int preempt_count;/* 0 => preemptable, <0 => BUG */ +int migration_count;/* 0: can migrate, <0 => BUG */ int softirq_count;int hardirq_count;
@@ -65,6 +66,7 @@ .flags = 0, \ .cpu = 0, \ .preempt_count = 1, \ + .migration_count = 0, \ .restart_block = { \ .fn = do_no_restart_syscall, \ }, \
include/asm-sparc64/thread_info.h +2 -0
@@ -47,6 +47,7 @@struct pt_regs *kregs;struct exec_domain *exec_domain;int preempt_count;/* 0 => preemptable, <0 => BUG */ +int migration_count;/* 0: can migrate, <0 => BUG */ __u8 new_child;__u8 syscall_noerror;__u16 cpu;
@@ -137,6 +138,7 @@ .flags = ((unsigned long)ASI_P) << TI_FLAG_CURRENT_DS_SHIFT, \ .exec_domain = &default_exec_domain, \ .preempt_count = 1, \ + .migration_count = 0, \ .restart_block = { \ .fn = do_no_restart_syscall, \ }, \
include/asm-um/thread_info.h +2 -0
@@ -18,6 +18,7 @@__u32 cpu;/* current CPU */ int preempt_count;/* 0 => preemptable, <0 => BUG */ +int migration_count;/* 0: can migrate, <0 => BUG */ mm_segment_t addr_limit;/* thread address space: 0-0xBFFFFFFF for user 0-0xFFFFFFFF for kernel */
@@ -32,6 +33,7 @@ .flags = 0, \ .cpu = 0, \ .preempt_count = 1, \ + .migration_count = 0, \ .addr_limit = KERNEL_DS, \ .restart_block = { \ .fn = do_no_restart_syscall, \
include/asm-v850/thread_info.h +2 -0
@@ -32,6 +32,7 @@int cpu;/* cpu we're on */ int preempt_count;/* 0 => preemptable, <0 => BUG */ +int migration_count;/* 0: can migrate, <0 => BUG */ struct restart_block restart_block; };
@@ -42,6 +43,7 @@ .flags = 0, \ .cpu = 0, \ .preempt_count = 1, \ + .migration_count = 0, \ .restart_block = { \ .fn = do_no_restart_syscall, \ }, \
include/asm-x86_64/thread_info.h +2 -0
@@ -30,6 +30,7 @@__u32 status;/* thread synchronous flags */ __u32 cpu;/* current CPU */ int preempt_count;/* 0 => preemptable, <0 => BUG */ +int migration_count;/* 0: can migrate, <0 => BUG */ mm_segment_t addr_limit;struct restart_block restart_block;
@@ -48,6 +49,7 @@ .flags = 0, \ .cpu = 0, \ .preempt_count = 1, \ + .migration_count = 0, \ .addr_limit = KERNEL_DS, \ .restart_block = { \ .fn = do_no_restart_syscall, \
include/asm-xtensa/thread_info.h +2 -0
@@ -34,6 +34,7 @@unsigned long status;/* thread-synchronous flags */ __u32 cpu;/* current CPU */ __s32 preempt_count;/* 0 => preemptable,< 0 => BUG*/ +__s32 migration_count;/* 0: can migrate, <0 => BUG */ mm_segment_t addr_limit;/* thread address space */ struct restart_block restart_block;
@@ -72,6 +73,7 @@ .flags = 0, \ .cpu = 0, \ .preempt_count = 1, \ + .migration_count = 0, \ .addr_limit = KERNEL_DS, \ .restart_block = { \ .fn = do_no_restart_syscall, \
Signature · 3 lines
Mathieu Desnoyers Computer Engineering Ph.D. Student, Ecole Polytechnique de Montreal OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F BA06 3F25 A8FE 3BAE 9A68
Steven Rostedt
reviewerQuoted 13 lines · Mathieu Desnoyers
Thread Migration Preemption
This patch adds the ability to protect critical sections from migration to another CPU without disabling preemption.
This will be useful to minimize the amount of preemption disabling for the -rt patch. It will help leveraging improvements brought by the
local_ttypes inasm/local.h(seeDocumentation/local_ops.txt). Note that the updates done to variables protected bymigration_disablemust be either atomic or protected from concurrent updates done by other threads.Typical use:
Quoted 8 lines
migration_disable();
local_inc(&__get_cpu_var(&my_local_t_var));migration_enable();Which will increment the variable atomically wrt the local CPU.
Comments (such as how to integrate this in the already almost full
preempt_count) are welcome.
Ingo and Thomas, this also would help with the IRQ thread running a softirq issue. We wouldn't need to bind to a CPU the thread. We could simply disable the ability to migrate while the IRQ thread was handling the softirqs.
-- Steve
Quoted 13 lines · Mathieu Desnoyers
Thread Migration Preemption
This patch adds the ability to protect critical sections from migration to another CPU without disabling preemption.
This will be useful to minimize the amount of preemption disabling for the -rt patch. It will help leveraging improvements brought by the
local_ttypes inasm/local.h(seeDocumentation/local_ops.txt). Note that the updates done to variables protected bymigration_disablemust be either atomic or protected from concurrent updates done by other threads.Typical use:
Quoted 8 lines
migration_disable();
local_inc(&__get_cpu_var(&my_local_t_var));migration_enable();Which will increment the variable atomically wrt the local CPU.
Comments (such as how to integrate this in the already almost full
preempt_count) are welcome.
This seems like way too much stuff to add just for this type of thing. Why not just disable and reenable preempt? Surely local_inc is not going to take so long that disabling preemption matters.
The task struct is not something we should just be carefree putting crap into because it is seemingly free :(
Signature · 1 line
SUSE Labs, Novell Inc.
Steven Rostedt
reviewerQuoted 6 lines · Nick Piggin
Quoted 4 lines · Mathieu Desnoyers
migration_disable();
local_inc(&__get_cpu_var(&my_local_t_var));migration_enable();
[...]
Quoted 3 lines
This seems like way too much stuff to add just for this type of thing. Why not just disable and reenable preempt? Surely
local_incis not going to take so long that disabling preemption matters.
For this given example, it may be too much fine tuning. But there are other things (at least in RT) where this would be very helpful. One thing is that in RT an IRQ thread might service a softirq if that softirq thread is of the same priority as the IRQ thread. The difference between an IRQ thread and a softirq thread is that the IRQ thread may migrate but the softirq thread may not. So to do this performance enhancement, we need to temporarily pin the IRQ thread to the CPU, which is expensive (set_cpus_allowed). This would make it much simpler and light weight to implement.
Quoted 4 lines
The task struct is not something we should just be carefree putting crap into because it is seemingly free :(
Agreed, but as the subject says "RFC". Perhaps we can make it a bit more complex and put this as one of the most significant bits in the preempt_count. We would just need to mask off that bit in all the archs when determining if we should preempt or not. That's more complex, but keeps the task struct free from more luggage.
-- Steve
Quoted 7 lines · Steven Rostedt
thing is that in RT an IRQ thread might service a softirq if that softirq thread is of the same priority as the IRQ thread. The difference between an IRQ thread and a softirq thread is that the IRQ thread may migrate but the softirq thread may not. So to do this performance enhancement, we need to temporarily pin the IRQ thread to the CPU, which is expensive (
set_cpus_allowed). This would make it much simpler and light weight to implement.
Are there any other reasons?
Daniel
Quoted 19 lines · Steven Rostedt
Quoted 7 lines · Nick Piggin
Quoted 4 lines · Mathieu Desnoyers
migration_disable();
local_inc(&__get_cpu_var(&my_local_t_var));migration_enable();[...]
Quoted 3 lines
This seems like way too much stuff to add just for this type of thing. Why not just disable and reenable preempt? Surely
local_incis not going to take so long that disabling preemption matters.
Quoted 9 lines
For this given example, it may be too much fine tuning. But there are other things (at least in RT) where this would be very helpful. One thing is that in RT an IRQ thread might service a softirq if that softirq thread is of the same priority as the IRQ thread. The difference between an IRQ thread and a softirq thread is that the IRQ thread may migrate but the softirq thread may not. So to do this performance enhancement, we need to temporarily pin the IRQ thread to the CPU, which is expensive (
set_cpus_allowed). This would make it much simpler and light weight to implement.
Well if this was just intended for -rt, then OK.
Quoted 10 lines
Quoted 3 lines
The task struct is not something we should just be carefree putting crap into because it is seemingly free :(
Agreed, but as the subject says "RFC". Perhaps we can make it a bit more complex and put this as one of the most significant bits in the
preempt_count. We would just need to mask off that bit in all the archs when determining if we should preempt or not. That's more complex, but keeps the task struct free from more luggage.
Just so long as it stays out of mainline without a good reason that's fine.
Signature · 1 line
SUSE Labs, Novell Inc.
Quoted 21 lines · * Nick Piggin ([email protected]) wrote:
Quoted 20 lines · Mathieu Desnoyers
Thread Migration Preemption
This patch adds the ability to protect critical sections from migration to another CPU without disabling preemption.
This will be useful to minimize the amount of preemption disabling for the -rt
patch. It will help leveraging improvements brought by thelocal_ttypes inasm/local.h(seeDocumentation/local_ops.txt). Note that the updates done to
variables protected bymigration_disablemust be either atomic or protected from
concurrent updates done by other threads.Typical use:
migration_disable();
local_inc(&__get_cpu_var(&my_local_t_var));migration_enable();
Quoted 9 lines
Quoted 4 lines
Which will increment the variable atomically wrt the local CPU.
Comments (such as how to integrate this in the already almost full
preempt_count) are welcome.This seems like way too much stuff to add just for this type of thing. Why not just disable and reenable preempt? Surely
local_incis not going to take so long that disabling preemption matters.
Disabling preemption seems to cause some discomfort in the RT community. Steven already noticed that they could leverage migration disabling to enhance the way they deal with softirqs. Basically, anything that must be tied to a CPU for a short time, can deal with concurrency from other threads running on the same CPU, but where we do not want to add to the scheduler latency would be a good prospect for the migration_disable().
Quoted 3 lines
The task struct is not something we should just be carefree putting crap into because it is seemingly free :(
I agree. I added it knowing that we could either pack this in the preempt_count (using masks, if there is still room, which does not seem to be the case) or, if not, people who could make extensive use of this could justify adding this field to the thread_info struct. I am barely sending the idea out there so it can be helpful for the RT team.
Mathieu
Signature · 3 lines
Mathieu Desnoyers Computer Engineering Ph.D. Student, Ecole Polytechnique de Montreal OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F BA06 3F25 A8FE 3BAE 9A68
Quoted 21 lines · Nick Piggin
Quoted 20 lines · Mathieu Desnoyers
Thread Migration Preemption
This patch adds the ability to protect critical sections from migration to another CPU without disabling preemption.
This will be useful to minimize the amount of preemption disabling for the -rt
patch. It will help leveraging improvements brought by thelocal_ttypes inasm/local.h(seeDocumentation/local_ops.txt). Note that the updates done to
variables protected bymigration_disablemust be either atomic or protected from
concurrent updates done by other threads.Typical use:
migration_disable();
local_inc(&__get_cpu_var(&my_local_t_var));migration_enable();
Quoted 8 lines
Quoted 4 lines
Which will increment the variable atomically wrt the local CPU.
Comments (such as how to integrate this in the already almost full
preempt_count) are welcome.This seems like way too much stuff to add just for this type of thing. Why not just disable and reenable preempt? Surely
local_incis not going to take so long that disabling preemption matters.
I like this patch a lot. Even if we don't add the underlying mechanism right now, adding migration_disable as an alias for preempt_disable will much better document quite a number of the users.
Quoted 2 lines
The task struct is not something we should just be carefree putting crap into because it is seemingly free :(
Sadly, it is free at the moment. We can only fit 3 task_structs in an order-1 SLAB, with lots of slop.
Signature · 1 line
Mathematics is the supreme nostalgia of our time.
Quoted 27 lines · Matt Mackall
Quoted 25 lines · Nick Piggin
Quoted 22 lines · Mathieu Desnoyers
Thread Migration Preemption
This patch adds the ability to protect critical sections from migration to another CPU without disabling preemption.
This will be useful to minimize the amount of preemption disabling for the -rt
patch. It will help leveraging improvements brought by thelocal_ttypes inasm/local.h(seeDocumentation/local_ops.txt). Note that the updates done to variables protected bymigration_disablemust be either atomic or protected from concurrent updates done by other threads.Typical use:
migration_disable();
local_inc(&__get_cpu_var(&my_local_t_var));migration_enable();Which will increment the variable atomically wrt the local CPU.
Comments (such as how to integrate this in the already almost full
preempt_count) are welcome.
Quoted 8 lines
Quoted 3 lines
This seems like way too much stuff to add just for this type of thing. Why not just disable and reenable preempt? Surely
local_incis not going to take so long that disabling preemption matters.I like this patch a lot. Even if we don't add the underlying mechanism right now, adding
migration_disableas an alias forpreempt_disablewill much better document quite a number of the users.
I'd have no problem with that, and it might make it easier in future to justify a more complex scheme.
Quoted 6 lines
Quoted 2 lines
The task struct is not something we should just be carefree putting crap into because it is seemingly free :(
Sadly, it is free at the moment. We can only fit 3
task_structsin an order-1 SLAB, with lots of slop.
Well apart from more cacheline access, that's why I say seemingly free. But actually it uses up space we may like for something else in future and/or makes it harder to shrink down if any effort ever goes into that.
Signature · 1 line
SUSE Labs, Novell Inc.
Quoted 6 lines · Nick Piggin
Quoted 3 lines
I like this patch a lot. Even if we don't add the underlying mechanism right now, adding
migration_disableas an alias forpreempt_disablewill much better document quite a number of the users.I'd have no problem with that, and it might make it easier in future to justify a more complex scheme.
What do you think, Mathieu?
Also, small nit: it ought to be migrate_disable to match the form of preempt_disable.
Quoted 10 lines
Quoted 6 lines
Quoted 2 lines
The task struct is not something we should just be carefree putting crap into because it is seemingly free :(
Sadly, it is free at the moment. We can only fit 3
task_structsin an order-1 SLAB,
with lots of slop.Well apart from more cacheline access, that's why I say seemingly free. But actually it uses up space we may like for something else in future and/or makes it harder to shrink down if any effort ever goes into that.
Yes, I'm just trying to draw the sad state of task_struct to people's attention.
Signature · 1 line
Mathematics is the supreme nostalgia of our time.
Quoted 10 lines · * Matt Mackall ([email protected]) wrote:
Quoted 6 lines · Nick Piggin
Quoted 3 lines
I like this patch a lot. Even if we don't add the underlying mechanism right now, adding
migration_disableas an alias forpreempt_disablewill much better document quite a number of the users.I'd have no problem with that, and it might make it easier in future to justify a more complex scheme.
What do you think, Mathieu?
That's an excellent idea. It should probably come with some documentation explaining what difference should be expected between preempt_disable and migrate_disable in the future so that people can choose the right alternative for their code.
Quoted 2 lines
Also, small nit: it ought to be
migrate_disableto match the form ofpreempt_disable.
Yes, that's fine with me. :)
I though a little more about this whole concept of migrate_disable, and I think it could be brought further. One of the main problems with per cpu variables is that this concept cannot be ported to this grey place called "user-space" because preemption cannot be disabled. The equivalent (kind of) is currently per thread variables, but it can consume a lot of space if many threads are running.
However, it could be possible, if we implement a vsyscall based migration preemption counter accessible with read/write access from user-space, to tie a thread to a CPU during a user-space critical path. If we combine this with local atomic operations done in user-space, we could have highly scalable access to per cpu data structures reentrant with respect to signal handlers.
Mathieu
Signature · 3 lines
Mathieu Desnoyers Computer Engineering Ph.D. Student, Ecole Polytechnique de Montreal OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F BA06 3F25 A8FE 3BAE 9A68
Quoted 34 lines · Mathieu Desnoyers
Quoted 11 lines · * Matt Mackall ([email protected]) wrote:
Quoted 6 lines · Nick Piggin
Quoted 3 lines
I like this patch a lot. Even if we don't add the underlying mechanism right now, adding
migration_disableas an alias forpreempt_disablewill much better document quite a number of the users.I'd have no problem with that, and it might make it easier in future to justify a more complex scheme.
What do you think, Mathieu?
That's an excellent idea. It should probably come with some documentation explaining what difference should be expected between
preempt_disableandmigrate_disablein the future so that people can choose the right alternative for their code.Quoted 2 lines
Also, small nit: it ought to be
migrate_disableto match the form ofpreempt_disable.Yes, that's fine with me. :)
I though a little more about this whole concept of
migrate_disable, and I think it could be brought further. One of the main problems with per cpu variables is that this concept cannot be ported to this grey place called "user-space" because preemption cannot be disabled. The equivalent (kind of) is currently per thread variables, but it can consume a lot of space if many threads are running.
Quoted 6 lines
However, it could be possible, if we implement a vsyscall based migration preemption counter accessible with read/write access from user-space, to tie a thread to a CPU during a user-space critical path. If we combine this with local atomic operations done in user-space, we could have highly scalable access to per cpu data structures reentrant with respect to signal handlers.
That's all well and good, but for most non-trivial stuff, you have to disable preemption as well which you cannot do in userspace. Which I suspect is why there is not a great deal that can use it in kernelspace either.
So it will remain to be seen what kind of per-cpu data structures you can access in a highly scalable way, and how big the niche is between real per-thread data structures and real locking. I'm skeptical :)
Signature · 1 line
SUSE Labs, Novell Inc.
Quoted 4 lines · Mathieu Desnoyers
Thread Migration Preemption
This patch adds the ability to protect critical sections from migration to another CPU without disabling preemption.
Good idea.
I sometimes think we could have avoided _much_ trouble if that had been always default for processes running in kernel space.
Quoted 11 lines
This will be useful to minimize the amount of preemption disabling for the -rt patch. It will help leveraging improvements brought by the
local_ttypes inasm/local.h(seeDocumentation/local_ops.txt). Note that the updates done to variables protected bymigration_disablemust be either atomic or protected from concurrent updates done by other threads.Typical use:
migration_disable();
local_inc(&__get_cpu_var(&my_local_t_var));migration_enable();
It seems strange to have a new interface for this. We already have get_cpu()/put_cpu(). So why not use that?
include/asm-avr32/thread_info.h line 28
Quoted 4 lines
include/asm-avr32/thread_info.h +1
-0unsigned long flags;/* low level flags */ __u32 cpu;__s32 preempt_count;/* 0 => preemptable, <0 => BUG */ +int migration_count;/* 0: can migrate, <0 => BUG */
Can you turn preempt_count into a short first and use a short? That should be enough and cache line usage wouldn't be increased. That's ok on x86; on RISCs int might be faster
-Andi
Quoted 4 lines · * Andi Kleen ([email protected]) wrote:
Quoted 2 lines · Mathieu Desnoyers
Thread Migration Preemption
Quoted 9 lines
Quoted 2 lines
This patch adds the ability to protect critical sections from migration to another CPU without disabling preemption.
Good idea.
I sometimes think we could have avoided _much_ trouble if that had been always default for processes running in kernel space.
I haven't thought about making it the default for kernel space preemption, but yes, it would make sense.
Quoted 6 lines
Quoted 6 lines
This will be useful to minimize the amount of preemption disabling for the -rt patch. It will help leveraging improvements brought by the
local_ttypes inasm/local.h(seeDocumentation/local_ops.txt). Note that the updates done to variables protected bymigration_disablemust be either atomic or protected from concurrent updates done by other threads.
Quoted 9 lines
Quoted 5 lines
Typical use:
migration_disable();
local_inc(&__get_cpu_var(&my_local_t_var));migration_enable();It seems strange to have a new interface for this. We already have get_cpu()/put_cpu(). So why not use that?
Because get/put_cpu() implicitly insure mutual exclusion between threads on the same CPU by disabling preemption. migration_disable() does not:
this is why I only do local atomic operations on these variables.
Quoted 9 lines
include/asm-avr32/thread_info.h line 28
Quoted 4 lines
include/asm-avr32/thread_info.h +1
-0unsigned long flags;/* low level flags */ __u32 cpu;__s32 preempt_count;/* 0 => preemptable, <0 => BUG */ +int migration_count;/* 0: can migrate, <0 => BUG */ Can you turn
preempt_countinto a short first and use a short? That should be enough and cache line usage wouldn't be increased. That's ok on x86; on RISCs int might be faster
using a short instead of an int on modern x86 will cause pipeline stalls due to partial register use. Also, it won't really reduce the cache line usage, since it is followed by an unsigned long; gcc structure alignment will put padding instead of the integer, which does not buy us anything space-wise. If you find me a case on some architectures where it improves performances, I will be more than happy to change it, but as things are, an int seems as efficient, and even more due to partial register stalls on x86, than a short.
Regards,
Mathieu
Quoted 2 lines
-Andi
Signature · 3 lines
Mathieu Desnoyers Computer Engineering Ph.D. Student, Ecole Polytechnique de Montreal OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F BA06 3F25 A8FE 3BAE 9A68
Quoted 2 lines · Mathieu Desnoyers
I haven't thought about making it the default for kernel space preemption, but yes, it would make sense.
Now it's too late -- getcpu() has infected the kernel everywhere. It would have made sense a few years ago.
Quoted 1 line
...
getcpu()...
Hmm ok, although i suspect it's rare to assume that. But understood you don't want to audit all getcpu users because of this.
Quoted 2 lines
using a short instead of an int on modern x86 will cause pipeline stalls due to partial register use.
Sorry, that's totally bogus. Primarily because the access would be directly on memory and there is no partial register tracking there.
Besides pipeline stall is not the correct description on what would happen if you used a register, the worst you get is a single false dependency but no pipeline flush.
Besides the latest x86 cpus (C2, K8) don't have much trouble with these false dependencies in general.
Quoted 4 lines
usage, since it is followed by an unsigned long; gcc structure alignment
will put padding instead of the integer, which does not buy us anything
on i386 unsigned long is 4 bytes.
-Andi
Quoted 19 lines · * Andi Kleen ([email protected]) wrote:
Quoted 2 lines · Mathieu Desnoyers
I haven't thought about making it the default for kernel space preemption, but yes, it would make sense.
Now it's too late --
getcpu()has infected the kernel everywhere. It would have made sense a few years ago.Quoted 1 line
...
getcpu()...Hmm ok, although i suspect it's rare to assume that. But understood you don't want to audit all getcpu users because of this.
Quoted 2 lines
using a short instead of an int on modern x86 will cause pipeline stalls due to partial register use.
Sorry, that's totally bogus. Primarily because the access would be directly on memory and there is no partial register tracking there.
Quoted 7 lines
Besides pipeline stall is not the correct description on what would happen if you used a register, the worst you get is a single false dependency but no pipeline flush.
Besides the latest x86 cpus (C2, K8) don't have much trouble with these false dependencies in general.
Yes, false dependency is what I meant. And hrm, yeah I guess that mostly movzbl would be used to get the byte from memory and zero-extend, making sure there is no false dependency.
Quoted 7 lines
Quoted 4 lines
usage, since it is followed by an unsigned long; gcc structure alignment
will put padding instead of the integer, which does not buy us anything
on i386 unsigned long is 4 bytes.
Since we have, on i386:
int preempt_count; /* 0 => preemptable, <0 => BUG */
int migrate_count;/* 0: can migrate, <0: BUG */
mm_segment_t addr_limit; /* thread address space:and:
typedef struct {
unsigned long seg;
} mm_segment_t;Turning migrate_count into a char would only put padding between migrate_count and addr_limit. So I still do not see a clear improvement in memory usage there... but I don't specially care about it being an integer or a byte though :)
Mathieu
Quoted 1 line
-Andi
Signature · 3 lines
Mathieu Desnoyers Computer Engineering Ph.D. Student, Ecole Polytechnique de Montreal OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F BA06 3F25 A8FE 3BAE 9A68
Quoted 7 lines · Mathieu Desnoyers
Thread Migration Preemption
This patch adds the ability to protect critical sections from migration to another CPU without disabling preemption.
This will be useful to minimize the amount of preemption disabling for the -rt patch. [...]
unfortunately this abstraction has a number of fundamental disadvantages. (and hence the -rt patch does not make use of this facility)
firstly, its effects on load-balancing are quite brutal: it disables migration to another CPU, hence disturbing SMP load-balancing. It also adds an unplannable O(N) overhead into the load-balancer.
secondly, it has similar problems as preempt_disable(): it's opaque (not attached to a data structure) and thus it does not truly map the per-CPU data structure in any explicit way. It's little more than a per-task BKL for per-CPU data structures.
so in -rt we resisted the disable-migration approach from the early days on and are using two approaches to 'map' per-cpu data structures toPREEMPT_RT: firstly the use of PER_CPU_LOCKED data structures (which attach a per-cpu sleeping lock to per_cpu() data structures) - these are relatively easy first-approach conversions. As a second approach, if performance matters, the use of true atomic data structures.
Ingo