Re: [RFC][PATCH][EXPERIMENTAL] CPU hotplug with frozen tasks
On 04/16, Rafael J. Wysocki wrote:
>
> Appended is the updated version of the patch (in addition to the changes
> mentioned above I've eliminated the magic constant 0x0008 from cpu.c by
> changing the new definitions in notifier.h).
Most sub-systems doesn't care about CPU_TASKS_FROZEN bit. Take for example
workqueue.c,
> --- linux-2.6.21-rc7.orig/kernel/workqueue.c 2007-04-16 23:05:17.0
> +0200
> +++ linux-2.6.21-rc7/kernel/workqueue.c 2007-04-16 23:05:45.0
> +0200
> @@ -757,6 +757,7 @@ static int __devinit workqueue_cpu_callb
>
> switch (action) {
> case CPU_UP_PREPARE:
> + case CPU_UP_PREPARE_FROZEN:
> mutex_lock(_mutex);
> /* Create a new workqueue thread for it. */
> list_for_each_entry(wq, , list) {
> @@ -768,6 +769,7 @@ static int __devinit workqueue_cpu_callb
> break;
>
> case CPU_ONLINE:
> + case CPU_ONLINE_FROZEN:
> /* Kick off worker threads. */
> list_for_each_entry(wq, , list) {
> struct cpu_workqueue_struct *cwq;
> @@ -780,6 +782,7 @@ static int __devinit workqueue_cpu_callb
> break;
>
> case CPU_UP_CANCELED:
> + case CPU_UP_CANCELED_FROZEN:
> list_for_each_entry(wq, , list) {
> if (!per_cpu_ptr(wq->cpu_wq, hotcpu)->thread)
> continue;
> @@ -792,14 +795,17 @@ static int __devinit workqueue_cpu_callb
> break;
>
> case CPU_DOWN_PREPARE:
> + case CPU_DOWN_PREPARE_FROZEN:
> mutex_lock(_mutex);
> break;
>
> case CPU_DOWN_FAILED:
> + case CPU_DOWN_FAILED_FROZEN:
> mutex_unlock(_mutex);
> break;
>
> case CPU_DEAD:
> + case CPU_DEAD_FROZEN:
> list_for_each_entry(wq, , list)
> cleanup_workqueue_thread(wq, hotcpu);
> list_for_each_entry(wq, , list)
I think it is better to add
action &= ~CPU_TASKS_FROZEN;
at the head of workqueue_cpu_callback() instead. I think this way
you can make this patch a lot simpler and smaller.
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] CPU hotplug with frozen tasks
On 04/16, Rafael J. Wysocki wrote:
Appended is the updated version of the patch (in addition to the changes
mentioned above I've eliminated the magic constant 0x0008 from cpu.c by
changing the new definitions in notifier.h).
Most sub-systems doesn't care about CPU_TASKS_FROZEN bit. Take for example
workqueue.c,
--- linux-2.6.21-rc7.orig/kernel/workqueue.c 2007-04-16 23:05:17.0
+0200
+++ linux-2.6.21-rc7/kernel/workqueue.c 2007-04-16 23:05:45.0
+0200
@@ -757,6 +757,7 @@ static int __devinit workqueue_cpu_callb
switch (action) {
case CPU_UP_PREPARE:
+ case CPU_UP_PREPARE_FROZEN:
mutex_lock(workqueue_mutex);
/* Create a new workqueue thread for it. */
list_for_each_entry(wq, workqueues, list) {
@@ -768,6 +769,7 @@ static int __devinit workqueue_cpu_callb
break;
case CPU_ONLINE:
+ case CPU_ONLINE_FROZEN:
/* Kick off worker threads. */
list_for_each_entry(wq, workqueues, list) {
struct cpu_workqueue_struct *cwq;
@@ -780,6 +782,7 @@ static int __devinit workqueue_cpu_callb
break;
case CPU_UP_CANCELED:
+ case CPU_UP_CANCELED_FROZEN:
list_for_each_entry(wq, workqueues, list) {
if (!per_cpu_ptr(wq-cpu_wq, hotcpu)-thread)
continue;
@@ -792,14 +795,17 @@ static int __devinit workqueue_cpu_callb
break;
case CPU_DOWN_PREPARE:
+ case CPU_DOWN_PREPARE_FROZEN:
mutex_lock(workqueue_mutex);
break;
case CPU_DOWN_FAILED:
+ case CPU_DOWN_FAILED_FROZEN:
mutex_unlock(workqueue_mutex);
break;
case CPU_DEAD:
+ case CPU_DEAD_FROZEN:
list_for_each_entry(wq, workqueues, list)
cleanup_workqueue_thread(wq, hotcpu);
list_for_each_entry(wq, workqueues, list)
I think it is better to add
action = ~CPU_TASKS_FROZEN;
at the head of workqueue_cpu_callback() instead. I think this way
you can make this patch a lot simpler and smaller.
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] CPU hotplug with frozen tasks
On Wednesday, 18 April 2007 11:42, Gautham R Shenoy wrote: > Hi, > > The patch looks good to me. > > On Mon, Apr 16, 2007 at 11:27:58PM +0200, Rafael J. Wysocki wrote: > > > > --- > > Documentation/cpu-hotplug.txt |9 +++-- > > arch/i386/kernel/cpu/intel_cacheinfo.c|2 ++ > > arch/i386/kernel/cpu/mcheck/therm_throt.c |2 ++ > > arch/i386/kernel/cpuid.c |2 ++ > > [snip] > > Though I am wondering what might be the usecase for microcode! Well, for 2.6.21-rc I had to introduce the variable suspend_cpu_hotplug (in cpu.c) and make the microcode driver use it to distinguish between the 'normal' and suspend-related CPU hotplug. I'd like to get rid of this ugliness ASAP. > Guess we'll see that patch soon :-) Yes, in a couple of days. I have to make both patches apply to -mm first. :-) 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] CPU hotplug with frozen tasks
Hi, The patch looks good to me. On Mon, Apr 16, 2007 at 11:27:58PM +0200, Rafael J. Wysocki wrote: > > --- > Documentation/cpu-hotplug.txt |9 +++-- > arch/i386/kernel/cpu/intel_cacheinfo.c|2 ++ > arch/i386/kernel/cpu/mcheck/therm_throt.c |2 ++ > arch/i386/kernel/cpuid.c |2 ++ [snip] Though I am wondering what might be the usecase for microcode! Guess we'll see that patch soon :-) 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] CPU hotplug with frozen tasks
Hi, The patch looks good to me. On Mon, Apr 16, 2007 at 11:27:58PM +0200, Rafael J. Wysocki wrote: --- Documentation/cpu-hotplug.txt |9 +++-- arch/i386/kernel/cpu/intel_cacheinfo.c|2 ++ arch/i386/kernel/cpu/mcheck/therm_throt.c |2 ++ arch/i386/kernel/cpuid.c |2 ++ [snip] Though I am wondering what might be the usecase for microcode! Guess we'll see that patch soon :-) 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] CPU hotplug with frozen tasks
On Wednesday, 18 April 2007 11:42, Gautham R Shenoy wrote: Hi, The patch looks good to me. On Mon, Apr 16, 2007 at 11:27:58PM +0200, Rafael J. Wysocki wrote: --- Documentation/cpu-hotplug.txt |9 +++-- arch/i386/kernel/cpu/intel_cacheinfo.c|2 ++ arch/i386/kernel/cpu/mcheck/therm_throt.c |2 ++ arch/i386/kernel/cpuid.c |2 ++ [snip] Though I am wondering what might be the usecase for microcode! Well, for 2.6.21-rc I had to introduce the variable suspend_cpu_hotplug (in cpu.c) and make the microcode driver use it to distinguish between the 'normal' and suspend-related CPU hotplug. I'd like to get rid of this ugliness ASAP. Guess we'll see that patch soon :-) Yes, in a couple of days. I have to make both patches apply to -mm first. :-) 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] CPU hotplug with frozen tasks
On Monday, 16 April 2007 11:50, Gautham Shenoy wrote: > Hi Rafael, > > On 4/15/07, Rafael J. Wysocki <[EMAIL PROTECTED]> wrote: > > Hi, > > > > As I said before, we have a problem with using the CPU hotplug for > > suspending > > because of the notifiers that are called from within cpu_up()/cpu_down() and > > (sometimes) assume that the system is fully functional. > > > > Right. In order to use freezer for CPU hotplug, we need to perform > that audit anyway. > > > One obvious solution of this problem would be to make the notifiers behave > > differently if tasks are frozen, but for this purpose we'd need to tell them > > that this is the case. In principle, we could do it in many different ways > > (eg. by using a global variable, with the help of suspend notifiers etc.), > > but > > IMO one of the cleanest methods woud be to use some special values for the > > notifications occuring while tasks are frozen (eg. CPU_DEAD_FROZEN instead > > of > > CPU_DEAD etc.). In that case the notifiers could react in some special ways > > to the "FROZEN" notfifications and that would allow us to simplify some code > > paths (eg. in the microcode driver). > > > > Agreed. > > > The appended patch introduces such "FROZEN" notfifications, modifies the CPU > > hotplug core to use them and updates all of the users of CPU hotplug > > notifiers > > to recognize them. For now, they are treated in the same way as the > > corresponding "normal" notifications, but I'm going to modify the microcode > > driver to really use them and I believe that some other subsystems can > > benefit > > from using them as well. > > > > Ok. A minor doubt. > > When you say FROZEN, do you mean frozen due to suspend ? Yes. I have modified the patch to stress that in the documentation (and in the comment in notifier.h). > If yes, then it makes sense. Otherwise once cpu-hotplug starts using the > freezer > (hopefully it will someday soon :-)) won't this patch become redundant ? Well, I'm not entirely sure. > [Except of course fixing a few glitches > due to the assumption that the system is fully functional, when it's > actually frozen.] > > I am of the opinion that we should have notifications which help the > cpu-hotplug aware > subsystems differentiate between a normal cpu-hotplug and a > cpu-hotplug initiated by > suspend. Thereby they can handle it accordingly and not destroy any > percpu resources > and reuse them instead during resume. [Unless, of course, the CPU in question refuses to go online during the resume.] I agree. Initially I thought the patch might cover some scenarios other than just the suspend, but then I changed my mind. For this reason I'm not sure if the "_FROZEN" parts of the new constants names are exactly right, but I have no better ideas anyway. > Am I missing something? No, I don't think so. :-) Appended is the updated version of the patch (in addition to the changes mentioned above I've eliminated the magic constant 0x0008 from cpu.c by changing the new definitions in notifier.h). Greetings, Rafael --- Documentation/cpu-hotplug.txt |9 +++-- arch/i386/kernel/cpu/intel_cacheinfo.c|2 ++ arch/i386/kernel/cpu/mcheck/therm_throt.c |2 ++ arch/i386/kernel/cpuid.c |2 ++ arch/i386/kernel/microcode.c |3 +++ arch/i386/kernel/msr.c|2 ++ arch/ia64/kernel/palinfo.c|2 ++ arch/ia64/kernel/salinfo.c|2 ++ arch/ia64/kernel/topology.c |2 ++ arch/powerpc/kernel/sysfs.c |2 ++ arch/powerpc/mm/numa.c|3 +++ arch/s390/appldata/appldata_base.c|2 ++ arch/x86_64/kernel/mce.c |2 ++ arch/x86_64/kernel/mce_amd.c |2 ++ arch/x86_64/kernel/vsyscall.c |2 +- block/ll_rw_blk.c |2 +- drivers/base/topology.c |3 +++ drivers/cpufreq/cpufreq.c |3 +++ drivers/cpufreq/cpufreq_stats.c |2 ++ drivers/infiniband/hw/ehca/ehca_irq.c |6 ++ drivers/kvm/kvm_main.c|3 +++ fs/buffer.c |2 +- fs/xfs/xfs_mount.c|3 +++ include/linux/notifier.h | 12 kernel/cpu.c | 26 ++ kernel/hrtimer.c |2 ++ kernel/profile.c |4 kernel/rcupdate.c |2 ++ kernel/relay.c|2 ++ kernel/sched.c| 10 ++ kernel/softirq.c |4 kernel/softlockup.c |4 kernel/timer.c|2 ++ kernel/workqueue.c|6 ++ lib/radix-tree.c |2 +-
Re: [RFC][PATCH][EXPERIMENTAL] CPU hotplug with frozen tasks
On Monday, 16 April 2007 09:05, Pavel Machek wrote:
> Hi!
>
> > As I said before, we have a problem with using the CPU hotplug for
> > suspending
> > because of the notifiers that are called from within cpu_up()/cpu_down() and
> > (sometimes) assume that the system is fully functional.
> >
> > One obvious solution of this problem would be to make the notifiers behave
> > differently if tasks are frozen, but for this purpose we'd need to tell them
> > that this is the case. In principle, we could do it in many different ways
> > (eg. by using a global variable, with the help of suspend notifiers etc.),
> > but
> > IMO one of the cleanest methods woud be to use some special values for the
> > notifications occuring while tasks are frozen (eg. CPU_DEAD_FROZEN instead
> > of
> > CPU_DEAD etc.). In that case the notifiers could react in some special ways
> > to the "FROZEN" notfifications and that would allow us to simplify some code
> > paths (eg. in the microcode driver).
> >
> > The appended patch introduces such "FROZEN" notfifications, modifies the CPU
> > hotplug core to use them and updates all of the users of CPU hotplug
> > notifiers
> > to recognize them. For now, they are treated in the same way as the
> > corresponding "normal" notifications, but I'm going to modify the microcode
> > driver to really use them and I believe that some other subsystems can
> > benefit
> > from using them as well.
> >
> > The patch is totally experimental and untested, although it's been
> > successfully
> > compiled on x86_64 and it's main purpose is to show what exactly I
> > mean. :-)
>
> Looks sane to me.
>
> > Index: linux-2.6.21-rc6/kernel/cpu.c
> > ===
> > --- linux-2.6.21-rc6.orig/kernel/cpu.c 2007-04-16 00:24:56.0
> > +0200
> > +++ linux-2.6.21-rc6/kernel/cpu.c 2007-04-16 00:25:14.0 +0200
> > @@ -120,11 +120,12 @@ static int take_cpu_down(void *unused)
> > }
> >
> > /* Requires cpu_add_remove_lock to be held */
> > -static int _cpu_down(unsigned int cpu)
> > +static int _cpu_down(unsigned int cpu, int tasks_frozen)
> > {
> > int err;
> > struct task_struct *p;
> > cpumask_t old_allowed, tmp;
> > + unsigned long mod = tasks_frozen ? 0x0008 : 0;
> >
>
> Can we get constant instead of 0x0008 here?
Sure. Updated patch is in the reply to Gautham.
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] CPU hotplug with frozen tasks
Hi Rafael, On 4/15/07, Rafael J. Wysocki <[EMAIL PROTECTED]> wrote: Hi, As I said before, we have a problem with using the CPU hotplug for suspending because of the notifiers that are called from within cpu_up()/cpu_down() and (sometimes) assume that the system is fully functional. Right. In order to use freezer for CPU hotplug, we need to perform that audit anyway. One obvious solution of this problem would be to make the notifiers behave differently if tasks are frozen, but for this purpose we'd need to tell them that this is the case. In principle, we could do it in many different ways (eg. by using a global variable, with the help of suspend notifiers etc.), but IMO one of the cleanest methods woud be to use some special values for the notifications occuring while tasks are frozen (eg. CPU_DEAD_FROZEN instead of CPU_DEAD etc.). In that case the notifiers could react in some special ways to the "FROZEN" notfifications and that would allow us to simplify some code paths (eg. in the microcode driver). Agreed. The appended patch introduces such "FROZEN" notfifications, modifies the CPU hotplug core to use them and updates all of the users of CPU hotplug notifiers to recognize them. For now, they are treated in the same way as the corresponding "normal" notifications, but I'm going to modify the microcode driver to really use them and I believe that some other subsystems can benefit from using them as well. Ok. A minor doubt. When you say FROZEN, do you mean frozen due to suspend ? If yes, then it makes sense. Otherwise once cpu-hotplug starts using the freezer (hopefully it will someday soon :-)) won't this patch become redundant ? [Except of course fixing a few glitches due to the assumption that the system is fully functional, when it's actually frozen.] I am of the opinion that we should have notifications which help the cpu-hotplug aware subsystems differentiate between a normal cpu-hotplug and a cpu-hotplug initiated by suspend. Thereby they can handle it accordingly and not destroy any percpu resources and reuse them instead during resume. Am I missing something? The patch is totally experimental and untested, although it's been successfully compiled on x86_64 and it's main purpose is to show what exactly I mean. :-) Comments welcome. Other than that, I am ok with the patch. Greetings, Rafael Thanks and Regards gautham. - 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] CPU hotplug with frozen tasks
Hi!
> As I said before, we have a problem with using the CPU hotplug for suspending
> because of the notifiers that are called from within cpu_up()/cpu_down() and
> (sometimes) assume that the system is fully functional.
>
> One obvious solution of this problem would be to make the notifiers behave
> differently if tasks are frozen, but for this purpose we'd need to tell them
> that this is the case. In principle, we could do it in many different ways
> (eg. by using a global variable, with the help of suspend notifiers etc.), but
> IMO one of the cleanest methods woud be to use some special values for the
> notifications occuring while tasks are frozen (eg. CPU_DEAD_FROZEN instead of
> CPU_DEAD etc.). In that case the notifiers could react in some special ways
> to the "FROZEN" notfifications and that would allow us to simplify some code
> paths (eg. in the microcode driver).
>
> The appended patch introduces such "FROZEN" notfifications, modifies the CPU
> hotplug core to use them and updates all of the users of CPU hotplug notifiers
> to recognize them. For now, they are treated in the same way as the
> corresponding "normal" notifications, but I'm going to modify the microcode
> driver to really use them and I believe that some other subsystems can benefit
> from using them as well.
>
> The patch is totally experimental and untested, although it's been
> successfully
> compiled on x86_64 and it's main purpose is to show what exactly I
> mean. :-)
Looks sane to me.
> Index: linux-2.6.21-rc6/kernel/cpu.c
> ===
> --- linux-2.6.21-rc6.orig/kernel/cpu.c2007-04-16 00:24:56.0
> +0200
> +++ linux-2.6.21-rc6/kernel/cpu.c 2007-04-16 00:25:14.0 +0200
> @@ -120,11 +120,12 @@ static int take_cpu_down(void *unused)
> }
>
> /* Requires cpu_add_remove_lock to be held */
> -static int _cpu_down(unsigned int cpu)
> +static int _cpu_down(unsigned int cpu, int tasks_frozen)
> {
> int err;
> struct task_struct *p;
> cpumask_t old_allowed, tmp;
> + unsigned long mod = tasks_frozen ? 0x0008 : 0;
>
Can we get constant instead of 0x0008 here?
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures)
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
-
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] CPU hotplug with frozen tasks
Hi!
As I said before, we have a problem with using the CPU hotplug for suspending
because of the notifiers that are called from within cpu_up()/cpu_down() and
(sometimes) assume that the system is fully functional.
One obvious solution of this problem would be to make the notifiers behave
differently if tasks are frozen, but for this purpose we'd need to tell them
that this is the case. In principle, we could do it in many different ways
(eg. by using a global variable, with the help of suspend notifiers etc.), but
IMO one of the cleanest methods woud be to use some special values for the
notifications occuring while tasks are frozen (eg. CPU_DEAD_FROZEN instead of
CPU_DEAD etc.). In that case the notifiers could react in some special ways
to the FROZEN notfifications and that would allow us to simplify some code
paths (eg. in the microcode driver).
The appended patch introduces such FROZEN notfifications, modifies the CPU
hotplug core to use them and updates all of the users of CPU hotplug notifiers
to recognize them. For now, they are treated in the same way as the
corresponding normal notifications, but I'm going to modify the microcode
driver to really use them and I believe that some other subsystems can benefit
from using them as well.
The patch is totally experimental and untested, although it's been
successfully
compiled on x86_64 and it's main purpose is to show what exactly I
mean. :-)
Looks sane to me.
Index: linux-2.6.21-rc6/kernel/cpu.c
===
--- linux-2.6.21-rc6.orig/kernel/cpu.c2007-04-16 00:24:56.0
+0200
+++ linux-2.6.21-rc6/kernel/cpu.c 2007-04-16 00:25:14.0 +0200
@@ -120,11 +120,12 @@ static int take_cpu_down(void *unused)
}
/* Requires cpu_add_remove_lock to be held */
-static int _cpu_down(unsigned int cpu)
+static int _cpu_down(unsigned int cpu, int tasks_frozen)
{
int err;
struct task_struct *p;
cpumask_t old_allowed, tmp;
+ unsigned long mod = tasks_frozen ? 0x0008 : 0;
Can we get constant instead of 0x0008 here?
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures)
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
-
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] CPU hotplug with frozen tasks
Hi Rafael, On 4/15/07, Rafael J. Wysocki [EMAIL PROTECTED] wrote: Hi, As I said before, we have a problem with using the CPU hotplug for suspending because of the notifiers that are called from within cpu_up()/cpu_down() and (sometimes) assume that the system is fully functional. Right. In order to use freezer for CPU hotplug, we need to perform that audit anyway. One obvious solution of this problem would be to make the notifiers behave differently if tasks are frozen, but for this purpose we'd need to tell them that this is the case. In principle, we could do it in many different ways (eg. by using a global variable, with the help of suspend notifiers etc.), but IMO one of the cleanest methods woud be to use some special values for the notifications occuring while tasks are frozen (eg. CPU_DEAD_FROZEN instead of CPU_DEAD etc.). In that case the notifiers could react in some special ways to the FROZEN notfifications and that would allow us to simplify some code paths (eg. in the microcode driver). Agreed. The appended patch introduces such FROZEN notfifications, modifies the CPU hotplug core to use them and updates all of the users of CPU hotplug notifiers to recognize them. For now, they are treated in the same way as the corresponding normal notifications, but I'm going to modify the microcode driver to really use them and I believe that some other subsystems can benefit from using them as well. Ok. A minor doubt. When you say FROZEN, do you mean frozen due to suspend ? If yes, then it makes sense. Otherwise once cpu-hotplug starts using the freezer (hopefully it will someday soon :-)) won't this patch become redundant ? [Except of course fixing a few glitches due to the assumption that the system is fully functional, when it's actually frozen.] I am of the opinion that we should have notifications which help the cpu-hotplug aware subsystems differentiate between a normal cpu-hotplug and a cpu-hotplug initiated by suspend. Thereby they can handle it accordingly and not destroy any percpu resources and reuse them instead during resume. Am I missing something? The patch is totally experimental and untested, although it's been successfully compiled on x86_64 and it's main purpose is to show what exactly I mean. :-) Comments welcome. Other than that, I am ok with the patch. Greetings, Rafael Thanks and Regards gautham. - 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] CPU hotplug with frozen tasks
On Monday, 16 April 2007 09:05, Pavel Machek wrote:
Hi!
As I said before, we have a problem with using the CPU hotplug for
suspending
because of the notifiers that are called from within cpu_up()/cpu_down() and
(sometimes) assume that the system is fully functional.
One obvious solution of this problem would be to make the notifiers behave
differently if tasks are frozen, but for this purpose we'd need to tell them
that this is the case. In principle, we could do it in many different ways
(eg. by using a global variable, with the help of suspend notifiers etc.),
but
IMO one of the cleanest methods woud be to use some special values for the
notifications occuring while tasks are frozen (eg. CPU_DEAD_FROZEN instead
of
CPU_DEAD etc.). In that case the notifiers could react in some special ways
to the FROZEN notfifications and that would allow us to simplify some code
paths (eg. in the microcode driver).
The appended patch introduces such FROZEN notfifications, modifies the CPU
hotplug core to use them and updates all of the users of CPU hotplug
notifiers
to recognize them. For now, they are treated in the same way as the
corresponding normal notifications, but I'm going to modify the microcode
driver to really use them and I believe that some other subsystems can
benefit
from using them as well.
The patch is totally experimental and untested, although it's been
successfully
compiled on x86_64 and it's main purpose is to show what exactly I
mean. :-)
Looks sane to me.
Index: linux-2.6.21-rc6/kernel/cpu.c
===
--- linux-2.6.21-rc6.orig/kernel/cpu.c 2007-04-16 00:24:56.0
+0200
+++ linux-2.6.21-rc6/kernel/cpu.c 2007-04-16 00:25:14.0 +0200
@@ -120,11 +120,12 @@ static int take_cpu_down(void *unused)
}
/* Requires cpu_add_remove_lock to be held */
-static int _cpu_down(unsigned int cpu)
+static int _cpu_down(unsigned int cpu, int tasks_frozen)
{
int err;
struct task_struct *p;
cpumask_t old_allowed, tmp;
+ unsigned long mod = tasks_frozen ? 0x0008 : 0;
Can we get constant instead of 0x0008 here?
Sure. Updated patch is in the reply to Gautham.
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] CPU hotplug with frozen tasks
On Monday, 16 April 2007 11:50, Gautham Shenoy wrote: Hi Rafael, On 4/15/07, Rafael J. Wysocki [EMAIL PROTECTED] wrote: Hi, As I said before, we have a problem with using the CPU hotplug for suspending because of the notifiers that are called from within cpu_up()/cpu_down() and (sometimes) assume that the system is fully functional. Right. In order to use freezer for CPU hotplug, we need to perform that audit anyway. One obvious solution of this problem would be to make the notifiers behave differently if tasks are frozen, but for this purpose we'd need to tell them that this is the case. In principle, we could do it in many different ways (eg. by using a global variable, with the help of suspend notifiers etc.), but IMO one of the cleanest methods woud be to use some special values for the notifications occuring while tasks are frozen (eg. CPU_DEAD_FROZEN instead of CPU_DEAD etc.). In that case the notifiers could react in some special ways to the FROZEN notfifications and that would allow us to simplify some code paths (eg. in the microcode driver). Agreed. The appended patch introduces such FROZEN notfifications, modifies the CPU hotplug core to use them and updates all of the users of CPU hotplug notifiers to recognize them. For now, they are treated in the same way as the corresponding normal notifications, but I'm going to modify the microcode driver to really use them and I believe that some other subsystems can benefit from using them as well. Ok. A minor doubt. When you say FROZEN, do you mean frozen due to suspend ? Yes. I have modified the patch to stress that in the documentation (and in the comment in notifier.h). If yes, then it makes sense. Otherwise once cpu-hotplug starts using the freezer (hopefully it will someday soon :-)) won't this patch become redundant ? Well, I'm not entirely sure. [Except of course fixing a few glitches due to the assumption that the system is fully functional, when it's actually frozen.] I am of the opinion that we should have notifications which help the cpu-hotplug aware subsystems differentiate between a normal cpu-hotplug and a cpu-hotplug initiated by suspend. Thereby they can handle it accordingly and not destroy any percpu resources and reuse them instead during resume. [Unless, of course, the CPU in question refuses to go online during the resume.] I agree. Initially I thought the patch might cover some scenarios other than just the suspend, but then I changed my mind. For this reason I'm not sure if the _FROZEN parts of the new constants names are exactly right, but I have no better ideas anyway. Am I missing something? No, I don't think so. :-) Appended is the updated version of the patch (in addition to the changes mentioned above I've eliminated the magic constant 0x0008 from cpu.c by changing the new definitions in notifier.h). Greetings, Rafael --- Documentation/cpu-hotplug.txt |9 +++-- arch/i386/kernel/cpu/intel_cacheinfo.c|2 ++ arch/i386/kernel/cpu/mcheck/therm_throt.c |2 ++ arch/i386/kernel/cpuid.c |2 ++ arch/i386/kernel/microcode.c |3 +++ arch/i386/kernel/msr.c|2 ++ arch/ia64/kernel/palinfo.c|2 ++ arch/ia64/kernel/salinfo.c|2 ++ arch/ia64/kernel/topology.c |2 ++ arch/powerpc/kernel/sysfs.c |2 ++ arch/powerpc/mm/numa.c|3 +++ arch/s390/appldata/appldata_base.c|2 ++ arch/x86_64/kernel/mce.c |2 ++ arch/x86_64/kernel/mce_amd.c |2 ++ arch/x86_64/kernel/vsyscall.c |2 +- block/ll_rw_blk.c |2 +- drivers/base/topology.c |3 +++ drivers/cpufreq/cpufreq.c |3 +++ drivers/cpufreq/cpufreq_stats.c |2 ++ drivers/infiniband/hw/ehca/ehca_irq.c |6 ++ drivers/kvm/kvm_main.c|3 +++ fs/buffer.c |2 +- fs/xfs/xfs_mount.c|3 +++ include/linux/notifier.h | 12 kernel/cpu.c | 26 ++ kernel/hrtimer.c |2 ++ kernel/profile.c |4 kernel/rcupdate.c |2 ++ kernel/relay.c|2 ++ kernel/sched.c| 10 ++ kernel/softirq.c |4 kernel/softlockup.c |4 kernel/timer.c|2 ++ kernel/workqueue.c|6 ++ lib/radix-tree.c |2 +- mm/page_alloc.c |5 - mm/slab.c
[RFC][PATCH][EXPERIMENTAL] CPU hotplug with frozen tasks
Hi, As I said before, we have a problem with using the CPU hotplug for suspending because of the notifiers that are called from within cpu_up()/cpu_down() and (sometimes) assume that the system is fully functional. One obvious solution of this problem would be to make the notifiers behave differently if tasks are frozen, but for this purpose we'd need to tell them that this is the case. In principle, we could do it in many different ways (eg. by using a global variable, with the help of suspend notifiers etc.), but IMO one of the cleanest methods woud be to use some special values for the notifications occuring while tasks are frozen (eg. CPU_DEAD_FROZEN instead of CPU_DEAD etc.). In that case the notifiers could react in some special ways to the "FROZEN" notfifications and that would allow us to simplify some code paths (eg. in the microcode driver). The appended patch introduces such "FROZEN" notfifications, modifies the CPU hotplug core to use them and updates all of the users of CPU hotplug notifiers to recognize them. For now, they are treated in the same way as the corresponding "normal" notifications, but I'm going to modify the microcode driver to really use them and I believe that some other subsystems can benefit from using them as well. The patch is totally experimental and untested, although it's been successfully compiled on x86_64 and it's main purpose is to show what exactly I mean. :-) Comments welcome. Greetings, Rafael --- Documentation/cpu-hotplug.txt |8 ++-- arch/i386/kernel/cpu/intel_cacheinfo.c|2 ++ arch/i386/kernel/cpu/mcheck/therm_throt.c |2 ++ arch/i386/kernel/cpuid.c |2 ++ arch/i386/kernel/microcode.c |3 +++ arch/i386/kernel/msr.c|2 ++ arch/ia64/kernel/palinfo.c|2 ++ arch/ia64/kernel/salinfo.c|2 ++ arch/ia64/kernel/topology.c |2 ++ arch/powerpc/kernel/sysfs.c |2 ++ arch/powerpc/mm/numa.c|3 +++ arch/s390/appldata/appldata_base.c|2 ++ arch/x86_64/kernel/mce.c |2 ++ arch/x86_64/kernel/mce_amd.c |2 ++ arch/x86_64/kernel/vsyscall.c |2 +- block/ll_rw_blk.c |2 +- drivers/base/topology.c |3 +++ drivers/cpufreq/cpufreq.c |3 +++ drivers/cpufreq/cpufreq_stats.c |2 ++ drivers/infiniband/hw/ehca/ehca_irq.c |6 ++ drivers/kvm/kvm_main.c|3 +++ fs/buffer.c |2 +- fs/xfs/xfs_mount.c|3 +++ include/linux/notifier.h |9 + kernel/cpu.c | 26 ++ kernel/hrtimer.c |2 ++ kernel/profile.c |4 kernel/rcupdate.c |2 ++ kernel/relay.c|2 ++ kernel/sched.c| 10 ++ kernel/softirq.c |4 kernel/softlockup.c |4 kernel/timer.c|2 ++ kernel/workqueue.c|6 ++ lib/radix-tree.c |2 +- mm/page_alloc.c |5 - mm/slab.c |6 ++ mm/swap.c |2 +- mm/vmscan.c |2 +- mm/vmstat.c |3 +++ net/core/dev.c|2 +- net/core/flow.c |2 +- net/iucv/iucv.c |6 ++ 43 files changed, 140 insertions(+), 23 deletions(-) Index: linux-2.6.21-rc6/include/linux/notifier.h === --- linux-2.6.21-rc6.orig/include/linux/notifier.h 2007-04-16 00:24:57.0 +0200 +++ linux-2.6.21-rc6/include/linux/notifier.h 2007-04-16 00:25:14.0 +0200 @@ -186,6 +186,15 @@ extern int srcu_notifier_call_chain(stru #define CPU_DOWN_PREPARE 0x0005 /* CPU (unsigned)v going down */ #define CPU_DOWN_FAILED0x0006 /* CPU (unsigned)v NOT going down */ #define CPU_DEAD 0x0007 /* CPU (unsigned)v dead */ +/* The following values are used for CPU hotplug events occuring while tasks are + * frozen (eg. during a suspend) + */ +#define CPU_ONLINE_FROZEN 0x000A /* CPU (unsigned)v is up */ +#define CPU_UP_PREPARE_FROZEN 0x000B /* CPU (unsigned)v coming up */ +#define CPU_UP_CANCELED_FROZEN 0x000C /* CPU (unsigned)v NOT coming up */ +#define CPU_DOWN_PREPARE_FROZEN0x000D /* CPU (unsigned)v going down */ +#define CPU_DOWN_FAILED_FROZEN 0x000E /* CPU (unsigned)v NOT going down */ +#define CPU_DEAD_FROZEN
[RFC][PATCH][EXPERIMENTAL] CPU hotplug with frozen tasks
Hi, As I said before, we have a problem with using the CPU hotplug for suspending because of the notifiers that are called from within cpu_up()/cpu_down() and (sometimes) assume that the system is fully functional. One obvious solution of this problem would be to make the notifiers behave differently if tasks are frozen, but for this purpose we'd need to tell them that this is the case. In principle, we could do it in many different ways (eg. by using a global variable, with the help of suspend notifiers etc.), but IMO one of the cleanest methods woud be to use some special values for the notifications occuring while tasks are frozen (eg. CPU_DEAD_FROZEN instead of CPU_DEAD etc.). In that case the notifiers could react in some special ways to the FROZEN notfifications and that would allow us to simplify some code paths (eg. in the microcode driver). The appended patch introduces such FROZEN notfifications, modifies the CPU hotplug core to use them and updates all of the users of CPU hotplug notifiers to recognize them. For now, they are treated in the same way as the corresponding normal notifications, but I'm going to modify the microcode driver to really use them and I believe that some other subsystems can benefit from using them as well. The patch is totally experimental and untested, although it's been successfully compiled on x86_64 and it's main purpose is to show what exactly I mean. :-) Comments welcome. Greetings, Rafael --- Documentation/cpu-hotplug.txt |8 ++-- arch/i386/kernel/cpu/intel_cacheinfo.c|2 ++ arch/i386/kernel/cpu/mcheck/therm_throt.c |2 ++ arch/i386/kernel/cpuid.c |2 ++ arch/i386/kernel/microcode.c |3 +++ arch/i386/kernel/msr.c|2 ++ arch/ia64/kernel/palinfo.c|2 ++ arch/ia64/kernel/salinfo.c|2 ++ arch/ia64/kernel/topology.c |2 ++ arch/powerpc/kernel/sysfs.c |2 ++ arch/powerpc/mm/numa.c|3 +++ arch/s390/appldata/appldata_base.c|2 ++ arch/x86_64/kernel/mce.c |2 ++ arch/x86_64/kernel/mce_amd.c |2 ++ arch/x86_64/kernel/vsyscall.c |2 +- block/ll_rw_blk.c |2 +- drivers/base/topology.c |3 +++ drivers/cpufreq/cpufreq.c |3 +++ drivers/cpufreq/cpufreq_stats.c |2 ++ drivers/infiniband/hw/ehca/ehca_irq.c |6 ++ drivers/kvm/kvm_main.c|3 +++ fs/buffer.c |2 +- fs/xfs/xfs_mount.c|3 +++ include/linux/notifier.h |9 + kernel/cpu.c | 26 ++ kernel/hrtimer.c |2 ++ kernel/profile.c |4 kernel/rcupdate.c |2 ++ kernel/relay.c|2 ++ kernel/sched.c| 10 ++ kernel/softirq.c |4 kernel/softlockup.c |4 kernel/timer.c|2 ++ kernel/workqueue.c|6 ++ lib/radix-tree.c |2 +- mm/page_alloc.c |5 - mm/slab.c |6 ++ mm/swap.c |2 +- mm/vmscan.c |2 +- mm/vmstat.c |3 +++ net/core/dev.c|2 +- net/core/flow.c |2 +- net/iucv/iucv.c |6 ++ 43 files changed, 140 insertions(+), 23 deletions(-) Index: linux-2.6.21-rc6/include/linux/notifier.h === --- linux-2.6.21-rc6.orig/include/linux/notifier.h 2007-04-16 00:24:57.0 +0200 +++ linux-2.6.21-rc6/include/linux/notifier.h 2007-04-16 00:25:14.0 +0200 @@ -186,6 +186,15 @@ extern int srcu_notifier_call_chain(stru #define CPU_DOWN_PREPARE 0x0005 /* CPU (unsigned)v going down */ #define CPU_DOWN_FAILED0x0006 /* CPU (unsigned)v NOT going down */ #define CPU_DEAD 0x0007 /* CPU (unsigned)v dead */ +/* The following values are used for CPU hotplug events occuring while tasks are + * frozen (eg. during a suspend) + */ +#define CPU_ONLINE_FROZEN 0x000A /* CPU (unsigned)v is up */ +#define CPU_UP_PREPARE_FROZEN 0x000B /* CPU (unsigned)v coming up */ +#define CPU_UP_CANCELED_FROZEN 0x000C /* CPU (unsigned)v NOT coming up */ +#define CPU_DOWN_PREPARE_FROZEN0x000D /* CPU (unsigned)v going down */ +#define CPU_DOWN_FAILED_FROZEN 0x000E /* CPU (unsigned)v NOT going down */ +#define CPU_DEAD_FROZEN0x000F
