Re: 2.6.26-rc9-mmotm oops - suspect kernel-schedc-add-missing-attr-parameter.patch

From: Takashi Iwai
Date: Mon Jul 14 2008 - 03:11:47 EST


At Sun, 13 Jul 2008 21:02:52 -0700,
Andrew Morton wrote:
>
> On Sun, 13 Jul 2008 23:19:01 -0400 Valdis.Kletnieks@xxxxxx wrote:
>
> > On Sun, 13 Jul 2008 05:45:53 EDT, Valdis.Kletnieks@xxxxxx said:
> > > --==_Exmh_1215942353_3043P
> > > Content-Type: text/plain; charset=us-ascii
> > >
> > > kernel-schedc-add-missing-attr-parameter.patch is my leading suspect, as it's
> > > the last thing to poke around in the failing code...
> > >
> > > My /etc/rc.local had this in it:
> > >
> > > echo 1 > /sys/devices/system/cpu/sched_mc_power_savings
> > >
> > > Which got me an oops:
> >
> > powertop 1.11 just pulled another oops on me, that looks related:
> >
> > [ 9802.991006] BUG: unable to handle kernel paging request at ffffffff802291c0
> > [ 9802.991006] IP: [<ffffffff8037fba0>] number+0x1e6/0x216
> > [ 9802.991006] PGD 203067 PUD 207063 PMD 7f84c163 PTE 229161
> > [ 9802.991006] Oops: 0003 [1] PREEMPT SMP
> > [ 9802.991006] last sysfs file: /sys/devices/system/cpu/sched_mc_power_savings
> > [ 9802.991006] CPU 0
> > [ 9802.991006] Modules linked in: irnet ppp_generic slhc irtty_sir sir_dev ircomm_tty ircomm irda crc_ccitt coretemp nf_conntrack_ftp xt_pkttype ipt_REJECT ipt_osf nf_conntrack_ipv4 xt_ipisforif ipt_recent ipt_LOG xt_u32 iptable_filter ip_tables xt_tcpudp nf_conntrack_ipv6 xt_state nf_conntrack ip6t_LOG xt_limit ip6table_filter ip6_tables x_tables sha256_generic aes_generic rtc acpi_cpufreq tpm_tis tpm tpm_bios pcmcia arc4 ecb iwl3945 yenta_socket rsrc_nonstatic rfkill pcmcia_core dcdbas ohci1394 nvidia(P) iTCO_wdt ieee1394 mac80211 video iTCO_vendor_support snd_hda_intel intel_agp output led_class thermal tg3 processor ac battery firmware_class button cfg80211 libphy [last unloaded: microcode]
> > [ 9802.991006] Pid: 21426, comm: powertop Tainted: P 2.6.26-rc9-mm1 #2
> > [ 9802.991006] RIP: 0010:[<ffffffff8037fba0>] [<ffffffff8037fba0>] number+0x1e6/0x216
> > [ 9802.991006] RSP: 0018:ffff88005188dba8 EFLAGS: 00010293
> > [ 9802.991006] RAX: 0000000000000030 RBX: ffff88005188ddb8 RCX: 0000000000000000
> > [ 9802.991006] RDX: 00000000fffffffc RSI: 0000000000000000 RDI: 0000000000000000
> > [ 9802.991006] RBP: ffff88005188dc48 R08: 00000000ffffffff R09: 0000000000000000
> > [ 9802.991006] R10: ffffffffffffffff R11: 000000000000000a R12: ffffffff802291c0
> > [ 9802.991006] R13: 00000000ffffffff R14: 0000000000000000 R15: 0000000000000000
> > [ 9802.991006] FS: 00007f85234d76f0(0000) GS:ffffffff80770ec0(0000) knlGS:0000000000000000
> > [ 9802.991006] CS: 0010 DS: 0000 ES: 0000 CR0: 000000008005003b
> > [ 9802.991006] CR2: ffffffff802291c0 CR3: 0000000067285000 CR4: 00000000000006e0
> > [ 9802.991006] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
> > [ 9802.991006] DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000400
> > [ 9802.991006] Process powertop (pid: 21426, threadinfo ffff88005188c000, task ffff88005afe1600)
> > [ 9802.991006] Stack: ffffffff80348e86 0000000000000001 ffff88005188dbf8 00ffffffffffffff
> > [ 9802.991006] ffffffff80349430 ffffffff80349e86 0000000000000001 ffff88006bc43fb0
> > [ 9802.991006] 0000000000000203 000000000000002d ffff88005188dc18 ffffffff80561638
> > [ 9802.991006] Call Trace:
> > [ 9802.991006] [<ffffffff80348e86>] ? avc_has_perm_noaudit+0x4e/0x62b
> > [ 9802.991006] [<ffffffff80349430>] ? avc_has_perm_noaudit+0x5f8/0x62b
> > [ 9802.991006] [<ffffffff80349e86>] ? avc_has_perm+0x31/0x60
> > [ 9802.991006] [<ffffffff80561638>] ? sub_preempt_count+0x60/0x75
> > [ 9802.991006] [<ffffffff8025a95a>] ? __lock_acquire+0x6e/0x1131
> > [ 9802.991006] [<ffffffff802291c0>] ? sched_mc_power_savings_show+0x0/0x1f
> > [ 9802.991006] [<ffffffff803808dc>] vsnprintf+0x59e/0x5f0
> > [ 9802.991006] [<ffffffff802291c0>] ? sched_mc_power_savings_show+0x0/0x1f
>
> hm, yes, I'd say that you have fingered the appropriate patch. But
> without that patch, the warnings which it fixes reappear, so we're
> busted either way.

My fix wasn't right. Please drop it.
Just reading the code more further, the problem is that sched.c uses
sysfs_create_file() for sysdev_class. The below is an untested fix.


thanks,

Takashi


===
[PATCH] Use sysdev_class* in sched.c

The power_saving* attributes are really sysdev_class stuff, so we
must create them via sysdev_class_create_file(). The parameters of
callback functions must be also fixed.


Signed-off-by: Takashi Iwai <tiwai@xxxxxxx>

---
diff --git a/kernel/sched.c b/kernel/sched.c
index 8500035..eee1888 100644
--- a/kernel/sched.c
+++ b/kernel/sched.c
@@ -7732,31 +7732,35 @@ 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, 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,
+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,
- sched_mc_power_savings_store);
+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, 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,
+static ssize_t sched_smt_power_savings_store(struct sysdv_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,
- sched_smt_power_savings_store);
+static SYSDEV_CLASS_ATTR(sched_smt_power_savings, 0644,
+ sched_smt_power_savings_show,
+ sched_smt_power_savings_store);
#endif

int sched_create_sysfs_power_savings_entries(struct sysdev_class *cls)
@@ -7765,13 +7769,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;
}
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/