Re: [RFC PATCH(Experimental) 2/4] Revert changes to workqueue.c

2007-02-21 Thread Srivatsa Vaddagiri
On Wed, Feb 21, 2007 at 05:30:10PM +0300, Oleg Nesterov wrote: > Agreed. Note that we don't need the new "del_work". It is always safe to > use cancel_work_sync() if we know that the workqueue is frozen, it won't > block. We can also do > > if (!cancel_delayed_work()) >

Re: [RFC PATCH(Experimental) 2/4] Revert changes to workqueue.c

2007-02-21 Thread Gautham R Shenoy
On Wed, Feb 21, 2007 at 05:30:10PM +0300, Oleg Nesterov wrote: > On 02/21, Srivatsa Vaddagiri wrote: > > > > On Tue, Feb 20, 2007 at 11:09:36PM +0300, Oleg Nesterov wrote: > > > > Which caller are you referring to here? Maybe we can decide on the > > > > option after we see the users of

Re: [RFC PATCH(Experimental) 2/4] Revert changes to workqueue.c

2007-02-21 Thread Oleg Nesterov
On 02/21, Srivatsa Vaddagiri wrote: > > On Tue, Feb 20, 2007 at 11:09:36PM +0300, Oleg Nesterov wrote: > > > Which caller are you referring to here? Maybe we can decide on the > > > option after we see the users of flush_workqueue() in DOWN_PREPARE. > > > > mm/slab.c:cpuup_callback() > > The

Re: [RFC PATCH(Experimental) 2/4] Revert changes to workqueue.c

2007-02-21 Thread Oleg Nesterov
On 02/21, Srivatsa Vaddagiri wrote: On Tue, Feb 20, 2007 at 11:09:36PM +0300, Oleg Nesterov wrote: Which caller are you referring to here? Maybe we can decide on the option after we see the users of flush_workqueue() in DOWN_PREPARE. mm/slab.c:cpuup_callback() The

Re: [RFC PATCH(Experimental) 2/4] Revert changes to workqueue.c

2007-02-21 Thread Gautham R Shenoy
On Wed, Feb 21, 2007 at 05:30:10PM +0300, Oleg Nesterov wrote: On 02/21, Srivatsa Vaddagiri wrote: On Tue, Feb 20, 2007 at 11:09:36PM +0300, Oleg Nesterov wrote: Which caller are you referring to here? Maybe we can decide on the option after we see the users of flush_workqueue() in

Re: [RFC PATCH(Experimental) 2/4] Revert changes to workqueue.c

2007-02-21 Thread Srivatsa Vaddagiri
On Wed, Feb 21, 2007 at 05:30:10PM +0300, Oleg Nesterov wrote: Agreed. Note that we don't need the new del_work. It is always safe to use cancel_work_sync() if we know that the workqueue is frozen, it won't block. We can also do if (!cancel_delayed_work())

Re: [RFC PATCH(Experimental) 2/4] Revert changes to workqueue.c

2007-02-20 Thread Srivatsa Vaddagiri
On Tue, Feb 20, 2007 at 11:09:36PM +0300, Oleg Nesterov wrote: > > Which caller are you referring to here? Maybe we can decide on the > > option after we see the users of flush_workqueue() in DOWN_PREPARE. > > mm/slab.c:cpuup_callback() The cancel_rearming_delayed_work, if used as it is in

Re: [RFC PATCH(Experimental) 2/4] Revert changes to workqueue.c

2007-02-20 Thread Oleg Nesterov
On 02/20, Srivatsa Vaddagiri wrote: > > On Sun, Feb 18, 2007 at 12:59:28AM +0300, Oleg Nesterov wrote: > > Before you begin. You are doing CPU_DOWN_PREPARE after freeze_processes(). > > Not good. This makes impossible to do flush_workueue() at CPU_DOWN_PREPARE > > stage, we have callers. > > We

Re: [RFC PATCH(Experimental) 2/4] Revert changes to workqueue.c

2007-02-20 Thread Srivatsa Vaddagiri
On Sun, Feb 18, 2007 at 12:59:28AM +0300, Oleg Nesterov wrote: > Before you begin. You are doing CPU_DOWN_PREPARE after freeze_processes(). > Not good. This makes impossible to do flush_workueue() at CPU_DOWN_PREPARE > stage, we have callers. We have few solutions to deal with this: a. Mark such

Re: [RFC PATCH(Experimental) 2/4] Revert changes to workqueue.c

2007-02-20 Thread Srivatsa Vaddagiri
On Sun, Feb 18, 2007 at 12:59:28AM +0300, Oleg Nesterov wrote: Before you begin. You are doing CPU_DOWN_PREPARE after freeze_processes(). Not good. This makes impossible to do flush_workueue() at CPU_DOWN_PREPARE stage, we have callers. We have few solutions to deal with this: a. Mark such

Re: [RFC PATCH(Experimental) 2/4] Revert changes to workqueue.c

2007-02-20 Thread Oleg Nesterov
On 02/20, Srivatsa Vaddagiri wrote: On Sun, Feb 18, 2007 at 12:59:28AM +0300, Oleg Nesterov wrote: Before you begin. You are doing CPU_DOWN_PREPARE after freeze_processes(). Not good. This makes impossible to do flush_workueue() at CPU_DOWN_PREPARE stage, we have callers. We have few

Re: [RFC PATCH(Experimental) 2/4] Revert changes to workqueue.c

2007-02-20 Thread Srivatsa Vaddagiri
On Tue, Feb 20, 2007 at 11:09:36PM +0300, Oleg Nesterov wrote: Which caller are you referring to here? Maybe we can decide on the option after we see the users of flush_workqueue() in DOWN_PREPARE. mm/slab.c:cpuup_callback() The cancel_rearming_delayed_work, if used as it is in

Re: [RFC PATCH(Experimental) 2/4] Revert changes to workqueue.c

2007-02-17 Thread Oleg Nesterov
On 02/17, Srivatsa Vaddagiri wrote: > > Yeah, thats what I thought. We will try to split it to the extent > possible in the next iteration. Before you begin. You are doing CPU_DOWN_PREPARE after freeze_processes(). Not good. This makes impossible to do flush_workueue() at CPU_DOWN_PREPARE stage,

Re: [RFC PATCH(Experimental) 2/4] Revert changes to workqueue.c

2007-02-17 Thread Oleg Nesterov
On 02/17, Srivatsa Vaddagiri wrote: Yeah, thats what I thought. We will try to split it to the extent possible in the next iteration. Before you begin. You are doing CPU_DOWN_PREPARE after freeze_processes(). Not good. This makes impossible to do flush_workueue() at CPU_DOWN_PREPARE stage, we

Re: [RFC PATCH(Experimental) 2/4] Revert changes to workqueue.c

2007-02-16 Thread Srivatsa Vaddagiri
On Sat, Feb 17, 2007 at 02:59:39AM +0300, Oleg Nesterov wrote: > In that case (all processes are frozen when workqueue_cpu_callback() > calls cleanup_workqueue_thread()) I agree, it is better to just use > kthread_stop/kthread_should_stop. Great, thanks! > This also means that probably it won't

Re: [RFC PATCH(Experimental) 2/4] Revert changes to workqueue.c

2007-02-16 Thread Oleg Nesterov
On 02/16, Srivatsa Vaddagiri wrote: > > Note with the change proposed in refrigerator, we can avoid > CPU_DEAD_KILL_THREADS and do all cleanup in CPU_DEAD itself. In that case (all processes are frozen when workqueue_cpu_callback() calls cleanup_workqueue_thread()) I agree, it is better to just

Re: [RFC PATCH(Experimental) 2/4] Revert changes to workqueue.c

2007-02-16 Thread Oleg Nesterov
On 02/16, Srivatsa Vaddagiri wrote: > > 2.6.20-mm1 (cwq->should_stop) > = > > static void cleanup_workqueue_thread(struct cpu_workqueue_struct *cwq, int > cpu) > { > struct wq_barrier barr; > int alive = 0; > > spin_lock_irq(>lock); >

Re: [RFC PATCH(Experimental) 2/4] Revert changes to workqueue.c

2007-02-16 Thread Srivatsa Vaddagiri
On Fri, Feb 16, 2007 at 06:33:21PM +0300, Oleg Nesterov wrote: > I take my words back. It is not "ugly" any longer because with this change > we don't do kthread_stop()->wakeup_process() while cwq->thread may sleep in > work->func(). Still I don't see (ok, I am biased and probably wrong, please >

Re: [RFC PATCH(Experimental) 2/4] Revert changes to workqueue.c

2007-02-16 Thread Oleg Nesterov
On 02/16, Srivatsa Vaddagiri wrote: > > On Wed, Feb 14, 2007 at 11:09:04PM +0300, Oleg Nesterov wrote: > > What else you don't like? Why do you want to remove cwq_should_stop() and > > restore an ugly (ugly for workqueue.c) kthread_stop/kthread_should_stop() ? > > What is ugly abt kthread_stop in

Re: [RFC PATCH(Experimental) 2/4] Revert changes to workqueue.c

2007-02-16 Thread Oleg Nesterov
On 02/16, Srivatsa Vaddagiri wrote: On Wed, Feb 14, 2007 at 11:09:04PM +0300, Oleg Nesterov wrote: What else you don't like? Why do you want to remove cwq_should_stop() and restore an ugly (ugly for workqueue.c) kthread_stop/kthread_should_stop() ? What is ugly abt kthread_stop in

Re: [RFC PATCH(Experimental) 2/4] Revert changes to workqueue.c

2007-02-16 Thread Srivatsa Vaddagiri
On Fri, Feb 16, 2007 at 06:33:21PM +0300, Oleg Nesterov wrote: I take my words back. It is not ugly any longer because with this change we don't do kthread_stop()-wakeup_process() while cwq-thread may sleep in work-func(). Still I don't see (ok, I am biased and probably wrong, please correct

Re: [RFC PATCH(Experimental) 2/4] Revert changes to workqueue.c

2007-02-16 Thread Oleg Nesterov
On 02/16, Srivatsa Vaddagiri wrote: 2.6.20-mm1 (cwq-should_stop) = static void cleanup_workqueue_thread(struct cpu_workqueue_struct *cwq, int cpu) { struct wq_barrier barr; int alive = 0; spin_lock_irq(cwq-lock); if

Re: [RFC PATCH(Experimental) 2/4] Revert changes to workqueue.c

2007-02-16 Thread Oleg Nesterov
On 02/16, Srivatsa Vaddagiri wrote: Note with the change proposed in refrigerator, we can avoid CPU_DEAD_KILL_THREADS and do all cleanup in CPU_DEAD itself. In that case (all processes are frozen when workqueue_cpu_callback() calls cleanup_workqueue_thread()) I agree, it is better to just use

Re: [RFC PATCH(Experimental) 2/4] Revert changes to workqueue.c

2007-02-16 Thread Srivatsa Vaddagiri
On Sat, Feb 17, 2007 at 02:59:39AM +0300, Oleg Nesterov wrote: In that case (all processes are frozen when workqueue_cpu_callback() calls cleanup_workqueue_thread()) I agree, it is better to just use kthread_stop/kthread_should_stop. Great, thanks! This also means that probably it won't be

Re: [RFC PATCH(Experimental) 2/4] Revert changes to workqueue.c

2007-02-15 Thread Srivatsa Vaddagiri
On Wed, Feb 14, 2007 at 11:09:04PM +0300, Oleg Nesterov wrote: > What else you don't like? Why do you want to remove cwq_should_stop() and > restore an ugly (ugly for workqueue.c) kthread_stop/kthread_should_stop() ? What is ugly abt kthread_stop in workqueue.c? I feel it is nice if the cleanup

Re: [RFC PATCH(Experimental) 2/4] Revert changes to workqueue.c

2007-02-15 Thread Srivatsa Vaddagiri
On Wed, Feb 14, 2007 at 11:09:04PM +0300, Oleg Nesterov wrote: What else you don't like? Why do you want to remove cwq_should_stop() and restore an ugly (ugly for workqueue.c) kthread_stop/kthread_should_stop() ? What is ugly abt kthread_stop in workqueue.c? I feel it is nice if the cleanup is

Re: [RFC PATCH(Experimental) 2/4] Revert changes to workqueue.c

2007-02-14 Thread Oleg Nesterov
On 02/14, Srivatsa Vaddagiri wrote: > > On Wed, Feb 14, 2007 at 08:13:05PM +0530, Gautham R Shenoy wrote: > > + switch (action) { > > + case CPU_UP_PREPARE: > > + /* Create a new workqueue thread for it. */ > > + list_for_each_entry(wq, , list) { > > Its probably safe to

Re: [RFC PATCH(Experimental) 2/4] Revert changes to workqueue.c

2007-02-14 Thread Oleg Nesterov
On 02/14, Gautham R Shenoy wrote: > > This patch reverts all the recent workqueue hacks added to make it > hotplug safe. In my opinion these hacks are cleanups :) Ok. If we use freezer then yes, we can remove cpu_populated_map and just use for_each_online_cpu(). This is easy and good. What

Re: [RFC PATCH(Experimental) 2/4] Revert changes to workqueue.c

2007-02-14 Thread Srivatsa Vaddagiri
On Wed, Feb 14, 2007 at 08:13:05PM +0530, Gautham R Shenoy wrote: > + switch (action) { > + case CPU_UP_PREPARE: > + /* Create a new workqueue thread for it. */ > + list_for_each_entry(wq, , list) { Its probably safe to take the workqueue (spin) lock here (and

Re: [RFC PATCH(Experimental) 2/4] Revert changes to workqueue.c

2007-02-14 Thread Srivatsa Vaddagiri
On Wed, Feb 14, 2007 at 08:13:05PM +0530, Gautham R Shenoy wrote: > This patch reverts all the recent workqueue hacks added to make it > hotplug safe. Oleg, This patch probably needs review for any races we may have missed to account for. Also we have considered only workqueue.c

[RFC PATCH(Experimental) 2/4] Revert changes to workqueue.c

2007-02-14 Thread Gautham R Shenoy
This patch reverts all the recent workqueue hacks added to make it hotplug safe. Signed-off-by : Srivatsa Vaddagiri <[EMAIL PROTECTED]> Signed-off-by : Gautham R Shenoy <[EMAIL PROTECTED]> kernel/workqueue.c | 225 +++-- 1 files changed, 98

[RFC PATCH(Experimental) 2/4] Revert changes to workqueue.c

2007-02-14 Thread Gautham R Shenoy
This patch reverts all the recent workqueue hacks added to make it hotplug safe. Signed-off-by : Srivatsa Vaddagiri [EMAIL PROTECTED] Signed-off-by : Gautham R Shenoy [EMAIL PROTECTED] kernel/workqueue.c | 225 +++-- 1 files changed, 98

Re: [RFC PATCH(Experimental) 2/4] Revert changes to workqueue.c

2007-02-14 Thread Srivatsa Vaddagiri
On Wed, Feb 14, 2007 at 08:13:05PM +0530, Gautham R Shenoy wrote: This patch reverts all the recent workqueue hacks added to make it hotplug safe. Oleg, This patch probably needs review for any races we may have missed to account for. Also we have considered only workqueue.c present

Re: [RFC PATCH(Experimental) 2/4] Revert changes to workqueue.c

2007-02-14 Thread Srivatsa Vaddagiri
On Wed, Feb 14, 2007 at 08:13:05PM +0530, Gautham R Shenoy wrote: + switch (action) { + case CPU_UP_PREPARE: + /* Create a new workqueue thread for it. */ + list_for_each_entry(wq, workqueues, list) { Its probably safe to take the workqueue (spin) lock here

Re: [RFC PATCH(Experimental) 2/4] Revert changes to workqueue.c

2007-02-14 Thread Oleg Nesterov
On 02/14, Gautham R Shenoy wrote: This patch reverts all the recent workqueue hacks added to make it hotplug safe. In my opinion these hacks are cleanups :) Ok. If we use freezer then yes, we can remove cpu_populated_map and just use for_each_online_cpu(). This is easy and good. What else

Re: [RFC PATCH(Experimental) 2/4] Revert changes to workqueue.c

2007-02-14 Thread Oleg Nesterov
On 02/14, Srivatsa Vaddagiri wrote: On Wed, Feb 14, 2007 at 08:13:05PM +0530, Gautham R Shenoy wrote: + switch (action) { + case CPU_UP_PREPARE: + /* Create a new workqueue thread for it. */ + list_for_each_entry(wq, workqueues, list) { Its probably safe to