[  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>] ...
[ message continues ]

Hello  Aneesh,


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 ...
[ message continues ]

--

[ message continues ]

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

--

[ message continues ]

On Tue, 29 Jul 2008 14:09:11 +0200

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 arjan@linux.intel.com
For development, discussion and tips for power savings, 
visit http://www.lesswatts.org
--

[ message continues ]

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
--

[ message continues ]

On Tue, 29 Jul 2008 16:53:48 +0200


the illusion that power only matters on battery got buried a few years
ago ;)


--

[ message continues ]

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.

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

--

[ message continues ]

On Tue, 29 Jul 2008 18:31:13 +0200

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.

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

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 arjan@linux.intel.com
For development, discussion and tips for power savings, 
visit http://www.lesswatts.org
--

[ message continues ]

That's a special case. It's fine but they should explicitely configure it.


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
--

[ message continues ]

Greg?

	Ingo
--

[ message continues ]

We should make him maintain 2.6.27-rc1.1 ;)
--

[ message continues ]

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
--

[ message continues ]