[ 163.378265] BUG: unable to handle kernel NULL pointer dereference at 00000002
[ 163.378276] IP: [<c0124933>] sched_power_savings_store+0x13/0x70
[ 163.378288] *pde = 00000000
[ 163.378294] Oops: 0000 [#1] SMP
[ 163.378301] Modules linked in: ipv6 usbhid hid radeon drm rfcomm l2cap bluetooth kvm_intel kvm uinput ppdev autofs4 acpi_cpufreq cpufreq_conservative cpufreq_userspace c pufreq_ondemand cpufreq_stats freq_table cpufreq_powersave sbs sbshc container iptable_filter ip_tables x_tables dm_crypt dm_mod parport_pc lp parport joydev pcmcia iTCO_wd t serio_raw yenta_socket iTCO_vendor_support rsrc_nonstatic arc4 psmouse pcspkr ecb crypto_blkcipher pcmcia_core ac battery iwl3945 rfkill mac80211 cfg80211 snd_hda_intel e 1000e video output snd_pcm snd_timer snd soundcore snd_page_alloc button shpchp pci_hotplug intel_agp agpgart thinkpad_acpi led_class nvram evdev ext3 jbd mbcache sg sr_mod cdrom sd_mod ata_generic pata_acpi ata_piix ahci ehci_hcd uhci_hcd libata scsi_mod usbcore dock thermal processor fan thermal_sys fuse
[ 163.378431]
[ 163.378436] Pid: 6456, comm: sched-powersave Not tainted (2.6.26-00149-gc8988f9-dirty #6)
[ 163.378442] EIP: 0060:[<c0124933>] EFLAGS: 00010282 CPU: 0
[ 163.378448] EIP is at sched_power_savings_store+0x13/0x70
[ 163.378453] EAX: 00000002 EBX: 00000000 ECX: ffffffea EDX: f480a480
[ 163.378458] ESI: f480a480 EDI: f69a1000 EBP: c0430464 ESP: f6b15f40
[ 163.378463] DS: 007b ES: 007b FS: 00d8 GS: 0033 SS: 0068
[ 163.378468] Process sched-powersave (pid: 6456, ti=f6b14000
task=f6ad3720 task.ti=f6b14000)
[ 163.378473] Stack: c01249a0 c0430464 c027e329 f480a480 c0430158
00000002 c01d8ca1 00000002
[ 163.378487] f6ae8000 f69ae314 c0430158 f6ae8000 080fc408
c01d8be0 00000002 c0191c3f
[ 163.378501] f6b15fa0 f6ae8000 f6ae8000 fffffff7 080fc408
f6b14000 c0192241 f6b15fa0
[ 163.378515] Call Trace:
[ 163.378519] [<c01249a0>] sched_mc_power_savings_store+0x0/0x10
[ 163.378527] [<c027e329>] sysdev_class_store+0x29/0x40
[ 163.378537] [<c01d8ca1>] sysfs_write_file+0xc1/0x110
[ 163.378546] [<c01d8be0>] sysfs_write_file+0x0/0x110
[ 163.378553] [<c0191c3f>] vfs_write+0x9f/0x170
[ 163.378561] [<c0192241>] sys_write+0x41/0x70
[ 163.378568] [<c0103d6f>] sysenter_past_esp+0x78/0xb9
[ 163.378575] [<c0320000>] set_cpu_sibling_map+0x1e0/0x2a0
[ 163.378584] =======================
[ 163.378587] Code: ff 89 c3 b8 08 ed 41 c0 e8 5b ef 1f 00 e8 26 66 00
00 89 d8 5b c3 66 90 83 ec 08 89 1c 24 89 cb b9 ea ff ff ff 89 74 24 04
89 d6 <0f> b6 10 8d 42 d0 3c 01 76 13 8b 1c 24 89 c8 8b 74 24 04 83 c4
[ 163.378662] EIP: [<c0124933>] sched_power_savings_store+0x13/0x70
SS:ESP 0068:f6b15f40
[ 163.378673] ---[ end trace 90dd601c24cdd750 ]---
Hello Aneesh,
On Fri, Jul 25, 2008 at 03:23:17PM +0530, Aneesh Kumar K.V wrote:
> [ 163.378265] BUG: unable to handle kernel NULL pointer dereference at 00000002
> [ 163.378276] IP: [<c0124933>] sched_power_savings_store+0x13/0x70
Does the attached patch solve the problem?
Regards,
Frederik
diff --git a/kernel/sched.c b/kernel/sched.c
index 0047bd9..090b397 100644
--- a/kernel/sched.c
+++ b/kernel/sched.c
@@ -7643,34 +7643,30 @@ static ssize_t sched_power_savings_store(const char *buf, size_t count, int smt)
}
#ifdef CONFIG_SCHED_MC
-static ssize_t sched_mc_power_savings_show(struct sys_device *dev,
- struct sysdev_attribute *attr, char *page)
+static ssize_t sched_mc_power_savings_show(struct sysdev_class *cls, char *page)
{
return sprintf(page, "%u\n", sched_mc_power_savings);
}
-static ssize_t sched_mc_power_savings_store(struct sys_device *dev,
- struct sysdev_attribute *attr,
+static ssize_t sched_mc_power_savings_store(struct sysdev_class *cls,
const char *buf, size_t count)
{
return sched_power_savings_store(buf, count, 0);
}
-static SYSDEV_ATTR(sched_mc_power_savings, 0644, sched_mc_power_savings_show,
+static SYSDEV_CLASS_ATTR(sched_mc_power_savings, 0644, sched_mc_power_savings_show,
sched_mc_power_savings_store);
#endif
#ifdef CONFIG_SCHED_SMT
-static ssize_t sched_smt_power_savings_show(struct sys_device *dev,
- struct sysdev_attribute *attr, char *page)
+static ssize_t sched_smt_power_savings_show(struct sysdev_class *cls, char *page)
{
return sprintf(page, "%u\n", sched_smt_power_savings);
}
-static ssize_t sched_smt_power_savings_store(struct sys_device *dev,
- struct sysdev_attribute *attr,
+static ssize_t sched_smt_power_savings_store(struct sysdev_class *cls,
const char *buf, size_t count)
{
return sched_power_savings_store(buf, count, 1);
}
-static SYSDEV_ATTR(sched_smt_power_savings, 0644, sched_smt_power_savings_show,
+static SYSDEV_CLASS_ATTR(sched_smt_power_savings, 0644, sched_smt_power_savings_show,
sched_smt_power_savings_store);
#endif
@@ -7680,13 +7676,13 @@ int sched_create_sysfs_power_savings_entries(struct sysdev_class *cls)
#ifdef CONFIG_SCHED_SMT
if (smt_capable())
- err = sysfs_create_file(&cls->kset.kobj,
- &attr_sched_smt_power_savings.attr);
+ err = sysdev_class_create_file(cls,
+ &attr_sched_smt_power_savings);
#endif
#ifdef CONFIG_SCHED_MC
if (!err && mc_capable())
- err = sysfs_create_file(&cls->kset.kobj,
- &attr_sched_mc_power_savings.attr);
+ err = sysdev_class_create_file(cls,
+ &attr_sched_mc_power_savings);
#endif
return err;
}
On Tue, 2008-07-29 at 00:26 +0200, Frederik Deweerdt wrote:
> Hello Aneesh,
>
> On Fri, Jul 25, 2008 at 03:23:17PM +0530, Aneesh Kumar K.V wrote:
> > [ 163.378265] BUG: unable to handle kernel NULL pointer dereference at 00000002
> > [ 163.378276] IP: [<c0124933>] sched_power_savings_store+0x13/0x70
>
> Does the attached patch solve the problem?
Patch seems to have missed the boat for rc1 too.
>
> Regards,
> Frederik
>
> diff --git a/kernel/sched.c b/kernel/sched.c
> index 0047bd9..090b397 100644
> --- a/kernel/sched.c
> +++ b/kernel/sched.c
> @@ -7643,34 +7643,30 @@ static ssize_t sched_power_savings_store(const char *buf, size_t count, int smt)
> }
>
> #ifdef CONFIG_SCHED_MC
> -static ssize_t sched_mc_power_savings_show(struct sys_device *dev,
> - struct sysdev_attribute *attr, char *page)
> +static ssize_t sched_mc_power_savings_show(struct sysdev_class *cls, char *page)
> {
> return sprintf(page, "%u\n", sched_mc_power_savings);
> }
> -static ssize_t sched_mc_power_savings_store(struct sys_device *dev,
> - struct sysdev_attribute *attr,
> +static ssize_t sched_mc_power_savings_store(struct sysdev_class *cls,
> const char *buf, size_t count)
> {
> return sched_power_savings_store(buf, count, 0);
> }
> -static SYSDEV_ATTR(sched_mc_power_savings, 0644, sched_mc_power_savings_show,
> +static SYSDEV_CLASS_ATTR(sched_mc_power_savings, 0644, sched_mc_power_savings_show,
> sched_mc_power_savings_store);
> #endif
>
> #ifdef CONFIG_SCHED_SMT
> -static ssize_t sched_smt_power_savings_show(struct sys_device *dev,
> - struct sysdev_attribute *attr, char *page)
> +static ssize_t sched_smt_power_savings_show(struct sysdev_class *cls, char *page)
> {
> return sprintf(page, "%u\n", sched_smt_power_savings);
> }
> -static ssize_t sched_smt_power_savings_store(struct sys_device *dev,
> - struct sysdev_attribute *attr,
> +static ssize_t sched_smt_power_savings_store(struct sysdev_class *cls,
> const char *buf, size_t count)
> {
> return sched_power_savings_store(buf, count, 1);
> }
> -static SYSDEV_ATTR(sched_smt_power_savings, 0644, sched_smt_power_savings_show,
> +static SYSDEV_CLASS_ATTR(sched_smt_power_savings, 0644, sched_smt_power_savings_show,
> sched_smt_power_savings_store);
> #endif
>
> @@ -7680,13 +7676,13 @@ int sched_create_sysfs_power_savings_entries(struct sysdev_class *cls)
>
> #ifdef CONFIG_SCHED_SMT
> if (smt_capable())
> - err = sysfs_create_file(&cls->kset.kobj,
> - &attr_sched_smt_power_savings.attr);
> + err = sysdev_class_create_file(cls,
> + &attr_sched_smt_power_savings);
> #endif
> #ifdef CONFIG_SCHED_MC
> if (!err && mc_capable())
> - err = sysfs_create_file(&cls->kset.kobj,
> - &attr_sched_mc_power_savings.attr);
> + err = sysdev_class_create_file(cls,
> + &attr_sched_mc_power_savings);
> #endif
> return err;
> }
On Tue, Jul 29, 2008 at 06:22:15AM +0200, Mike Galbraith wrote:
> On Tue, 2008-07-29 at 00:26 +0200, Frederik Deweerdt wrote:
> > Hello Aneesh,
> >
> > On Fri, Jul 25, 2008 at 03:23:17PM +0530, Aneesh Kumar K.V wrote:
> > > [ 163.378265] BUG: unable to handle kernel NULL pointer dereference at 00000002
> > > [ 163.378276] IP: [<c0124933>] sched_power_savings_store+0x13/0x70
> >
> > Does the attached patch solve the problem?
>
> Patch seems to have missed the boat for rc1 too.
Hmm. I'll resend.
BTW i think it's clearly a bug that distributions are even accessing
that file. It doesn't make any sense in the context they are using
it (like at every boot).
-Andi
On Tue, 29 Jul 2008 14:09:11 +0200
Andi Kleen <[email protected]> wrote:
> On Tue, Jul 29, 2008 at 06:22:15AM +0200, Mike Galbraith wrote:
> > On Tue, 2008-07-29 at 00:26 +0200, Frederik Deweerdt wrote:
> > > Hello Aneesh,
> > >
> > > On Fri, Jul 25, 2008 at 03:23:17PM +0530, Aneesh Kumar K.V wrote:
> > > > [ 163.378265] BUG: unable to handle kernel NULL pointer
> > > > dereference at 00000002 [ 163.378276] IP: [<c0124933>]
> > > > sched_power_savings_store+0x13/0x70
> > >
> > > Does the attached patch solve the problem?
> >
> > Patch seems to have missed the boat for rc1 too.
>
> Hmm. I'll resend.
>
> BTW i think it's clearly a bug that distributions are even accessing
> that file. It doesn't make any sense in the context they are using
> it (like at every boot).
it's a power saving feature that they very likely turn on by default...
--
If you want to reach me at my work email, use [email protected]
For development, discussion and tips for power savings,
visit http://www.lesswatts.org
On Tue, Jul 29, 2008 at 06:56:35AM -0700, Arjan van de Ven wrote:
> On Tue, 29 Jul 2008 14:09:11 +0200
> Andi Kleen <[email protected]> wrote:
>
> > On Tue, Jul 29, 2008 at 06:22:15AM +0200, Mike Galbraith wrote:
> > > On Tue, 2008-07-29 at 00:26 +0200, Frederik Deweerdt wrote:
> > > > Hello Aneesh,
> > > >
> > > > On Fri, Jul 25, 2008 at 03:23:17PM +0530, Aneesh Kumar K.V wrote:
> > > > > [ 163.378265] BUG: unable to handle kernel NULL pointer
> > > > > dereference at 00000002 [ 163.378276] IP: [<c0124933>]
> > > > > sched_power_savings_store+0x13/0x70
> > > >
> > > > Does the attached patch solve the problem?
> > >
> > > Patch seems to have missed the boat for rc1 too.
> >
> > Hmm. I'll resend.
> >
> > BTW i think it's clearly a bug that distributions are even accessing
> > that file. It doesn't make any sense in the context they are using
> > it (like at every boot).
>
> it's a power saving feature that they very likely turn on by default...
A power saving feature that has a significant trade off between power
and performance.
This means performance will go down. Perhaps it would be ok on battery,
but it's a feature that only makes sense on servers which don't have batteries.
So enabling it by default in a standard installation seems quite wrong
to me.
BTW opensuse seems to even set it to 2 which doesn't even exist. This means
there was a BOF at OLS discussing adding more modes, but right now there is
only 0 or 1.
-Andi
On Tue, 29 Jul 2008 16:53:48 +0200
Andi Kleen <[email protected]> wrote:
> On Tue, Jul 29, 2008 at 06:56:35AM -0700, Arjan van de Ven wrote:
> > On Tue, 29 Jul 2008 14:09:11 +0200
> > Andi Kleen <[email protected]> wrote:
> >
> > > On Tue, Jul 29, 2008 at 06:22:15AM +0200, Mike Galbraith wrote:
> > > > On Tue, 2008-07-29 at 00:26 +0200, Frederik Deweerdt wrote:
> > > > > Hello Aneesh,
> > > > >
> > > > > On Fri, Jul 25, 2008 at 03:23:17PM +0530, Aneesh Kumar K.V
> > > > > wrote:
> > > > > > [ 163.378265] BUG: unable to handle kernel NULL pointer
> > > > > > dereference at 00000002 [ 163.378276] IP: [<c0124933>]
> > > > > > sched_power_savings_store+0x13/0x70
> > > > >
> > > > > Does the attached patch solve the problem?
> > > >
> > > > Patch seems to have missed the boat for rc1 too.
> > >
> > > Hmm. I'll resend.
> > >
> > > BTW i think it's clearly a bug that distributions are even
> > > accessing that file. It doesn't make any sense in the context
> > > they are using it (like at every boot).
> >
> > it's a power saving feature that they very likely turn on by
> > default...
>
> A power saving feature that has a significant trade off between power
> and performance.
do you have numbers to explain "significant tradeoff" ?
> This means performance will go down. Perhaps it would be ok on
> battery,
the illusion that power only matters on battery got buried a few years
ago ;)
> > A power saving feature that has a significant trade off between power
> > and performance.
>
> do you have numbers to explain "significant tradeoff" ?
I don't have numbers, but from the theory it seems pretty clear.
When you e.g. have two processes with 6MB cache foot print and
you have two 2C CPUs with 6MB cache they will fit in cache, but
with power aware scheduler they won't because both processes will run on
the single 6MB package. With NUMA the effect is even worse because
also the memory controllers are not used evenly.
And there's the FSB bandwidth, but that's a secondary issue.
>
>
> > This means performance will go down. Perhaps it would be ok on
> > battery,
>
> the illusion that power only matters on battery got buried a few years
> ago ;)
My understanding was always that unless you're on battery power saving
features that are enabled by default are not supposed to impact performance
significantly. When the user says impacting performance is ok then
doing that is fine of course, but not by default. And I don't think
that's an illusion. In fact if power saving means losing a lot of performance
people would get discouraged from using it, and surely you don't want that.
-Andi
On Tue, 29 Jul 2008 18:31:13 +0200
Andi Kleen <[email protected]> wrote:
> > > A power saving feature that has a significant trade off between
> > > power and performance.
> >
> > do you have numbers to explain "significant tradeoff" ?
>
> I don't have numbers, but from the theory it seems pretty clear.
> When you e.g. have two processes with 6MB cache foot print and
> you have two 2C CPUs with 6MB cache they will fit in cache, but
> with power aware scheduler they won't because both processes will run
> on the single 6MB package. With NUMA the effect is even worse because
> also the memory controllers are not used evenly.
> And there's the FSB bandwidth, but that's a secondary issue.
but if you have 2 threads that share a lot of data, it's the opposite.
Or if you have a bunch of things which are smaller than the cache...
(and they do share, for example glibc will be largely shared).
it's not a clear heavy loss, it's one of those "depends" cases.
(which sucks)
> >
> >
> > > This means performance will go down. Perhaps it would be ok on
> > > battery,
> >
> > the illusion that power only matters on battery got buried a few
> > years ago ;)
>
> My understanding was always that unless you're on battery power saving
> features that are enabled by default are not supposed to impact
> performance significantly.
That's not what datacenter people say. As long as power gain is bigger
than performance loss.. they tend to want it.
Also "significantly" is extremely subjective, like in this case it can
be a win or a loss, depending.
> When the user says impacting performance
> is ok then doing that is fine of course, but not by default.
that's a fine kernel policy.
Distros will override this policy if their users tell them they're
willing to do the tradeoff.. they will pick that default. In fact..
that's a big part of their job..
--
If you want to reach me at my work email, use [email protected]
For development, discussion and tips for power savings,
visit http://www.lesswatts.org
> That's not what datacenter people say. As long as power gain is bigger
> than performance loss.. they tend to want it.
That's a special case. It's fine but they should explicitely configure it.
I suspect even the data center people prefer "opt in" versus "opt out" here.
> Also "significantly" is extremely subjective, like in this case it can
> be a win or a loss, depending.
My impression is that the losses are more likely than the wins here.
>
> > When the user says impacting performance
> > is ok then doing that is fine of course, but not by default.
>
> that's a fine kernel policy.
>
> Distros will override this policy if their users tell them they're
> willing to do the tradeoff.. they will pick that default. In fact..
> that's a big part of their job..
I'm not fully convinced that was done intentionally in this case.
If there's an explicit setting somewhere that's fine anyways, but I think
here it more looks like a mistake.
-Andi
* Mike Galbraith <[email protected]> wrote:
> On Tue, 2008-07-29 at 00:26 +0200, Frederik Deweerdt wrote:
> > Hello Aneesh,
> >
> > On Fri, Jul 25, 2008 at 03:23:17PM +0530, Aneesh Kumar K.V wrote:
> > > [ 163.378265] BUG: unable to handle kernel NULL pointer dereference at 00000002
> > > [ 163.378276] IP: [<c0124933>] sched_power_savings_store+0x13/0x70
> >
> > Does the attached patch solve the problem?
>
> Patch seems to have missed the boat for rc1 too.
Greg?
Ingo
On Wed, 30 Jul 2008 11:05:49 +0200 Ingo Molnar <[email protected]> wrote:
>
> * Mike Galbraith <[email protected]> wrote:
>
> > On Tue, 2008-07-29 at 00:26 +0200, Frederik Deweerdt wrote:
> > > Hello Aneesh,
> > >
> > > On Fri, Jul 25, 2008 at 03:23:17PM +0530, Aneesh Kumar K.V wrote:
> > > > [ 163.378265] BUG: unable to handle kernel NULL pointer dereference at 00000002
> > > > [ 163.378276] IP: [<c0124933>] sched_power_savings_store+0x13/0x70
> > >
> > > Does the attached patch solve the problem?
> >
> > Patch seems to have missed the boat for rc1 too.
I grabbed Andi's 53rd copy and have sent it Linuswards.
> Greg?
We should make him maintain 2.6.27-rc1.1 ;)
On Wed, Jul 30, 2008 at 02:26:31AM -0700, Andrew Morton wrote:
> On Wed, 30 Jul 2008 11:05:49 +0200 Ingo Molnar <[email protected]> wrote:
>
> >
> > * Mike Galbraith <[email protected]> wrote:
> >
> > > On Tue, 2008-07-29 at 00:26 +0200, Frederik Deweerdt wrote:
> > > > Hello Aneesh,
> > > >
> > > > On Fri, Jul 25, 2008 at 03:23:17PM +0530, Aneesh Kumar K.V wrote:
> > > > > [ 163.378265] BUG: unable to handle kernel NULL pointer dereference at 00000002
> > > > > [ 163.378276] IP: [<c0124933>] sched_power_savings_store+0x13/0x70
> > > >
> > > > Does the attached patch solve the problem?
> > >
> > > Patch seems to have missed the boat for rc1 too.
>
> I grabbed Andi's 53rd copy and have sent it Linuswards.
>
> > Greg?
>
> We should make him maintain 2.6.27-rc1.1 ;)
I know I've been slow in getting back up to speed due to vacations and
work issues, but that's just cruel to suggest such a thing :)
thanks,
greg k-h