cpu_stop: implement and use cpu_stop
Subsystems (5)
PATCH 1/1 cpu_stop: implement and use cpu_stop
Hello, all.
cpu_hog has been renamed to cpu_stop and moved into kernel/stop_machine.c per Peter Zijlstra's suggestion. This patchset is feature-wise identical to the second take of cpuhog[L]. The only changes are the rename, relocation and refresh against the current sched/core.
The following API renames took place.
- hog_one_cpu() -> stop_one_cpu()
- hog_one_cpu_nowait() -> stop_one_cpu_nowait()
- hog_cpus() -> stop_cpus()
- try_hog_cpus() -> try_stop_cpus()
- *_hog() callbacks -> *_cpu_stop()
Internal names have been renamed accordingly. e.g. cpuhog thread became cpu_stopper thread and so on.
This patchset contains the following four patches.
0001-cpu_stop-implement-stop_cpu-s.patch 0002-stop_machine-reimplement-using-cpu_stop.patch 0003-scheduler-replace-migration_thread-with-cpu_stop.patch 0004-scheduler-kill-paranoia-check-in-synchronize_sched_e.patch
The patches are against the current linux-2.6-tip/sched/core (09a40af5240de02d848247ab82440ad75b31ab11) and are available in the following git tree.
git://git.kernel.org/pub/scm/linux/kernel/git/tj/misc.git cpu_stop
I retained the original acked/reviewed-by's as the changes are mostly cosmetic. If you disagree, please let me know. I'll try to push this through sched/core again once Peter acks.
diffstat follows.
Documentation/RCU/torture.txt | 10 arch/s390/kernel/time.c | 1 drivers/xen/manage.c | 14 - include/linux/rcutiny.h | 2 include/linux/rcutree.h | 1 include/linux/stop_machine.h | 59 ++-- kernel/cpu.c | 8 kernel/module.c | 14 - kernel/rcutorture.c | 2 kernel/sched.c | 271 +++------------------ kernel/sched_fair.c | 42 ++- kernel/stop_machine.c | 525 ++++++++++++++++++++++++++++++++---------- 12 files changed, 514 insertions(+), 435 deletions(-)
Thanks.
--
tejun
PATCH 1/4 cpu_stop: implement stop_cpu[s]()
Implement a simplistic per-cpu maximum priority cpu monopolization mechanism. A non-sleeping callback can be scheduled to run on one or multiple cpus with maximum priority monopolozing those cpus. This is primarily to replace and unify RT workqueue usage in stop_machine and scheduler migration_thread which currently is serving multiple purposes.
Four functions are provided - stop_one_cpu(), stop_one_cpu_nowait(), stop_cpus() and try_stop_cpus().
This is to allow clean sharing of resources among stop_cpu and all the migration thread users. One stopper thread per cpu is created which is currently named "stopper/CPU". This will eventually replace the migration thread and take on its name.
* This facility was originally named cpuhog and lived in separate files but Peter Zijlstra nacked the name and thus got renamed to cpu_stop and moved into stop_machine.c.
--- include/linux/stop_machine.h | 39 ++++- kernel/stop_machine.c | 367 +++++++++++++++++++++++++++++++++++++++++- 2 files changed, 397 insertions(+), 9 deletions(-)
include/linux/stop_machine.h +35 -4
@@ -1,15 +1,46 @@#ifndef _LINUX_STOP_MACHINE#define _LINUX_STOP_MACHINE -/* "Bogolock": stop the entire machine, disable interrupts. This is a -very heavy lock, which is equivalent to grabbing every spinlock -(and more). So the "read" side to such a lock is anything which -disables preeempt. */ +#include <linux/cpu.h> #include <linux/cpumask.h> +#include <linux/list.h> #include <asm/system.h> #if defined(CONFIG_STOP_MACHINE ) && defined(CONFIG_SMP ) +/* +* stop_cpu[s]() is simplistic per-cpu maximum priority cpu +* monopolization mechanism. The caller can specify a non-sleeping +* function to be executed on a single or multiple cpus preempting all +* other processes and monopolizing those cpus until it finishes. +* +* Resources for this mechanism are preallocated when a cpu is brought +* up and requests are guaranteed to be served as long as the target +* cpus are online. +*/ + +typedef int (*cpu_stop_fn_t )(void *arg); + +struct cpu_stop_work { +struct list_head list;/* cpu_stopper->works */ +cpu_stop_fn_t fn; +void *arg; +struct cpu_stop_done *done; +}; + +int stop_one_cpu (unsigned int cpu,cpu_stop_fn_t fn,void *arg); +void stop_one_cpu_nowait (unsigned int cpu,cpu_stop_fn_t fn,void *arg, +struct cpu_stop_work *work_buf); +int stop_cpus (const struct cpumask *cpumask,cpu_stop_fn_t fn,void *arg); +int try_stop_cpus (const struct cpumask *cpumask,cpu_stop_fn_t fn,void *arg); + +/* +* stop_machine "Bogolock": stop the entire machine, disable +* interrupts. This is a very heavy lock, which is equivalent to +* grabbing every spinlock (and more). So the "read" side to such a +* lock is anything which disables preeempt. +*/ +/** * stop_machine: freeze the machine on all CPUs and run this function * @fn: the function to run
kernel/stop_machine.c +362 -5
379 lines not on this page · raw message
Signature · 1 line
1.6.4.2
Peter Zijlstra
maintainerRe: [PATCH 1/4] cpu_stop: implement stop_cpu[s]()
Quoted 37 lines · Tejun Heo
kernel/stop_machine.c +35
-0+struct cpu_stop_done { +atomic_t nr_todo;/* nr left to execute */ +bool executed;/* actually executed? */ +int ret;/* collected return value */ +struct completion completion;/* fired if nr_todo reaches 0 */ +}; + +/* signal completion unless @done is NULL */ +static void cpu_stop_signal_done (struct cpu_stop_done *done,bool executed) +{ +if (done) { +if (executed) + done->executed = true; +if (atomic_dec_and_test (&done->nr_todo)) +complete (&done->completion); + } +} + +/* queue @work to @stopper. if offline, @work is completed immediately */ +static void cpu_stop_queue_work (struct cpu_stopper *stopper, +struct cpu_stop_work *work) +{ +unsigned long flags; + +spin_lock_irqsave (&stopper->lock, flags); + +if (stopper->enabled) { +list_add_tail (&work->list, &stopper->works); +wake_up_process (stopper->thread); + }else +cpu_stop_signal_done (work->done, false); + +spin_unlock_irqrestore (&stopper->lock, flags); +} +
kernel/stop_machine.c line 112
Quoted 10 lines
kernel/stop_machine.c +10
-0+int stop_one_cpu (unsigned int cpu,cpu_stop_fn_t fn,void *arg) +{ +struct cpu_stop_done done; +struct cpu_stop_work work = { .fn = fn, .arg = arg, .done = &done }; + +cpu_stop_init_done (&done,1 ); +cpu_stop_queue_work (&per_cpu (cpu_stopper, cpu), &work); +wait_for_completion (&done.completion); +return done.executed ? done.ret : -ENOENT ; +}
If you do:
done = { .ret = -ENOENT, };
And remove that if()
kernel/stop_machine.c line 273
Quoted 4 lines
kernel/stop_machine.c +4
-0+ ret = work->fn (work->arg); +if (ret) + done->ret = ret; +
You can do away with all the ->executed bits.
Re: [PATCH 1/4] cpu_stop: implement stop_cpu[s]()
Hello,
Quoted 4 lines · Peter Zijlstra
If you do:
done = { .ret =
-ENOENT, };
Quoted 8 lines
And remove that
if()kernel/stop_machine.c line 273
Quoted 4 lines
kernel/stop_machine.c +4
-0+ ret = work->fn (work->arg); +if (ret) + done->ret = ret; +You can do away with all the ->executed bits.
Oh, I had code piece which wanted to discern between -ENOENT from non-excution and -ENOENT return from the work function which seems gone now. I'll check things again and drop ->executed if everything looks okay.
Thanks.
Signature · 1 line
tejun
Re: [PATCH 1/4] cpu_stop: implement stop_cpu[s]()
Hello, again.
Quoted 4 lines · Tejun Heo
Oh, I had code piece which wanted to discern between
-ENOENTfrom non-excution and-ENOENTreturn from the work function which seems gone now. I'll check things again and drop ->executed if everything looks okay.
Eh... now I remember. If we start with ->ret = 0, stop_cpus() can't return -ENOENT when none of the specified cpus executed without tracking execution status (so the current code). If we start with ->ret = -ENOENT, we can't tell whether all cpus executed successfully or none has executed unless we BUG_ON() -ENOENT return from work functions and let 0 return override -ENOENT.
Thanks.
Signature · 1 line
tejun
Peter Zijlstra
maintainerRe: [PATCH 1/4] cpu_stop: implement stop_cpu[s]()
In reply to Tejun Heokernel/stop_machine.c line 241
Quoted 3 lines · Tejun Heo
kernel/stop_machine.c +3
-0+static int cpu_stopper_thread (void *data) +{ +struct cpu_stopper *stopper = data;
BUG_ON(stopper != __get_cpu_var(cpu_stopper)); ?
kernel/stop_machine.c line 260
Quoted 8 lines
kernel/stop_machine.c +8
-0+ work =NULL ; +spin_lock_irq (&stopper->lock); +if (!list_empty (&stopper->works)) { + work =list_first_entry (&stopper->works, + struct cpu_stop_work, list); +list_del_init (&work->list); + } +spin_unlock_irq (&stopper->lock);
Not sure if its worth the hassle, but you could list_splice_init() the complete pending list onto a local list, possible avoiding some locks.
But since this isn't supposed to be used much, I doubt we'll ever see the difference.
kernel/stop_machine.c line 276
Quoted 3 lines
kernel/stop_machine.c +3
-0+/* restore preemption and check it's still balanced */ +preempt_enable (); +WARN_ON_ONCE (preempt_count ());
You would use WARN_ONCE() and print the function that last ran and leaked the preempt count.
Re: [PATCH 1/4] cpu_stop: implement stop_cpu[s]()
Hello,
Quoted 6 lines · Peter Zijlstra
kernel/stop_machine.c line 241
Quoted 3 lines · Tejun Heo
kernel/stop_machine.c +3
-0+static int cpu_stopper_thread (void *data) +{ +struct cpu_stopper *stopper = data;
BUG_ON(stopper != __get_cpu_var(cpu_stopper)); ?
Added.
Quoted 9 lines
kernel/stop_machine.c line 260
Quoted 8 lines
kernel/stop_machine.c +8
-0+ work =NULL ; +spin_lock_irq (&stopper->lock); +if (!list_empty (&stopper->works)) { + work =list_first_entry (&stopper->works, + struct cpu_stop_work, list); +list_del_init (&work->list); + } +spin_unlock_irq (&stopper->lock);
Quoted 5 lines
Not sure if its worth the hassle, but you could
list_splice_init()the complete pending list onto a local list, possible avoiding some locks.But since this isn't supposed to be used much, I doubt we'll ever see the difference.
Yeah, I think it would be better to keep the code simple there.
Quoted 6 lines
kernel/stop_machine.c line 276
Quoted 3 lines
kernel/stop_machine.c +3
-0+/* restore preemption and check it's still balanced */ +preempt_enable (); +WARN_ON_ONCE (preempt_count ());You would use
WARN_ONCE()and print the function that last ran and leaked the preempt count.
Updated to use WARN_ONCE() w/ print the function symbol and argument.
Thanks.
Signature · 1 line
tejun
Re: [PATCH 1/4] cpu_stop: implement stop_cpu[s]()
Quoted 9 lines · Tejun Heo
Quoted 6 lines · Peter Zijlstra
kernel/stop_machine.c line 241
Quoted 3 lines · Tejun Heo
kernel/stop_machine.c +3
-0+static int cpu_stopper_thread (void *data) +{ +struct cpu_stopper *stopper = data;
BUG_ON(stopper != __get_cpu_var(cpu_stopper)); ?Added.
Now that I think more about it, there's a subtle race condition with the above BUG_ON(). Stoppers are prepared by CPU_UP_PREPARE and started by CPU_ONLINE but brought down by CPU_DEAD. IOW, they're allowed to run detached from their designated CPUs between CPU_DYING and CPU_DEAD (the reponsibility of guaranteeing target cpus's onliness is on the callers). So, the above BUG_ON() might trigger spuriously if a cpu goes down after brought online before its cpu_stopper had a chance to pass through the BUG_ON() test.
Thanks.
Signature · 1 line
tejun
Peter Zijlstra
maintainerRe: [PATCH 1/4] cpu_stop: implement stop_cpu[s]()
Quoted 11 lines · Tejun Heo
Quoted 9 lines · Tejun Heo
Quoted 6 lines · Peter Zijlstra
kernel/stop_machine.c line 241
Quoted 3 lines · Tejun Heo
kernel/stop_machine.c +3
-0+static int cpu_stopper_thread (void *data) +{ +struct cpu_stopper *stopper = data;
BUG_ON(stopper != __get_cpu_var(cpu_stopper)); ?Added.
Quoted 8 lines
Now that I think more about it, there's a subtle race condition with the above
BUG_ON(). Stoppers are prepared byCPU_UP_PREPAREand started byCPU_ONLINEbut brought down byCPU_DEAD. IOW, they're allowed to run detached from their designated CPUs betweenCPU_DYINGandCPU_DEAD(the reponsibility of guaranteeing target cpus's onliness is on the callers). So, the aboveBUG_ON()might trigger spuriously if a cpu goes down after brought online before itscpu_stopperhad a chance to pass through theBUG_ON()test.
Ah indeed. A well, drop it then, its not worth making a more complicated test.
Peter Zijlstra
maintainerRe: [PATCH 1/4] cpu_stop: implement stop_cpu[s]()
In reply to Tejun Heokernel/stop_machine.c line 132
Quoted 7 lines · Tejun Heo
+void stop_one_cpu_nowait(unsigned int cpu,
cpu_stop_fn_tfn, void *arg,kernel/stop_machine.c +5
-0+ struct cpu_stop_work *work_buf) +{ +memset (work_buf,0 ,sizeof (*work_buf)); + work_buf->fn = fn; + work_buf->arg = arg;
*work_buf = (struct cpu_stop_work){ .fn = fn, .arg = arg, };
Re: [PATCH 1/4] cpu_stop: implement stop_cpu[s]()
Quoted 11 lines · Peter Zijlstra
kernel/stop_machine.c line 132
Quoted 7 lines · Tejun Heo
+void stop_one_cpu_nowait(unsigned int cpu,
cpu_stop_fn_tfn, void *arg,kernel/stop_machine.c +5
-0+ struct cpu_stop_work *work_buf) +{ +memset (work_buf,0 ,sizeof (*work_buf)); + work_buf->fn = fn; + work_buf->arg = arg;
*work_buf= (struct cpu_stop_work){ .fn = fn, .arg = arg, };
Updated.
Signature · 1 line
tejun
PATCH 2/4 stop_machine: reimplement using cpu_stop
In reply to Tejun HeoReimplement stop_machine using cpu_stop. As cpu stoppers are guaranteed to be available for all online cpus, stop_machine_create/destroy() are no longer necessary and removed.
With resource management and synchronization handled by cpu_stop, the new implementation is much simpler. Asking the cpu_stop to execute the stop_cpu() state machine on all online cpus with cpu hotplug disabled is enough.
stop_machine itself doesn't need to manage any global resources anymore, so all per-instance information is rolled into struct stop_machine_data and the mutex and all static data variables are removed.
The previous implementation created and destroyed RT workqueues as necessary which made stop_machine() calls highly expensive on very large machines. According to Dimitri Sivanich, preventing the dynamic creation/destruction makes booting faster more than twice on very large machines. cpu_stop resources are preallocated for all online cpus and should have the same effect.
--- arch/s390/kernel/time.c | 1 - drivers/xen/manage.c | 14 +--- include/linux/stop_machine.h | 20 ----- kernel/cpu.c | 8 -- kernel/module.c | 14 +--- kernel/stop_machine.c | 158 ++++++++++-------------------------------- 6 files changed, 42 insertions(+), 173 deletions(-)
arch/s390/kernel/time.c +0 -1
@@ -390,7 +390,6 @@ static void __init time_init_wq(void)if (time_sync_wq)return ; time_sync_wq =create_singlethread_workqueue ("timesync" ); -stop_machine_create (); }/*
drivers/xen/manage.c +2 -12
@@ -80,12 +80,6 @@ static void do_suspend(void) shutting_down =SHUTDOWN_SUSPEND ; - err =stop_machine_create (); -if (err) { -printk (KERN_ERR "xen suspend: failed to setup stop_machine %d\n" , err); - goto out; - } -#ifdef CONFIG_PREEMPT /* If the kernel is preemptible, we need to freeze all the processes to prevent them from being in the middle of a pagetable update
@@ -93,7 +87,7 @@ static void do_suspend(void) err =freeze_processes ();if (err) {printk (KERN_ERR "xen suspend: freeze failed %d\n" , err); - goto out_destroy_sm; + goto out; }#endif
@@ -136,12 +130,8 @@ out_resume: out_thaw: #ifdef CONFIG_PREEMPTthaw_processes (); - -out_destroy_sm: -#endif -stop_machine_destroy (); - out: +#endif shutting_down =SHUTDOWN_INVALID ; }#endif /* CONFIG_PM_SLEEP */
include/linux/stop_machine.h +0 -20
@@ -67,23 +67,6 @@ int stop_machine(int (*fn)(void *), void *data, const struct cpumask *cpus);*/ int __stop_machine (int (*fn)(void *),void *data,const struct cpumask *cpus); -/** -* stop_machine_create: create all stop_machine threads -* -* Description: This causes all stop_machine threads to be created before -* stop_machine actually gets called. This can be used by subsystems that -* need a non failing stop_machine infrastructure. -*/ -int stop_machine_create (void ); - -/** -* stop_machine_destroy: destroy all stop_machine threads -* -* Description: This causes all stop_machine threads which were created with -* stop_machine_create to be destroyed again. -*/ -void stop_machine_destroy (void ); -#else static inline int stop_machine(int (*fn)(void *), void *data,
@@ -96,8 +79,5 @@ static inline int stop_machine(int (*fn)(void *), void *data,return ret; } -static inline int stop_machine_create (void ) {return 0 ; } -static inline void stop_machine_destroy (void ) { } -#endif /* CONFIG_SMP */ #endif /* _LINUX_STOP_MACHINE */
kernel/cpu.c +0 -8
@@ -266,9 +266,6 @@ int __ref cpu_down(unsigned int cpu) {int err; - err =stop_machine_create (); -if (err) -return err;cpu_maps_update_begin ();if (cpu_hotplug_disabled) {
@@ -280,7 +277,6 @@ int __ref cpu_down(unsigned int cpu) out:cpu_maps_update_done (); -stop_machine_destroy ();return err; }EXPORT_SYMBOL (cpu_down);
@@ -361,9 +357,6 @@ int disable_nonboot_cpus(void) {int cpu, first_cpu, error; - error =stop_machine_create (); -if (error) -return error;cpu_maps_update_begin (); first_cpu =cpumask_first (cpu_online_mask);/*
@@ -394,7 +387,6 @@ int disable_nonboot_cpus(void)printk (KERN_ERR "Non-boot CPUs are not disabled\n" ); }cpu_maps_update_done (); -stop_machine_destroy ();return error; }
kernel/module.c +2 -12
@@ -723,16 +723,8 @@ SYSCALL_DEFINE2(delete_module, const char __user *, name_user,return -EFAULT ; name[MODULE_NAME_LEN -1 ] ='\0' ; -/* Create stop_machine threads since free_module relies on -* a non-failing stop_machine call. */ - ret =stop_machine_create (); -if (ret) -return ret; - -if (mutex_lock_interruptible (&module_mutex) !=0 ) { - ret = -EINTR ; - goto out_stop; - } +if (mutex_lock_interruptible (&module_mutex) !=0 ) +return -EINTR ; mod =find_module (name);if (!mod) {
@@ -792,8 +784,6 @@ SYSCALL_DEFINE2(delete_module, const char __user *, name_user, out:mutex_unlock (&module_mutex); -out_stop: -stop_machine_destroy ();return ret; }
kernel/stop_machine.c +38 -120
212 lines not on this page · raw message
Signature · 1 line
1.6.4.2
PATCH 3/4 scheduler: replace migration_thread with cpu_stop
In reply to Tejun HeoCurrently migration_thread is serving three purposes - migration pusher, context to execute active_load_balance() and forced context switcher for expedited RCU synchronize_sched. All three roles are hardcoded into migration_thread() and determining which job is scheduled is slightly messy.
This patch kills migration_thread and replaces all three uses with cpu_stop. The three different roles of migration_thread() are splitted into three separate cpu_stop callbacks - migration_cpu_stop(), active_load_balance_cpu_stop() and synchronize_sched_expedited_cpu_stop() - and each use case now simply asks cpu_stop to execute the callback as necessary.
synchronize_sched_expedited() was implemented with private preallocated resources and custom multi-cpu queueing and waiting logic, both of which are provided by cpu_stop. synchronize_sched_expedited_count is made atomic and all other shared resources along with the mutex are dropped.
synchronize_sched_expedited() also implemented a check to detect cases where not all the callback got executed on their assigned cpus and fall back to synchronize_sched(). If called with cpu hotplug blocked, cpu_stop already guarantees that and the condition cannot happen; otherwise, stop_machine() would break. However, this patch preserves the paranoid check using a cpumask to record on which cpus the stopper ran so that it can serve as a bisection point if something actually goes wrong theree.
Because the internal execution state is no longer visible, rcu_expedited_torture_stats() is removed.
This patch also renames cpu_stop threads to from "stopper/%d" to "migration/%d". The names of these threads ultimately don't matter and there's no reason to make unnecessary userland visible changes.
With this patch applied, stop_machine() and sched now share the same resources. stop_machine() is faster without wasting any resources and sched migration users are much cleaner.
--- Documentation/RCU/torture.txt | 10 -- include/linux/rcutiny.h | 2 - include/linux/rcutree.h | 1 - kernel/rcutorture.c | 2 +- kernel/sched.c | 303 +++++++++++------------------------------ kernel/sched_fair.c | 42 ++++-- kernel/stop_machine.c | 2 +- 7 files changed, 109 insertions(+), 253 deletions(-)
Documentation/RCU/torture.txt +0 -10
@@ -182,16 +182,6 @@ Similarly, sched_expedited RCU provides the following: sched_expedited-torture: Reader Pipe: 12660320201 95875 0 0 0 0 0 0 0 0 0 sched_expedited-torture: Reader Batch: 12660424885 0 0 0 0 0 0 0 0 0 0 sched_expedited-torture: Free-Block Circulation: 1090795 1090795 1090794 1090793 1090792 1090791 1090790 1090789 1090788 1090787 0 - state: -1 / 0:0 3:0 4:0 - -As before, the first four lines are similar to those for RCU. -The last line shows the task-migration state. The first number is --1 if synchronize_sched_expedited() is idle, -2 if in the process of -posting wakeups to the migration kthreads, and N when waiting on CPU N. -Each of the colon-separated fields following the "/" is a CPU:state pair. -Valid states are "0" for idle, "1" for waiting for quiescent state, -"2" for passed through quiescent state, and "3" when a race with a -CPU-hotplug event forces use of the synchronize_sched() primitive. USAGE
include/linux/rcutiny.h +0 -2
@@ -60,8 +60,6 @@ static inline long rcu_batches_completed_bh(void)return 0 ; } -extern int rcu_expedited_torture_stats (char *page); -static inline void rcu_force_quiescent_state (void ) { }
include/linux/rcutree.h +0 -1
@@ -35,7 +35,6 @@ struct notifier_block;extern void rcu_sched_qs (int cpu);extern void rcu_bh_qs (int cpu);extern int rcu_needs_cpu (int cpu); -extern int rcu_expedited_torture_stats (char *page);#ifdef CONFIG_TREE_PREEMPT_RCU
kernel/rcutorture.c +1 -1
@@ -669,7 +669,7 @@ static struct rcu_torture_ops sched_expedited_ops = { .sync = synchronize_sched_expedited, .cb_barrier = NULL, .fqs = rcu_sched_force_quiescent_state, - .stats = rcu_expedited_torture_stats, + .stats = NULL, .irq_capable = 1, .name = "sched_expedited" };
kernel/sched.c +79 -224
475 lines not on this page · raw message
kernel/sched_fair.c +28 -14
79 lines not on this page · raw message
kernel/stop_machine.c +1 -1
@@ -296,7 +296,7 @@ static int __cpuinit cpu_stop_cpu_callback(struct notifier_block *nfb,case CPU_UP_PREPARE :BUG_ON (stopper->thread || stopper->enabled || !list_empty (&stopper->works)); - p =kthread_create (cpu_stopper_thread, stopper,"stopper/%d" , + p =kthread_create (cpu_stopper_thread, stopper,"migration/%d" , cpu);if (IS_ERR (p))return NOTIFY_BAD ;
Signature · 1 line
1.6.4.2
Peter Zijlstra
maintainerRe: [PATCH 3/4] scheduler: replace migration_thread with cpu_stop
kernel/sched_fair.c redo: · line 2917
Quoted 8 lines · Tejun Heo
kernel/sched_fair.c +3
-1@@ -2909,7 +2912,9 @@ redo: }raw_spin_unlock_irqrestore (&busiest->lock, flags);if (active_balance) -wake_up_process (busiest->migration_thread); +stop_one_cpu_nowait (cpu_of (busiest), + active_load_balance_cpu_stop, busiest, + &busiest->active_balance_work);
So who guarantees busiest->active_balance_work isn't already enqueued by some other cpu's load-balancer run?
Re: [PATCH 3/4] scheduler: replace migration_thread with cpu_stop
Hello,
Quoted 13 lines · Peter Zijlstra
kernel/sched_fair.c redo: · line 2917
Quoted 8 lines · Tejun Heo
kernel/sched_fair.c +3
-1@@ -2909,7 +2912,9 @@ redo: }raw_spin_unlock_irqrestore (&busiest->lock, flags);if (active_balance) -wake_up_process (busiest->migration_thread); +stop_one_cpu_nowait (cpu_of (busiest), + active_load_balance_cpu_stop, busiest, + &busiest->active_balance_work);So who guarantees
busiest->active_balance_workisn't already enqueued by some other cpu's load-balancer run?
Hmmm... maybe I'm mistaken but isn't that guaranteed by busiest->active_balance which is protected by the rq lock? active_load_balance_cpu_stop is scheduled iff busiest->active_balance was changed from zero and only active_load_balance_cpu_stop() can clear it at the end of its execution at which point the active_balance_work is safe to reuse.
Thanks.
Signature · 1 line
tejun
Peter Zijlstra
maintainerRe: [PATCH 3/4] scheduler: replace migration_thread with cpu_stop
Quoted 17 lines · Tejun Heo
Hello,
Quoted 13 lines · Peter Zijlstra
kernel/sched_fair.c redo: · line 2917
Quoted 8 lines · Tejun Heo
kernel/sched_fair.c +3
-1@@ -2909,7 +2912,9 @@ redo: }raw_spin_unlock_irqrestore (&busiest->lock, flags);if (active_balance) -wake_up_process (busiest->migration_thread); +stop_one_cpu_nowait (cpu_of (busiest), + active_load_balance_cpu_stop, busiest, + &busiest->active_balance_work);So who guarantees
busiest->active_balance_workisn't already enqueued by some other cpu's load-balancer run?
Quoted 6 lines
Hmmm... maybe I'm mistaken but isn't that guaranteed by
busiest->active_balancewhich is protected by the rq lock?active_load_balance_cpu_stopis scheduled iffbusiest->active_balancewas changed from zero and onlyactive_load_balance_cpu_stop()can clear it at the end of its execution at which point theactive_balance_workis safe to reuse.
Ah, indeed. It wasn't obvious from looking at the patch, but when looking at the full code it fairly easy to see.
Re: [PATCH 3/4] scheduler: replace migration_thread with cpu_stop
Quoted 9 lines · Peter Zijlstra
Quoted 6 lines
Hmmm... maybe I'm mistaken but isn't that guaranteed by
busiest->active_balancewhich is protected by the rq lock?active_load_balance_cpu_stopis scheduled iffbusiest->active_balancewas changed from zero and onlyactive_load_balance_cpu_stop()can clear it at the end of its execution at which point theactive_balance_workis safe to reuse.Ah, indeed. It wasn't obvious from looking at the patch, but when looking at the full code it fairly easy to see.
Hmmm... it's probably worthwhile to note tho. I'll add a comment and send out the updated patches soon.
Thanks.
Signature · 1 line
tejun
PATCH 4/4 scheduler: kill paranoia check in synchronize_sched_expedited()
In reply to Tejun HeoThe paranoid check which verifies that the cpu_stop callback is actually called on all online cpus is completely superflous. It's guaranteed by cpu_stop facility and if it didn't work as advertised other things would go horribly wrong and trying to recover using synchronize_sched() wouldn't be very meaningful.
Kill the paranoid check. Removal of this feature is done as a separate step so that it can serve as a bisection point if something actually goes wrong.
--- kernel/sched.c | 40 +++------------------------------------- 1 files changed, 3 insertions(+), 37 deletions(-)
kernel/sched.c +3 -37
@@ -8884,14 +8884,6 @@ static atomic_t synchronize_sched_expedited_count = ATOMIC_INIT(0);static int synchronize_sched_expedited_cpu_stop (void *data) { -static DEFINE_SPINLOCK (done_mask_lock); -struct cpumask *done_mask = data; - -if (done_mask) { -spin_lock (&done_mask_lock); -cpumask_set_cpu (smp_processor_id (), done_mask); -spin_unlock (&done_mask_lock); - }return 0 ; }
@@ -8907,55 +8899,29 @@ static int synchronize_sched_expedited_cpu_stop(void *data)*/ void synchronize_sched_expedited (void ) { -cpumask_var_t done_mask_var; -struct cpumask *done_mask =NULL ;int snap, trycount =0 ; -/* -* done_mask is used to check that all cpus actually have -* finished running the stopper, which is guaranteed by -* stop_cpus() if it's called with cpu hotplug blocked. Keep -* the paranoia for now but it's best effort if cpumask is off -* stack. -*/ -if (zalloc_cpumask_var (&done_mask_var,GFP_ATOMIC )) - done_mask = done_mask_var; -smp_mb ();/* ensure prior mod happens before capturing snap. */ snap =atomic_read (&synchronize_sched_expedited_count) +1 ;get_online_cpus ();while (try_stop_cpus (cpu_online_mask, synchronize_sched_expedited_cpu_stop, - done_mask) == -EAGAIN ) { +NULL ) == -EAGAIN ) {put_online_cpus ();if (trycount++ <10 )udelay (trycount *num_online_cpus ());else {synchronize_sched (); - goto free_out; +return ; }if (atomic_read (&synchronize_sched_expedited_count) - snap >0 ) {smp_mb ();/* ensure test happens before caller kfree */ - goto free_out; +return ; }get_online_cpus (); }atomic_inc (&synchronize_sched_expedited_count); -if (done_mask) -cpumask_xor (done_mask, done_mask, cpu_online_mask);put_online_cpus (); - -/* paranoia - this can't happen */ -if (done_mask &&cpumask_weight (done_mask)) { -char buf[80 ]; - -cpulist_scnprintf (buf,sizeof (buf), done_mask); -WARN_ONCE (1 ,"synchronize_sched_expedited: cpu online and done masks disagree on %d cpus: %s\n" , -cpumask_weight (done_mask), buf); -synchronize_sched (); - } -free_out: -free_cpumask_var (done_mask_var); }EXPORT_SYMBOL_GPL (synchronize_sched_expedited);
Signature · 1 line
1.6.4.2