Re: [RFC PATCH(Experimental) 1/4] freezer-cpu-hotplug core

2007-02-17 Thread Gautham R Shenoy
On Sat, Feb 17, 2007 at 11:02:33AM +0530, Gautham R Shenoy wrote:

> This looks ok, but probably we could do it in a better way.
> How about an api to thaw only a specific task something like
> thaw_process(struct task_struct p). 

I see that thaw_process already exists in freezer.h! Awesome!!
So lets make use of it, instead of adding the kthread_should_stop 
change to the refrigerator :)

thanks and regards
gautham.

-- 
Gautham R Shenoy
Linux Technology Center
IBM India.
"Freedom comes with a price tag of responsibility, which is still a bargain,
because Freedom is priceless!"
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [RFC PATCH(Experimental) 1/4] freezer-cpu-hotplug core

2007-02-17 Thread Gautham R Shenoy
On Sat, Feb 17, 2007 at 11:02:33AM +0530, Gautham R Shenoy wrote:

 This looks ok, but probably we could do it in a better way.
 How about an api to thaw only a specific task something like
 thaw_process(struct task_struct p). 

I see that thaw_process already exists in freezer.h! Awesome!!
So lets make use of it, instead of adding the kthread_should_stop 
change to the refrigerator :)

thanks and regards
gautham.

-- 
Gautham R Shenoy
Linux Technology Center
IBM India.
Freedom comes with a price tag of responsibility, which is still a bargain,
because Freedom is priceless!
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [RFC PATCH(Experimental) 1/4] freezer-cpu-hotplug core

2007-02-16 Thread Gautham R Shenoy
On Fri, Feb 16, 2007 at 01:42:09PM +0530, Srivatsa Vaddagiri wrote:
> On Fri, Feb 16, 2007 at 12:46:17PM +0530, Srivatsa Vaddagiri wrote:
> > frozen. The only exception is cleaning up of per-cpu threads (which is
> > not possible with processes frozen - if we can find a way to make that
> > possible, then everything can be done in CPU_DEAD).
> 
> How abt a patch like below?
> 
> 
> --- process.c.org 2007-02-16 13:38:39.0 +0530
> +++ process.c 2007-02-16 13:38:59.0 +0530
> @@ -47,7 +47,7 @@ void refrigerator(void)
>   recalc_sigpending(); /* We sent fake signal, clean it up */
>   spin_unlock_irq(>sighand->siglock);
> 
> - while (frozen(current)) {
> + while (frozen(current) && !kthread_should_stop()) {
>   current->state = TASK_UNINTERRUPTIBLE;
>   schedule();
>   }

This looks ok, but probably we could do it in a better way.
How about an api to thaw only a specific task something like
thaw_process(struct task_struct p). 

That way, the CPU_DEAD handler which wants to kthread_stop a thread
can selectively thaw the thread before it does kthread_stop.

Rafael, does this have any negative impact on the freezer design?

> This should let us do kthread_stop() in CPU_DEAD itself (while processes
> are frozen)? That would allow us to do everything from CPU_DEAD itself
> (and not have CPU_DEAD_KILL_THREADS).
> 
> 
> -- 
> Regards,
> vatsa

thanks
gautham.
-- 
Gautham R Shenoy
Linux Technology Center
IBM India.
"Freedom comes with a price tag of responsibility, which is still a bargain,
because Freedom is priceless!"
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [RFC PATCH(Experimental) 1/4] freezer-cpu-hotplug core

2007-02-16 Thread Srivatsa Vaddagiri
On Fri, Feb 16, 2007 at 10:46:05PM +0300, Oleg Nesterov wrote:
> Instead, we can just clear PF_FROZEN before kthread_should_stop().

That should work too. Thanks!

> I don't claim this is better, but this way we don't need to add a
> subtle change to process.c.

-- 
Regards,
vatsa
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [RFC PATCH(Experimental) 1/4] freezer-cpu-hotplug core

2007-02-16 Thread Oleg Nesterov
On 02/16, Srivatsa Vaddagiri wrote:
>
> On Fri, Feb 16, 2007 at 12:46:17PM +0530, Srivatsa Vaddagiri wrote:
> > frozen. The only exception is cleaning up of per-cpu threads (which is
> > not possible with processes frozen - if we can find a way to make that
> > possible, then everything can be done in CPU_DEAD).
> 
> How abt a patch like below?
>
> --- process.c.org 2007-02-16 13:38:39.0 +0530
> +++ process.c 2007-02-16 13:38:59.0 +0530
> @@ -47,7 +47,7 @@ void refrigerator(void)
>   recalc_sigpending(); /* We sent fake signal, clean it up */
>   spin_unlock_irq(>sighand->siglock);
>  
> - while (frozen(current)) {
> + while (frozen(current) && !kthread_should_stop()) {
>   current->state = TASK_UNINTERRUPTIBLE;
>   schedule();
>   }

Instead, we can just clear PF_FROZEN before kthread_should_stop().
I don't claim this is better, but this way we don't need to add a
subtle change to process.c.

> This should let us do kthread_stop() in CPU_DEAD itself (while processes
> are frozen)? That would allow us to do everything from CPU_DEAD itself
> (and not have CPU_DEAD_KILL_THREADS).

... and probably avoid many races, good.

Oleg.

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [RFC PATCH(Experimental) 1/4] freezer-cpu-hotplug core

2007-02-16 Thread Oleg Nesterov
On 02/16, Srivatsa Vaddagiri wrote:
>
> On Wed, Feb 14, 2007 at 11:22:09PM +0300, Oleg Nesterov wrote:
> > > o Splits CPU_DEAD into two events namely
> > >   - CPU_DEAD: which will be handled while the processes are still
> > >   frozen.
> > > 
> > >   - CPU_DEAD_KILL_THREADS: To be handled after we thaw_processes.
> > 
> > 
> > Imho, this is not right. This change the meaning of CPU_DEAD, and so
> > we should fix all users of CPU_DEAD as well.
> 
> Why should we fix all users? Only users who were doing a kthread_stop()
> in CPU_DEAD need to be fixed. From my count, only 5 users (out of a
> total of 35) need to be fixed to not do kthread_stop in CPU_DEAD.

But still we need to fix or at least check them,

> > How about
> > 
> > CPU_DEAD_WHATEVER
> > the processes are still frozen
> > 
> > CPU_DEAD
> > after we thaw_processes
> > 
> > This way we can add processing of the new CPU_DEAD_WHATEVER event where
> > it may help. 
> 
> Well, -most- of the work needs to be done in a state when processes are
> frozen. The only exception is cleaning up of per-cpu threads (which is
> not possible with processes frozen - if we can find a way to make that
> possible, then everything can be done in CPU_DEAD).
> 
> If we go by the change suggested above, then we need to fix all users of
> CPU_DEAD

Sorry, I can't understand you.

This patch adds the new state, why should we fix all users of CPU_DEAD
if they were correct? CPU_DEAD retains its old meaning, all users should
work as before?

>   to do what they are doing in CPU_DEAD_WHATEVER (when processes
> are frozen).

We don't have such users! because we don't have CPU_DEAD_WHATEVER yet.

IOW: I think this new state should have a new name, CPU_DEAD should continue
to be called as a last step. Then we can teach cpu callback's to to take an
advantage of CPU_DEAD_WHATEVER, and we can do this in a separate patches.

No?

> > CPU_UP_PREPARE is called after freeze_processes()... Probably this works,
> > but imho this is no good. Suppose for a moment that khelper will be frozen
> > (yes, yes it can't be), then we can't do kthread_create().
> 
> Yes, I am worried about doing so many things with processes frozen.
> Maybe time (and more testing) will tell us if this is a bad thing or
> not. The only dependency I have found so far is that kthread workqueue needs 
> to
> be up (and hence its worker thread needs to be exempted from hotplug
> freeze). We should mark kthread workqueue accordingly as not freezable
> for hotplug.

Yes, this is what I was talking about.

Oleg.

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [RFC PATCH(Experimental) 1/4] freezer-cpu-hotplug core

2007-02-16 Thread Oleg Nesterov
On 02/16, Srivatsa Vaddagiri wrote:
>
> On Wed, Feb 14, 2007 at 10:47:42PM +0300, Oleg Nesterov wrote:
> > >   for (;;) {
> > > - if (cwq->wq->freezeable)
> > > + if (cwq->wq->freezeable) {
> > 
> > Else? This is wrong. The change like this should start from making all
> > cwq->threads freezeable, otherwise it just doesn't work.
> 
> I agree we need to have all threads frozen for hotplug.

Well, only multithreaded, strictly speaking.

>  Only exception I
> have found is kthread workqueue, which needs to be active after
> freeze_processes(). stop_machine and CPU_UP_PREPARE/kthread_create()
> depend on it to work.

Yes. That is why I worried about freeze_processes() before CPU_UP_PREPARE.

> A worker thread (like kthread workqueue), which has exempted itself from 
> hotplug-freeze, should essentially be prepared to get preempted any time and 
> made to run on any cpu. If that is the case, do you see any problems in 
> having 
> the if () statement above?

helper_wq ("kthread") is singlethread (see above), but this is not nice to
rely on that. (I am not sure I undestand you though).

> > > +wait_to_die:
> > > + /* Wait for kthread_stop */
> > > + set_current_state(TASK_INTERRUPTIBLE);
> > > + while (!kthread_should_stop()) {
> > > + schedule();
> > > + set_current_state(TASK_INTERRUPTIBLE);
> > > + }
> > > + __set_current_state(TASK_RUNNING);
> > > + return 0;
> > >  }
> > 
> > I believe this is not needed, see the comments for the next patch.
> 
> Without this, thread cleanup (cwq->should_stop)/create(CPU_UP_PREPARE) 
> becomes 
> racy

Could you explain? (Again, perhaps you are talking about the old code).

Oleg.

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [RFC PATCH(Experimental) 1/4] freezer-cpu-hotplug core

2007-02-16 Thread Rafael J. Wysocki
On Friday, 16 February 2007 10:59, Srivatsa Vaddagiri wrote:
> On Fri, Feb 16, 2007 at 10:29:20AM +0100, Rafael J. Wysocki wrote:
> > Well, the suspend code has been developed with the assumption that frozen
> > threads stay frozen until _we_ let them thaw by calling thaw_processes().  
> > I'm
> > a bit afraid of this change.
> 
> Note that only kernel threads created thr' kthread_create are allowed
> to exit like this from the refrigerator, that too only when
> (kthread_stop_info.k == current). So all other threads should be unaffected
> because of this change.

Yes, that's why I said "a bit". ;-)
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [RFC PATCH(Experimental) 1/4] freezer-cpu-hotplug core

2007-02-16 Thread Srivatsa Vaddagiri
On Fri, Feb 16, 2007 at 10:29:20AM +0100, Rafael J. Wysocki wrote:
> Well, the suspend code has been developed with the assumption that frozen
> threads stay frozen until _we_ let them thaw by calling thaw_processes().  I'm
> a bit afraid of this change.

Note that only kernel threads created thr' kthread_create are allowed
to exit like this from the refrigerator, that too only when
(kthread_stop_info.k == current). So all other threads should be unaffected
because of this change.

-- 
Regards,
vatsa
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [RFC PATCH(Experimental) 1/4] freezer-cpu-hotplug core

2007-02-16 Thread Rafael J. Wysocki
On Friday, 16 February 2007 09:12, Srivatsa Vaddagiri wrote:
> On Fri, Feb 16, 2007 at 12:46:17PM +0530, Srivatsa Vaddagiri wrote:
> > frozen. The only exception is cleaning up of per-cpu threads (which is
> > not possible with processes frozen - if we can find a way to make that
> > possible, then everything can be done in CPU_DEAD).
> 
> How abt a patch like below?
> 
> 
> --- process.c.org 2007-02-16 13:38:39.0 +0530
> +++ process.c 2007-02-16 13:38:59.0 +0530
> @@ -47,7 +47,7 @@ void refrigerator(void)
>   recalc_sigpending(); /* We sent fake signal, clean it up */
>   spin_unlock_irq(>sighand->siglock);
>  
> - while (frozen(current)) {
> + while (frozen(current) && !kthread_should_stop()) {
>   current->state = TASK_UNINTERRUPTIBLE;
>   schedule();
>   }
> 
> This should let us do kthread_stop() in CPU_DEAD itself (while processes
> are frozen)? That would allow us to do everything from CPU_DEAD itself
> (and not have CPU_DEAD_KILL_THREADS).

Well, the suspend code has been developed with the assumption that frozen
threads stay frozen until _we_ let them thaw by calling thaw_processes().  I'm
a bit afraid of this change.

Greetings,
Rafael
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [RFC PATCH(Experimental) 1/4] freezer-cpu-hotplug core

2007-02-16 Thread Srivatsa Vaddagiri
On Fri, Feb 16, 2007 at 12:46:17PM +0530, Srivatsa Vaddagiri wrote:
> frozen. The only exception is cleaning up of per-cpu threads (which is
> not possible with processes frozen - if we can find a way to make that
> possible, then everything can be done in CPU_DEAD).

How abt a patch like below?


--- process.c.org   2007-02-16 13:38:39.0 +0530
+++ process.c   2007-02-16 13:38:59.0 +0530
@@ -47,7 +47,7 @@ void refrigerator(void)
recalc_sigpending(); /* We sent fake signal, clean it up */
spin_unlock_irq(>sighand->siglock);
 
-   while (frozen(current)) {
+   while (frozen(current) && !kthread_should_stop()) {
current->state = TASK_UNINTERRUPTIBLE;
schedule();
}

This should let us do kthread_stop() in CPU_DEAD itself (while processes
are frozen)? That would allow us to do everything from CPU_DEAD itself
(and not have CPU_DEAD_KILL_THREADS).


-- 
Regards,
vatsa
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [RFC PATCH(Experimental) 1/4] freezer-cpu-hotplug core

2007-02-16 Thread Srivatsa Vaddagiri
On Fri, Feb 16, 2007 at 12:46:17PM +0530, Srivatsa Vaddagiri wrote:
 frozen. The only exception is cleaning up of per-cpu threads (which is
 not possible with processes frozen - if we can find a way to make that
 possible, then everything can be done in CPU_DEAD).

How abt a patch like below?


--- process.c.org   2007-02-16 13:38:39.0 +0530
+++ process.c   2007-02-16 13:38:59.0 +0530
@@ -47,7 +47,7 @@ void refrigerator(void)
recalc_sigpending(); /* We sent fake signal, clean it up */
spin_unlock_irq(current-sighand-siglock);
 
-   while (frozen(current)) {
+   while (frozen(current)  !kthread_should_stop()) {
current-state = TASK_UNINTERRUPTIBLE;
schedule();
}

This should let us do kthread_stop() in CPU_DEAD itself (while processes
are frozen)? That would allow us to do everything from CPU_DEAD itself
(and not have CPU_DEAD_KILL_THREADS).


-- 
Regards,
vatsa
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [RFC PATCH(Experimental) 1/4] freezer-cpu-hotplug core

2007-02-16 Thread Rafael J. Wysocki
On Friday, 16 February 2007 09:12, Srivatsa Vaddagiri wrote:
 On Fri, Feb 16, 2007 at 12:46:17PM +0530, Srivatsa Vaddagiri wrote:
  frozen. The only exception is cleaning up of per-cpu threads (which is
  not possible with processes frozen - if we can find a way to make that
  possible, then everything can be done in CPU_DEAD).
 
 How abt a patch like below?
 
 
 --- process.c.org 2007-02-16 13:38:39.0 +0530
 +++ process.c 2007-02-16 13:38:59.0 +0530
 @@ -47,7 +47,7 @@ void refrigerator(void)
   recalc_sigpending(); /* We sent fake signal, clean it up */
   spin_unlock_irq(current-sighand-siglock);
  
 - while (frozen(current)) {
 + while (frozen(current)  !kthread_should_stop()) {
   current-state = TASK_UNINTERRUPTIBLE;
   schedule();
   }
 
 This should let us do kthread_stop() in CPU_DEAD itself (while processes
 are frozen)? That would allow us to do everything from CPU_DEAD itself
 (and not have CPU_DEAD_KILL_THREADS).

Well, the suspend code has been developed with the assumption that frozen
threads stay frozen until _we_ let them thaw by calling thaw_processes().  I'm
a bit afraid of this change.

Greetings,
Rafael
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [RFC PATCH(Experimental) 1/4] freezer-cpu-hotplug core

2007-02-16 Thread Srivatsa Vaddagiri
On Fri, Feb 16, 2007 at 10:29:20AM +0100, Rafael J. Wysocki wrote:
 Well, the suspend code has been developed with the assumption that frozen
 threads stay frozen until _we_ let them thaw by calling thaw_processes().  I'm
 a bit afraid of this change.

Note that only kernel threads created thr' kthread_create are allowed
to exit like this from the refrigerator, that too only when
(kthread_stop_info.k == current). So all other threads should be unaffected
because of this change.

-- 
Regards,
vatsa
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [RFC PATCH(Experimental) 1/4] freezer-cpu-hotplug core

2007-02-16 Thread Rafael J. Wysocki
On Friday, 16 February 2007 10:59, Srivatsa Vaddagiri wrote:
 On Fri, Feb 16, 2007 at 10:29:20AM +0100, Rafael J. Wysocki wrote:
  Well, the suspend code has been developed with the assumption that frozen
  threads stay frozen until _we_ let them thaw by calling thaw_processes().  
  I'm
  a bit afraid of this change.
 
 Note that only kernel threads created thr' kthread_create are allowed
 to exit like this from the refrigerator, that too only when
 (kthread_stop_info.k == current). So all other threads should be unaffected
 because of this change.

Yes, that's why I said a bit. ;-)
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [RFC PATCH(Experimental) 1/4] freezer-cpu-hotplug core

2007-02-16 Thread Oleg Nesterov
On 02/16, Srivatsa Vaddagiri wrote:

 On Wed, Feb 14, 2007 at 10:47:42PM +0300, Oleg Nesterov wrote:
 for (;;) {
   - if (cwq-wq-freezeable)
   + if (cwq-wq-freezeable) {
  
  Else? This is wrong. The change like this should start from making all
  cwq-threads freezeable, otherwise it just doesn't work.
 
 I agree we need to have all threads frozen for hotplug.

Well, only multithreaded, strictly speaking.

  Only exception I
 have found is kthread workqueue, which needs to be active after
 freeze_processes(). stop_machine and CPU_UP_PREPARE/kthread_create()
 depend on it to work.

Yes. That is why I worried about freeze_processes() before CPU_UP_PREPARE.

 A worker thread (like kthread workqueue), which has exempted itself from 
 hotplug-freeze, should essentially be prepared to get preempted any time and 
 made to run on any cpu. If that is the case, do you see any problems in 
 having 
 the if () statement above?

helper_wq (kthread) is singlethread (see above), but this is not nice to
rely on that. (I am not sure I undestand you though).

   +wait_to_die:
   + /* Wait for kthread_stop */
   + set_current_state(TASK_INTERRUPTIBLE);
   + while (!kthread_should_stop()) {
   + schedule();
   + set_current_state(TASK_INTERRUPTIBLE);
   + }
   + __set_current_state(TASK_RUNNING);
   + return 0;
}
  
  I believe this is not needed, see the comments for the next patch.
 
 Without this, thread cleanup (cwq-should_stop)/create(CPU_UP_PREPARE) 
 becomes 
 racy

Could you explain? (Again, perhaps you are talking about the old code).

Oleg.

-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [RFC PATCH(Experimental) 1/4] freezer-cpu-hotplug core

2007-02-16 Thread Oleg Nesterov
On 02/16, Srivatsa Vaddagiri wrote:

 On Wed, Feb 14, 2007 at 11:22:09PM +0300, Oleg Nesterov wrote:
   o Splits CPU_DEAD into two events namely
 - CPU_DEAD: which will be handled while the processes are still
 frozen.
   
 - CPU_DEAD_KILL_THREADS: To be handled after we thaw_processes.
  
  
  Imho, this is not right. This change the meaning of CPU_DEAD, and so
  we should fix all users of CPU_DEAD as well.
 
 Why should we fix all users? Only users who were doing a kthread_stop()
 in CPU_DEAD need to be fixed. From my count, only 5 users (out of a
 total of 35) need to be fixed to not do kthread_stop in CPU_DEAD.

But still we need to fix or at least check them,

  How about
  
  CPU_DEAD_WHATEVER
  the processes are still frozen
  
  CPU_DEAD
  after we thaw_processes
  
  This way we can add processing of the new CPU_DEAD_WHATEVER event where
  it may help. 
 
 Well, -most- of the work needs to be done in a state when processes are
 frozen. The only exception is cleaning up of per-cpu threads (which is
 not possible with processes frozen - if we can find a way to make that
 possible, then everything can be done in CPU_DEAD).
 
 If we go by the change suggested above, then we need to fix all users of
 CPU_DEAD

Sorry, I can't understand you.

This patch adds the new state, why should we fix all users of CPU_DEAD
if they were correct? CPU_DEAD retains its old meaning, all users should
work as before?

   to do what they are doing in CPU_DEAD_WHATEVER (when processes
 are frozen).

We don't have such users! because we don't have CPU_DEAD_WHATEVER yet.

IOW: I think this new state should have a new name, CPU_DEAD should continue
to be called as a last step. Then we can teach cpu callback's to to take an
advantage of CPU_DEAD_WHATEVER, and we can do this in a separate patches.

No?

  CPU_UP_PREPARE is called after freeze_processes()... Probably this works,
  but imho this is no good. Suppose for a moment that khelper will be frozen
  (yes, yes it can't be), then we can't do kthread_create().
 
 Yes, I am worried about doing so many things with processes frozen.
 Maybe time (and more testing) will tell us if this is a bad thing or
 not. The only dependency I have found so far is that kthread workqueue needs 
 to
 be up (and hence its worker thread needs to be exempted from hotplug
 freeze). We should mark kthread workqueue accordingly as not freezable
 for hotplug.

Yes, this is what I was talking about.

Oleg.

-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [RFC PATCH(Experimental) 1/4] freezer-cpu-hotplug core

2007-02-16 Thread Oleg Nesterov
On 02/16, Srivatsa Vaddagiri wrote:

 On Fri, Feb 16, 2007 at 12:46:17PM +0530, Srivatsa Vaddagiri wrote:
  frozen. The only exception is cleaning up of per-cpu threads (which is
  not possible with processes frozen - if we can find a way to make that
  possible, then everything can be done in CPU_DEAD).
 
 How abt a patch like below?

 --- process.c.org 2007-02-16 13:38:39.0 +0530
 +++ process.c 2007-02-16 13:38:59.0 +0530
 @@ -47,7 +47,7 @@ void refrigerator(void)
   recalc_sigpending(); /* We sent fake signal, clean it up */
   spin_unlock_irq(current-sighand-siglock);
  
 - while (frozen(current)) {
 + while (frozen(current)  !kthread_should_stop()) {
   current-state = TASK_UNINTERRUPTIBLE;
   schedule();
   }

Instead, we can just clear PF_FROZEN before kthread_should_stop().
I don't claim this is better, but this way we don't need to add a
subtle change to process.c.

 This should let us do kthread_stop() in CPU_DEAD itself (while processes
 are frozen)? That would allow us to do everything from CPU_DEAD itself
 (and not have CPU_DEAD_KILL_THREADS).

... and probably avoid many races, good.

Oleg.

-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [RFC PATCH(Experimental) 1/4] freezer-cpu-hotplug core

2007-02-16 Thread Srivatsa Vaddagiri
On Fri, Feb 16, 2007 at 10:46:05PM +0300, Oleg Nesterov wrote:
 Instead, we can just clear PF_FROZEN before kthread_should_stop().

That should work too. Thanks!

 I don't claim this is better, but this way we don't need to add a
 subtle change to process.c.

-- 
Regards,
vatsa
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [RFC PATCH(Experimental) 1/4] freezer-cpu-hotplug core

2007-02-16 Thread Gautham R Shenoy
On Fri, Feb 16, 2007 at 01:42:09PM +0530, Srivatsa Vaddagiri wrote:
 On Fri, Feb 16, 2007 at 12:46:17PM +0530, Srivatsa Vaddagiri wrote:
  frozen. The only exception is cleaning up of per-cpu threads (which is
  not possible with processes frozen - if we can find a way to make that
  possible, then everything can be done in CPU_DEAD).
 
 How abt a patch like below?
 
 
 --- process.c.org 2007-02-16 13:38:39.0 +0530
 +++ process.c 2007-02-16 13:38:59.0 +0530
 @@ -47,7 +47,7 @@ void refrigerator(void)
   recalc_sigpending(); /* We sent fake signal, clean it up */
   spin_unlock_irq(current-sighand-siglock);
 
 - while (frozen(current)) {
 + while (frozen(current)  !kthread_should_stop()) {
   current-state = TASK_UNINTERRUPTIBLE;
   schedule();
   }

This looks ok, but probably we could do it in a better way.
How about an api to thaw only a specific task something like
thaw_process(struct task_struct p). 

That way, the CPU_DEAD handler which wants to kthread_stop a thread
can selectively thaw the thread before it does kthread_stop.

Rafael, does this have any negative impact on the freezer design?

 This should let us do kthread_stop() in CPU_DEAD itself (while processes
 are frozen)? That would allow us to do everything from CPU_DEAD itself
 (and not have CPU_DEAD_KILL_THREADS).
 
 
 -- 
 Regards,
 vatsa

thanks
gautham.
-- 
Gautham R Shenoy
Linux Technology Center
IBM India.
Freedom comes with a price tag of responsibility, which is still a bargain,
because Freedom is priceless!
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [RFC PATCH(Experimental) 1/4] freezer-cpu-hotplug core

2007-02-15 Thread Srivatsa Vaddagiri
On Wed, Feb 14, 2007 at 11:22:09PM +0300, Oleg Nesterov wrote:
> > o Splits CPU_DEAD into two events namely
> >   - CPU_DEAD: which will be handled while the processes are still
> >   frozen.
> > 
> >   - CPU_DEAD_KILL_THREADS: To be handled after we thaw_processes.
> 
> 
> Imho, this is not right. This change the meaning of CPU_DEAD, and so
> we should fix all users of CPU_DEAD as well.

Why should we fix all users? Only users who were doing a kthread_stop()
in CPU_DEAD need to be fixed. From my count, only 5 users (out of a
total of 35) need to be fixed to not do kthread_stop in CPU_DEAD.

> 
> How about
> 
>   CPU_DEAD_WHATEVER
>   the processes are still frozen
> 
>   CPU_DEAD
>   after we thaw_processes
> 
> This way we can add processing of the new CPU_DEAD_WHATEVER event where
> it may help. 

Well, -most- of the work needs to be done in a state when processes are
frozen. The only exception is cleaning up of per-cpu threads (which is
not possible with processes frozen - if we can find a way to make that
possible, then everything can be done in CPU_DEAD).

If we go by the change suggested above, then we need to fix all users of
CPU_DEAD to do what they are doing in CPU_DEAD_WHATEVER (when processes
are frozen). I would rather avoid this invasive change and let
CPU_DEAD be sent with processes frozen still.

> CPU_UP_PREPARE is called after freeze_processes()... Probably this works,
> but imho this is no good. Suppose for a moment that khelper will be frozen
> (yes, yes it can't be), then we can't do kthread_create().

Yes, I am worried about doing so many things with processes frozen.
Maybe time (and more testing) will tell us if this is a bad thing or
not. The only dependency I have found so far is that kthread workqueue needs to
be up (and hence its worker thread needs to be exempted from hotplug
freeze). We should mark kthread workqueue accordingly as not freezable
for hotplug.


-- 
Regards,
vatsa
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [RFC PATCH(Experimental) 1/4] freezer-cpu-hotplug core

2007-02-15 Thread Srivatsa Vaddagiri
On Wed, Feb 14, 2007 at 10:47:42PM +0300, Oleg Nesterov wrote:
> > for (;;) {
> > -   if (cwq->wq->freezeable)
> > +   if (cwq->wq->freezeable) {
> 
> Else? This is wrong. The change like this should start from making all
> cwq->threads freezeable, otherwise it just doesn't work.

I agree we need to have all threads frozen for hotplug. Only exception I
have found is kthread workqueue, which needs to be active after
freeze_processes(). stop_machine and CPU_UP_PREPARE/kthread_create()
depend on it to work.

A worker thread (like kthread workqueue), which has exempted itself from 
hotplug-freeze, should essentially be prepared to get preempted any time and 
made to run on any cpu. If that is the case, do you see any problems in having 
the if () statement above?

> > +wait_to_die:
> > +   /* Wait for kthread_stop */
> > +   set_current_state(TASK_INTERRUPTIBLE);
> > +   while (!kthread_should_stop()) {
> > +   schedule();
> > +   set_current_state(TASK_INTERRUPTIBLE);
> > +   }
> > +   __set_current_state(TASK_RUNNING);
> > +   return 0;
> >  }
> 
> I believe this is not needed, see the comments for the next patch.

Without this, thread cleanup (cwq->should_stop)/create(CPU_UP_PREPARE) becomes 
racy 

-- 
Regards,
vatsa
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [RFC PATCH(Experimental) 1/4] freezer-cpu-hotplug core

2007-02-15 Thread Srivatsa Vaddagiri
On Wed, Feb 14, 2007 at 10:47:42PM +0300, Oleg Nesterov wrote:
  for (;;) {
  -   if (cwq-wq-freezeable)
  +   if (cwq-wq-freezeable) {
 
 Else? This is wrong. The change like this should start from making all
 cwq-threads freezeable, otherwise it just doesn't work.

I agree we need to have all threads frozen for hotplug. Only exception I
have found is kthread workqueue, which needs to be active after
freeze_processes(). stop_machine and CPU_UP_PREPARE/kthread_create()
depend on it to work.

A worker thread (like kthread workqueue), which has exempted itself from 
hotplug-freeze, should essentially be prepared to get preempted any time and 
made to run on any cpu. If that is the case, do you see any problems in having 
the if () statement above?

  +wait_to_die:
  +   /* Wait for kthread_stop */
  +   set_current_state(TASK_INTERRUPTIBLE);
  +   while (!kthread_should_stop()) {
  +   schedule();
  +   set_current_state(TASK_INTERRUPTIBLE);
  +   }
  +   __set_current_state(TASK_RUNNING);
  +   return 0;
   }
 
 I believe this is not needed, see the comments for the next patch.

Without this, thread cleanup (cwq-should_stop)/create(CPU_UP_PREPARE) becomes 
racy 

-- 
Regards,
vatsa
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [RFC PATCH(Experimental) 1/4] freezer-cpu-hotplug core

2007-02-15 Thread Srivatsa Vaddagiri
On Wed, Feb 14, 2007 at 11:22:09PM +0300, Oleg Nesterov wrote:
  o Splits CPU_DEAD into two events namely
- CPU_DEAD: which will be handled while the processes are still
frozen.
  
- CPU_DEAD_KILL_THREADS: To be handled after we thaw_processes.
 
 
 Imho, this is not right. This change the meaning of CPU_DEAD, and so
 we should fix all users of CPU_DEAD as well.

Why should we fix all users? Only users who were doing a kthread_stop()
in CPU_DEAD need to be fixed. From my count, only 5 users (out of a
total of 35) need to be fixed to not do kthread_stop in CPU_DEAD.

 
 How about
 
   CPU_DEAD_WHATEVER
   the processes are still frozen
 
   CPU_DEAD
   after we thaw_processes
 
 This way we can add processing of the new CPU_DEAD_WHATEVER event where
 it may help. 

Well, -most- of the work needs to be done in a state when processes are
frozen. The only exception is cleaning up of per-cpu threads (which is
not possible with processes frozen - if we can find a way to make that
possible, then everything can be done in CPU_DEAD).

If we go by the change suggested above, then we need to fix all users of
CPU_DEAD to do what they are doing in CPU_DEAD_WHATEVER (when processes
are frozen). I would rather avoid this invasive change and let
CPU_DEAD be sent with processes frozen still.

 CPU_UP_PREPARE is called after freeze_processes()... Probably this works,
 but imho this is no good. Suppose for a moment that khelper will be frozen
 (yes, yes it can't be), then we can't do kthread_create().

Yes, I am worried about doing so many things with processes frozen.
Maybe time (and more testing) will tell us if this is a bad thing or
not. The only dependency I have found so far is that kthread workqueue needs to
be up (and hence its worker thread needs to be exempted from hotplug
freeze). We should mark kthread workqueue accordingly as not freezable
for hotplug.


-- 
Regards,
vatsa
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [RFC PATCH(Experimental) 1/4] freezer-cpu-hotplug core

2007-02-14 Thread Oleg Nesterov
On 02/14, Gautham R Shenoy wrote:
>
> o Splits CPU_DEAD into two events namely
>   - CPU_DEAD: which will be handled while the processes are still
>   frozen.
> 
>   - CPU_DEAD_KILL_THREADS: To be handled after we thaw_processes.


Imho, this is not right. This change the meaning of CPU_DEAD, and so
we should fix all users of CPU_DEAD as well.

How about

CPU_DEAD_WHATEVER
the processes are still frozen

CPU_DEAD
after we thaw_processes

This way we can add processing of the new CPU_DEAD_WHATEVER event where
it may help. We don't need to change (for example) workqueue.c with this
patch, we can do it in a separate patch.


CPU_UP_PREPARE is called after freeze_processes()... Probably this works,
but imho this is no good. Suppose for a moment that khelper will be frozen
(yes, yes it can't be), then we can't do kthread_create().

Oleg.

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [RFC PATCH(Experimental) 1/4] freezer-cpu-hotplug core

2007-02-14 Thread Oleg Nesterov
Gautham, I'll try to apply this patch and read the code on Sunday, right
now a couple of comments about workqueue.c changes.

On 02/14, Gautham R Shenoy wrote:
>
> --- hotplug.orig/kernel/workqueue.c
> +++ hotplug/kernel/workqueue.c
> @@ -368,6 +368,7 @@ static int worker_thread(void *__cwq)
>   DEFINE_WAIT(wait);
>   struct k_sigaction sa;
>   sigset_t blocked;
> + int bind_cpu = smp_processor_id();
>  
>   if (!cwq->wq->freezeable)
>   current->flags |= PF_NOFREEZE;
> @@ -392,8 +393,11 @@ static int worker_thread(void *__cwq)
>   do_sigaction(SIGCHLD, , (struct k_sigaction *)0);
>  
>   for (;;) {
> - if (cwq->wq->freezeable)
> + if (cwq->wq->freezeable) {

Else? This is wrong. The change like this should start from making all
cwq->threads freezeable, otherwise it just doesn't work.

>   try_to_freeze();
> + if (cpu_is_offline(bind_cpu))
> + goto wait_to_die;
> + }
>
> ...
>
> +
> +wait_to_die:
> + /* Wait for kthread_stop */
> + set_current_state(TASK_INTERRUPTIBLE);
> + while (!kthread_should_stop()) {
> + schedule();
> + set_current_state(TASK_INTERRUPTIBLE);
> + }
> + __set_current_state(TASK_RUNNING);
> + return 0;
>  }

I believe this is not needed, see the comments for the next patch.

Oleg.

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [RFC PATCH(Experimental) 1/4] freezer-cpu-hotplug core

2007-02-14 Thread Srivatsa Vaddagiri
On Wed, Feb 14, 2007 at 08:12:29PM +0530, Gautham R Shenoy wrote:
> o Can the SYSTEM_RUNNING hack in _cpu_up be avoided by some cleaner means.

Basically freeze_processes doesnt seem to work at the early stages of
bootup (during smp_init) and hence the hack.

One option is to investigate why it didnt work and possibly make it work
at that early stage as well ..


-- 
Regards,
vatsa
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[RFC PATCH(Experimental) 1/4] freezer-cpu-hotplug core

2007-02-14 Thread Gautham R Shenoy
This patch implements process_freezer based cpu-hotplug
core. 
The sailent features are:
o No more (un)lock_cpu_hotplug.
o No more CPU_LOCK_ACQUIRE and CPU_LOCK_RELEASE. Hence no per-subsystem
  hotcpu mutexes.
o Calls freeze_process/thaw_processes at the beginning/end of
  the hotplug operation.
o Splits CPU_DEAD into two events namely
  - CPU_DEAD: which will be handled while the processes are still
  frozen.

  - CPU_DEAD_KILL_THREADS: To be handled after we thaw_processes.

 This split is required because stopping of the per-cpu threads
 using kthread_stop cannot be done while the system is in the frozen
 state. Hence we need subsystems which have created per-cpu threads
 have to stop them once thaw_processes have been called.

o Handles CPU_DEAD and CPU_DEAD_KILL_THREADS for subsystems which
  create per-cpu threads.

Points to ponder: 
o Can calling CPU_DOWN_PREPARE/CPU_UP_PREPARE in the 
frozen context create any dirty dependencies in the future?

o Can the SYSTEM_RUNNING hack in _cpu_up be avoided by some cleaner means.

Signed-off-by : Srivatsa Vaddagiri <[EMAIL PROTECTED]>
Signed-off-by : Gautham R Shenoy   <[EMAIL PROTECTED]>
--
 include/linux/notifier.h |3 --
 kernel/cpu.c |   68 +++
 kernel/sched.c   |7 +++-
 kernel/softirq.c |   10 --
 kernel/softlockup.c  |3 +-
 kernel/workqueue.c   |   16 ++-
 6 files changed, 58 insertions(+), 49 deletions(-)

Index: hotplug/kernel/cpu.c
===
--- hotplug.orig/kernel/cpu.c
+++ hotplug/kernel/cpu.c
@@ -14,6 +14,7 @@
 #include 
 #include 
 #include 
+#include 
 
 /* This protects CPUs going up and down... */
 static DEFINE_MUTEX(cpu_add_remove_lock);
@@ -28,38 +29,15 @@ static int cpu_hotplug_disabled;
 
 #ifdef CONFIG_HOTPLUG_CPU
 
-/* Crappy recursive lock-takers in cpufreq! Complain loudly about idiots */
-static struct task_struct *recursive;
-static int recursive_depth;
-
 void lock_cpu_hotplug(void)
 {
-   struct task_struct *tsk = current;
-
-   if (tsk == recursive) {
-   static int warnings = 10;
-   if (warnings) {
-   printk(KERN_ERR "Lukewarm IQ detected in hotplug 
locking\n");
-   WARN_ON(1);
-   warnings--;
-   }
-   recursive_depth++;
-   return;
-   }
-   mutex_lock(_bitmask_lock);
-   recursive = tsk;
+   return;
 }
 EXPORT_SYMBOL_GPL(lock_cpu_hotplug);
 
 void unlock_cpu_hotplug(void)
 {
-   WARN_ON(recursive != current);
-   if (recursive_depth) {
-   recursive_depth--;
-   return;
-   }
-   recursive = NULL;
-   mutex_unlock(_bitmask_lock);
+   return;
 }
 EXPORT_SYMBOL_GPL(unlock_cpu_hotplug);
 
@@ -133,7 +111,11 @@ static int _cpu_down(unsigned int cpu)
if (!cpu_online(cpu))
return -EINVAL;
 
-   raw_notifier_call_chain(_chain, CPU_LOCK_ACQUIRE, hcpu);
+   if (freeze_processes()) {
+   thaw_processes();
+   return -EBUSY;
+   }
+
err = __raw_notifier_call_chain(_chain, CPU_DOWN_PREPARE,
hcpu, -1, _calls);
if (err == NOTIFY_BAD) {
@@ -141,8 +123,8 @@ static int _cpu_down(unsigned int cpu)
  nr_calls, NULL);
printk("%s: attempt to take down CPU %u failed\n",
__FUNCTION__, cpu);
-   err = -EINVAL;
-   goto out_release;
+   thaw_processes();
+   return -EINVAL;
}
 
/* Ensure that we are not runnable on dying cpu */
@@ -151,9 +133,7 @@ static int _cpu_down(unsigned int cpu)
cpu_clear(cpu, tmp);
set_cpus_allowed(current, tmp);
 
-   mutex_lock(_bitmask_lock);
p = __stop_machine_run(take_cpu_down, NULL, cpu);
-   mutex_unlock(_bitmask_lock);
 
if (IS_ERR(p) || cpu_online(cpu)) {
/* CPU didn't die: tell everyone.  Can't complain. */
@@ -161,9 +141,12 @@ static int _cpu_down(unsigned int cpu)
hcpu) == NOTIFY_BAD)
BUG();
 
+   set_cpus_allowed(current, old_allowed);
+   thaw_processes();
+
if (IS_ERR(p)) {
err = PTR_ERR(p);
-   goto out_allowed;
+   return err;
}
goto out_thread;
}
@@ -185,13 +168,12 @@ static int _cpu_down(unsigned int cpu)
 
check_for_tasks(cpu);
 
+   thaw_processes();
+
+   if (raw_notifier_call_chain(_chain, CPU_DEAD_KILL_THREADS, hcpu) == 
NOTIFY_BAD)
+   BUG();
 out_thread:
err = kthread_stop(p);
-out_allowed:
-   set_cpus_allowed(current, old_allowed);
-out_release:
-   

[RFC PATCH(Experimental) 1/4] freezer-cpu-hotplug core

2007-02-14 Thread Gautham R Shenoy
This patch implements process_freezer based cpu-hotplug
core. 
The sailent features are:
o No more (un)lock_cpu_hotplug.
o No more CPU_LOCK_ACQUIRE and CPU_LOCK_RELEASE. Hence no per-subsystem
  hotcpu mutexes.
o Calls freeze_process/thaw_processes at the beginning/end of
  the hotplug operation.
o Splits CPU_DEAD into two events namely
  - CPU_DEAD: which will be handled while the processes are still
  frozen.

  - CPU_DEAD_KILL_THREADS: To be handled after we thaw_processes.

 This split is required because stopping of the per-cpu threads
 using kthread_stop cannot be done while the system is in the frozen
 state. Hence we need subsystems which have created per-cpu threads
 have to stop them once thaw_processes have been called.

o Handles CPU_DEAD and CPU_DEAD_KILL_THREADS for subsystems which
  create per-cpu threads.

Points to ponder: 
o Can calling CPU_DOWN_PREPARE/CPU_UP_PREPARE in the 
frozen context create any dirty dependencies in the future?

o Can the SYSTEM_RUNNING hack in _cpu_up be avoided by some cleaner means.

Signed-off-by : Srivatsa Vaddagiri [EMAIL PROTECTED]
Signed-off-by : Gautham R Shenoy   [EMAIL PROTECTED]
--
 include/linux/notifier.h |3 --
 kernel/cpu.c |   68 +++
 kernel/sched.c   |7 +++-
 kernel/softirq.c |   10 --
 kernel/softlockup.c  |3 +-
 kernel/workqueue.c   |   16 ++-
 6 files changed, 58 insertions(+), 49 deletions(-)

Index: hotplug/kernel/cpu.c
===
--- hotplug.orig/kernel/cpu.c
+++ hotplug/kernel/cpu.c
@@ -14,6 +14,7 @@
 #include linux/kthread.h
 #include linux/stop_machine.h
 #include linux/mutex.h
+#include linux/freezer.h
 
 /* This protects CPUs going up and down... */
 static DEFINE_MUTEX(cpu_add_remove_lock);
@@ -28,38 +29,15 @@ static int cpu_hotplug_disabled;
 
 #ifdef CONFIG_HOTPLUG_CPU
 
-/* Crappy recursive lock-takers in cpufreq! Complain loudly about idiots */
-static struct task_struct *recursive;
-static int recursive_depth;
-
 void lock_cpu_hotplug(void)
 {
-   struct task_struct *tsk = current;
-
-   if (tsk == recursive) {
-   static int warnings = 10;
-   if (warnings) {
-   printk(KERN_ERR Lukewarm IQ detected in hotplug 
locking\n);
-   WARN_ON(1);
-   warnings--;
-   }
-   recursive_depth++;
-   return;
-   }
-   mutex_lock(cpu_bitmask_lock);
-   recursive = tsk;
+   return;
 }
 EXPORT_SYMBOL_GPL(lock_cpu_hotplug);
 
 void unlock_cpu_hotplug(void)
 {
-   WARN_ON(recursive != current);
-   if (recursive_depth) {
-   recursive_depth--;
-   return;
-   }
-   recursive = NULL;
-   mutex_unlock(cpu_bitmask_lock);
+   return;
 }
 EXPORT_SYMBOL_GPL(unlock_cpu_hotplug);
 
@@ -133,7 +111,11 @@ static int _cpu_down(unsigned int cpu)
if (!cpu_online(cpu))
return -EINVAL;
 
-   raw_notifier_call_chain(cpu_chain, CPU_LOCK_ACQUIRE, hcpu);
+   if (freeze_processes()) {
+   thaw_processes();
+   return -EBUSY;
+   }
+
err = __raw_notifier_call_chain(cpu_chain, CPU_DOWN_PREPARE,
hcpu, -1, nr_calls);
if (err == NOTIFY_BAD) {
@@ -141,8 +123,8 @@ static int _cpu_down(unsigned int cpu)
  nr_calls, NULL);
printk(%s: attempt to take down CPU %u failed\n,
__FUNCTION__, cpu);
-   err = -EINVAL;
-   goto out_release;
+   thaw_processes();
+   return -EINVAL;
}
 
/* Ensure that we are not runnable on dying cpu */
@@ -151,9 +133,7 @@ static int _cpu_down(unsigned int cpu)
cpu_clear(cpu, tmp);
set_cpus_allowed(current, tmp);
 
-   mutex_lock(cpu_bitmask_lock);
p = __stop_machine_run(take_cpu_down, NULL, cpu);
-   mutex_unlock(cpu_bitmask_lock);
 
if (IS_ERR(p) || cpu_online(cpu)) {
/* CPU didn't die: tell everyone.  Can't complain. */
@@ -161,9 +141,12 @@ static int _cpu_down(unsigned int cpu)
hcpu) == NOTIFY_BAD)
BUG();
 
+   set_cpus_allowed(current, old_allowed);
+   thaw_processes();
+
if (IS_ERR(p)) {
err = PTR_ERR(p);
-   goto out_allowed;
+   return err;
}
goto out_thread;
}
@@ -185,13 +168,12 @@ static int _cpu_down(unsigned int cpu)
 
check_for_tasks(cpu);
 
+   thaw_processes();
+
+   if (raw_notifier_call_chain(cpu_chain, CPU_DEAD_KILL_THREADS, hcpu) == 
NOTIFY_BAD)
+   BUG();
 out_thread:
err = kthread_stop(p);
-out_allowed:
-   

Re: [RFC PATCH(Experimental) 1/4] freezer-cpu-hotplug core

2007-02-14 Thread Srivatsa Vaddagiri
On Wed, Feb 14, 2007 at 08:12:29PM +0530, Gautham R Shenoy wrote:
 o Can the SYSTEM_RUNNING hack in _cpu_up be avoided by some cleaner means.

Basically freeze_processes doesnt seem to work at the early stages of
bootup (during smp_init) and hence the hack.

One option is to investigate why it didnt work and possibly make it work
at that early stage as well ..


-- 
Regards,
vatsa
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [RFC PATCH(Experimental) 1/4] freezer-cpu-hotplug core

2007-02-14 Thread Oleg Nesterov
Gautham, I'll try to apply this patch and read the code on Sunday, right
now a couple of comments about workqueue.c changes.

On 02/14, Gautham R Shenoy wrote:

 --- hotplug.orig/kernel/workqueue.c
 +++ hotplug/kernel/workqueue.c
 @@ -368,6 +368,7 @@ static int worker_thread(void *__cwq)
   DEFINE_WAIT(wait);
   struct k_sigaction sa;
   sigset_t blocked;
 + int bind_cpu = smp_processor_id();
  
   if (!cwq-wq-freezeable)
   current-flags |= PF_NOFREEZE;
 @@ -392,8 +393,11 @@ static int worker_thread(void *__cwq)
   do_sigaction(SIGCHLD, sa, (struct k_sigaction *)0);
  
   for (;;) {
 - if (cwq-wq-freezeable)
 + if (cwq-wq-freezeable) {

Else? This is wrong. The change like this should start from making all
cwq-threads freezeable, otherwise it just doesn't work.

   try_to_freeze();
 + if (cpu_is_offline(bind_cpu))
 + goto wait_to_die;
 + }

 ...

 +
 +wait_to_die:
 + /* Wait for kthread_stop */
 + set_current_state(TASK_INTERRUPTIBLE);
 + while (!kthread_should_stop()) {
 + schedule();
 + set_current_state(TASK_INTERRUPTIBLE);
 + }
 + __set_current_state(TASK_RUNNING);
 + return 0;
  }

I believe this is not needed, see the comments for the next patch.

Oleg.

-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [RFC PATCH(Experimental) 1/4] freezer-cpu-hotplug core

2007-02-14 Thread Oleg Nesterov
On 02/14, Gautham R Shenoy wrote:

 o Splits CPU_DEAD into two events namely
   - CPU_DEAD: which will be handled while the processes are still
   frozen.
 
   - CPU_DEAD_KILL_THREADS: To be handled after we thaw_processes.


Imho, this is not right. This change the meaning of CPU_DEAD, and so
we should fix all users of CPU_DEAD as well.

How about

CPU_DEAD_WHATEVER
the processes are still frozen

CPU_DEAD
after we thaw_processes

This way we can add processing of the new CPU_DEAD_WHATEVER event where
it may help. We don't need to change (for example) workqueue.c with this
patch, we can do it in a separate patch.


CPU_UP_PREPARE is called after freeze_processes()... Probably this works,
but imho this is no good. Suppose for a moment that khelper will be frozen
(yes, yes it can't be), then we can't do kthread_create().

Oleg.

-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/