login
Search
Hi all, News: This is the 6 month anniversary of linux-next (the first release was Feb 14). Changes since next-20080813: The usb-current tree lost its conflict. The ubifs tree lost its conflict. The slab tree lost its conflict. The hdlc tree gained a build fix patch. The ttydev tree gained a conflict against the usb tree. The creds tree gained a conflict against the ttydev tree and lost one against Linus' tree. I have also applied the following patches for known problems: Add cuImage.mpc866ads to the bootwrapper as a cuboot-8xx target xen-balloon: fix up sysfs issues ath9k: work around gcc ICE again usb: fix up use of usb_dbg (modified from yesterday to cover more failures) ---------------------------------------------------------------------------- I have created today's linux-next tree at git://git.kernel.org/pub/scm/linux/kernel/git/sfr/linux-next.git (patches at http://www.kernel.org/pub/linux/kernel/people/sfr/linux-next/). If you are tracking the linux-next tree using git, you should not use "git pull" to do so as that will try to merge the new linux-next release with the old one. You should use "git fetch" as mentioned in the FAQ on the wiki (see below). You can see which trees have been included by looking in the Next/Trees file in the source. There are also quilt-import.log and merge.log files in the Next directory. Between each merge, the tree was built with a ppc64_defconfig for powerpc and an allmodconfig for x86_64. After the final fixups, it is also built with powerpc allnoconfig, 44x_defconfig and allyesconfig and i386, sparc and sparc64 defconfig. Below is a summary of the state of the merge. We are up to 108 trees (counting Linus' and 14 trees of patches pending for Linus' tree), more are welcome (even if they are currently empty). Thanks to those who have contributed, and to those who haven't, please do. Status of my local build tests will be at http://kisskb.ellerman.id.au/linux-next . If maintainers want to ...
And linux-next helped me a lot in finding breakage early. Thanks for
your efforts, Stephen!
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
--
calling param_sysfs_init+0x0/0x1e7 ------------[ cut here ]------------ WARNING: at fs/sysfs/dir.c:463 sysfs_add_one+0x35/0x3d() sysfs: duplicate filename 'acpi' can not be created Modules linked in: Pid: 1, comm: swapper Not tainted 2.6.27-rc3-next-20080814 #1 Call Trace: [<ffffffff802358cb>] warn_slowpath+0xae/0xdd [<ffffffff8036a6be>] ? number+0x115/0x21f [<ffffffff8036574d>] ? __next_cpu+0x19/0x26 [<ffffffff8022d575>] ? find_busiest_group+0x25b/0x6ee [<ffffffff8036aefc>] ? string+0x3d/0xa2 [<ffffffff80365e02>] ? idr_get_empty_slot+0x161/0x24e [<ffffffff8036b2ea>] ? vsnprintf+0x389/0x678 [<ffffffff802def96>] ? sysfs_ilookup_test+0x0/0x14 [<ffffffff802aa5bf>] ? ifind+0x3d/0xac [<ffffffff802df1bd>] ? sysfs_find_dirent+0x1c/0x31 [<ffffffff802df1f1>] ? __sysfs_add_one+0x1f/0x7c [<ffffffff802df283>] sysfs_add_one+0x35/0x3d [<ffffffff802df7d2>] create_dir+0x58/0x93 [<ffffffff802df845>] sysfs_create_dir+0x38/0x4f [<ffffffff80366b8a>] kobject_add_internal+0xce/0x18f [<ffffffff80366d17>] kobject_add_varg+0x41/0x4d [<ffffffff80366d91>] kobject_init_and_add+0x6e/0x7c [<ffffffff803676e5>] ? kobject_uevent_env+0x350/0x37f [<ffffffff802455bf>] ? param_sysfs_setup+0xe1/0x104 [<ffffffff80764a30>] kernel_param_sysfs_setup+0x6b/0xc6 [<ffffffff80764bee>] param_sysfs_init+0x163/0x1e7 [<ffffffff80249b00>] ? ktime_get_ts+0x14/0x4e [<ffffffff80764a8b>] ? param_sysfs_init+0x0/0x1e7 [<ffffffff80209047>] _stext+0x47/0x13f [<ffffffff80269f25>] ? register_irq_proc+0xb7/0xd3 [<ffffffff807508e5>] kernel_init+0x11b/0x171 [<ffffffff802311ef>] ? schedule_tail+0x28/0x60 [<ffffffff8020ce49>] child_rip+0xa/0x11 [<ffffffff807507ca>] ? kernel_init+0x0/0x171 [<ffffffff8020ce3f>] ? child_rip+0x0/0x11 ---[ end trace 4eaa2a86a8e2da22 ]--- kobject_add_internal failed for acpi with -EEXIST, don't try to register things with the same name in the same directory. Pid: 1, comm: swapper Tainted: G W 2.6.27-rc3-next-20080814 #1 Call Trace: [<ffffffff8036698f>] ? ...
Why would this be a sysfs error, it's acpi doing something foolish, that's why the sysfs layer is warning about it :) thanks, greg k-h --
The NULL pointer reference further down actually looks like something foolish in the sysfs layer. But actually ACPI is not doing anything wrong here I think. According to the backtrace it happens when a kernel param (aka module_param) is set. And creating multiple params in the same acpi space is completely legal. It looks more like the high level code that sets up these parameters broke somehow and starts registering these twice and now ACPI is the first one to hit it (maybe because it starts with 'a' :-). Or perhaps sysfs checking just got more anal and the params code relied on being able to register duplicate directories? Putting rusty into cc. -Andi calling param_sysfs_init+0x0/0x1e7 ------------[ cut here ]------------ WARNING: at fs/sysfs/dir.c:463 sysfs_add_one+0x35/0x3d() sysfs: duplicate filename 'acpi' can not be created Modules linked in: Pid: 1, comm: swapper Not tainted 2.6.27-rc3-next-20080814 #1 Call Trace: [<ffffffff802358cb>] warn_slowpath+0xae/0xdd [<ffffffff8036a6be>] ? number+0x115/0x21f [<ffffffff8036574d>] ? __next_cpu+0x19/0x26 [<ffffffff8022d575>] ? find_busiest_group+0x25b/0x6ee [<ffffffff8036aefc>] ? string+0x3d/0xa2 [<ffffffff80365e02>] ? idr_get_empty_slot+0x161/0x24e [<ffffffff8036b2ea>] ? vsnprintf+0x389/0x678 [<ffffffff802def96>] ? sysfs_ilookup_test+0x0/0x14 [<ffffffff802aa5bf>] ? ifind+0x3d/0xac [<ffffffff802df1bd>] ? sysfs_find_dirent+0x1c/0x31 [<ffffffff802df1f1>] ? __sysfs_add_one+0x1f/0x7c [<ffffffff802df283>] sysfs_add_one+0x35/0x3d [<ffffffff802df7d2>] create_dir+0x58/0x93 [<ffffffff802df845>] sysfs_create_dir+0x38/0x4f [<ffffffff80366b8a>] kobject_add_internal+0xce/0x18f [<ffffffff80366d17>] kobject_add_varg+0x41/0x4d [<ffffffff80366d91>] kobject_init_and_add+0x6e/0x7c [<ffffffff803676e5>] ? kobject_uevent_env+0x350/0x37f [<ffffffff802455bf>] ? param_sysfs_setup+0xe1/0x104 [<ffffffff80764a30>] kernel_param_sysfs_setup+0x6b/0xc6 [<ffffffff80764bee>] ...
Perhaps we could try "acpi=off" and see if it still happens. thanks, rui --
I doubt that would help because module_param()s are always registered even with acpi=off. They are not really processed by ACPI, but by kernel/params.c -Andi --
Please attach the full dmesg output. thanks, rui --
--- ~Randy Linux Plumbers Conference, 17-19 September 2008, Portland, Oregon USA http://linuxplumbersconf.org/
Hi Randy,
care to add a printk to the module sysfs setup, and post dmesg again?
--- a/kernel/params.c
+++ b/kernel/params.c
@@ -603,6 +603,8 @@ static void __init param_sysfs_builtin(void)
}
name_len = dot - kp->name;
+ printk("XXX adding modparam:'%s' %i (%p)\n", kp->name, i, kp);
+
/* new kbuild_modname? */
if (strlen(modname) != name_len
|| strncmp(modname, kp->name, name_len) != 0) {
I might show the order of registering the /sys/modules/ directory, and
if there is possibly another "acpi" section, which tries to add
parameter names.
Thanks,
Kay
--
Done. Full boot log is attached. Extract is: XXX adding modparam:'acpi.power_nocheck' 34 (ffffffff806a4cf0) XXX adding modparam:'processor.latency_factor' 35 (ffffffff806a4d18) XXX adding modparam:'processor.nocst' 36 (ffffffff806a4d40) XXX adding modparam:'processor.max_cstate' 37 (ffffffff806a4d68) XXX adding modparam:'processor.ignore_ppc' 38 (ffffffff806a4d90) XXX adding modparam:'thermal.psv' 39 (ffffffff806a4db8) XXX adding modparam:'thermal.off' 40 (ffffffff806a4de0) XXX adding modparam:'thermal.nocrt' 41 (ffffffff806a4e08) XXX adding modparam:'thermal.tzp' 42 (ffffffff806a4e30) XXX adding modparam:'thermal.crt' 43 (ffffffff806a4e58) XXX adding modparam:'thermal.act' 44 (ffffffff806a4e80) XXX adding modparam:'acpi.acpica_version' 45 (ffffffff806a4ea8) XXX adding modparam:'keyboard.brl_nbchords' 46 (ffffffff806a4ed0) ------------[ cut here ]------------ WARNING: at fs/sysfs/dir.c:463 sysfs_add_one+0x35/0x3d() sysfs: duplicate filename 'acpi' can not be created --- ~Randy Linux Plumbers Conference, 17-19 September 2008, Portland, Oregon USA http://linuxplumbersconf.org/
Two different "modules" use the same prefix, which does not work with the current logic, they need to live next to each other in the sequence of options. This adds a new option: http://git.kernel.org/?p=linux/kernel/git/sfr/linux-next.git;a=commitdiff;h=1382827e93... which specifies MODULE_PARAM_PREFIX="acpi." in: drivers/acpi/power.c In the same way as: drivers/acpi/system.c Seems, two different modules should not declare parameters in different locations, and use the same MODULE_PARAM_PREFIX. Thanks, Kay --
That seems bogus to me. Assuming we have some code in a module and then split it out into two different modules. Or move an option from one file to another. Would we need to change the option name then? I think the generic params code should be fixed to handle this. -Andi --
Yeah, they are stored in a global array __start___param, and the current code expects the prefixes to be sorted, so the sysfs directories can be They have been module options, not prefixed kernel parameters so far, and the prefix was just the module name. So it just strikes back, that acpi uses generic names for the modules, there would have been no problem if "power" would be called "acpi_power" and the options would just be "acpi.acpica_version" and "acpi_power.nocheck". But well, there are driver modules just called "option", so acpi is not We could try to look up existing directories to use instead of expecting that we need to create and own them. I guess, we have the same issue if we compile the code as a module and load it later, while some other built-in code or module has already used the same prefix. Kay --
sysfs does this anyways, doesn't it. We would just need to teach it to not BUG() in this case, perhaps with a special entry point. Also a BUG() in general seems a little harsh for this, surely a WARN_ON should be enough. -Andi --
It is a WARN() call, not a BUG(). thanks, greg k-h --
Ok. Can we remove it? Or add a new entry point that allows to disable it? I don't think relying on link order like Rusty proposes is a good long term solution. -Andi --
No! What you are doing here is wrong, trying to create two files with the same name. You just should not be doing that at all, it's that simple. Fix the broken code/link order, don't paper it over in the sysfs layer. thanks, greg k-h --
Sorry, but relying on link order for anything is a mistake. It is subtle and fragile and just means it'll eventually break again because it's near impossible to properly maintain. The alternative is to always check if the file exists in the params.c code before creating it. We can do that too, but it's somewhat dumb because sysfs is already doing that. -Andi --
We rely on link order for all sorts of things, this isn't new at all. thanks, greg k-h --
Sure, but this code should be rewritten to check if the directory exists,
rather assuming it based on "previous prefix was the same".
It's relying on a horribly undocumented assumption, and it broke.
We need to change kernel_param_sysfs_setup() to do something
like "kobject_find(module_kset, name)" and only allocate a new mk if that
fails. Then we need to change the code to allow appending, like below.
diff -r e92a03313dc7 kernel/params.c
--- a/kernel/params.c Mon Aug 18 14:16:00 2008 +1000
+++ b/kernel/params.c Mon Aug 18 18:52:20 2008 +1000
@@ -384,6 +384,7 @@ struct param_attribute
struct module_param_attrs
{
+ unsigned int num;
struct attribute_group grp;
struct param_attribute attrs[0];
};
@@ -434,69 +435,86 @@ static ssize_t param_attr_store(struct m
#ifdef CONFIG_SYSFS
/*
- * param_sysfs_setup - setup sysfs support for one module or KBUILD_MODNAME
+ * add_sysfs_param - add a parameter to sysfs
+ * @mp: pointer to previous module_param_attrs, or NULL.
* @mk: struct module_kobject (contains parent kobject)
- * @kparam: array of struct kernel_param, the actual parameter definitions
- * @num_params: number of entries in array
- * @name_skip: offset where the parameter name start in kparam[].name. Needed for built-in "modules"
+ * @kparam: the actual parameter definition to add to sysfs
+ * @name: name of parameter
*
- * Create a kobject for a (per-module) group of parameters, and create files
- * in sysfs. A pointer to the param_kobject is returned on success,
- * NULL if there's no parameter to export, or other ERR_PTR(err).
+ * Create a kobject if for a (per-module) parameter if mp NULL, and
+ * create file in sysfs. Returns an error on out of memory. Always cleans up
+ * if there's an error.
*/
-static __modinit struct module_param_attrs *
-param_sysfs_setup(struct module_kobject *mk,
- struct kernel_param *kparam,
- unsigned int num_params,
- unsigned int name_skip)
+static __modinit int add_sysfs_param(struct ...kobj = kset_find_obj(module_kset, name) should return an existing object with that name (and take a reference on it, which needs to be dropped). Kay --
To be clear, I agree with Andi. If this is for current kernel I'd just fix link order, for longer term we need something cleverer. Cheers, Rusty. --
> > >>> "acpi_power" and the options would just be
There's a proper fix in linux-next for 2.6.28. I was unclear about whether this ACPI change was going into 2.6.27; since it's not, there's no real issue. Hope that helps, Rusty. --
Simplest fix is to shuffle Makefile. But better is to create an acpi "module" so the namespacing just works, something like below. Overriding MODULE_PREFIX only works for builtin code anyway. (Which makes sense: moving a parameter from one module to another isn't a change we can cover up). (Sam: foo-objs-y would make this neater) diff -r 5f7194400572 drivers/acpi/Makefile --- a/drivers/acpi/Makefile Sat Aug 16 13:23:26 2008 +1000 +++ b/drivers/acpi/Makefile Sat Aug 16 13:44:17 2008 +1000 @@ -21,10 +21,18 @@ obj-$(CONFIG_X86) += blacklist.o # # ACPI Core Subsystem (Interpreter) # -obj-y += osl.o utils.o reboot.o\ +obj-y += osl.o utils.o reboot.o acpi.o \ dispatcher/ events/ executer/ hardware/ \ namespace/ parser/ resources/ tables/ \ utilities/ + +# "acpi." module_param namespace. +ifdef CONFIG_ACPI_POWER +acpi-objs += power.o +endif +ifdef CONFIG_ACPI_SYSTEM +acpi-objs += system.o event.o +endif # # ACPI Bus and Device Drivers @@ -49,11 +57,9 @@ obj-$(CONFIG_ACPI_VIDEO) += video.o obj-$(CONFIG_ACPI_VIDEO) += video.o obj-y += pci_root.o pci_link.o pci_irq.o pci_bind.o obj-$(CONFIG_ACPI_PCI_SLOT) += pci_slot.o -obj-$(CONFIG_ACPI_POWER) += power.o obj-$(CONFIG_ACPI_PROCESSOR) += processor.o obj-$(CONFIG_ACPI_CONTAINER) += container.o obj-$(CONFIG_ACPI_THERMAL) += thermal.o -obj-$(CONFIG_ACPI_SYSTEM) += system.o event.o obj-$(CONFIG_ACPI_DEBUG) += debug.o obj-$(CONFIG_ACPI_NUMA) += numa.o obj-$(CONFIG_ACPI_WMI) += wmi.o diff -r 5f7194400572 drivers/acpi/system.c --- a/drivers/acpi/system.c Sat Aug 16 13:23:26 2008 +1000 +++ b/drivers/acpi/system.c Sat Aug 16 13:44:17 2008 +1000 @@ -33,10 +33,6 @@ #define _COMPONENT ACPI_SYSTEM_COMPONENT ACPI_MODULE_NAME("system"); -#ifdef MODULE_PARAM_PREFIX -#undef MODULE_PARAM_PREFIX -#endif -#define MODULE_PARAM_PREFIX "acpi." #define ACPI_SYSTEM_CLASS "system" #define ACPI_SYSTEM_DEVICE_NAME "System" --
Can't say I like this. This is fragile. I can just this exploding the next time again with some innocent change. -Andi --
becomes: acpi-y += system.o event.o [Assuming both config symbols are bool] Sam --
Ah, missed that. Thanks Sam! I had another bug: if both options were off, compile would fail. Put everything in acpi.o to fix this. Rusty. diff -r 79f0d024aebc drivers/acpi/Makefile --- a/drivers/acpi/Makefile Fri Aug 15 11:23:34 2008 +1000 +++ b/drivers/acpi/Makefile Sat Aug 16 15:55:10 2008 +1000 @@ -21,10 +21,15 @@ obj-$(CONFIG_X86) += blacklist.o # # ACPI Core Subsystem (Interpreter) # -obj-y += osl.o utils.o reboot.o\ +obj-y += acpi.o \ dispatcher/ events/ executer/ hardware/ \ namespace/ parser/ resources/ tables/ \ utilities/ + +# "acpi." module_param namespace. +acpi-y += osl.o utils.o reboot.o +acpi-$(CONFIG_ACPI_POWER) += power.o +acpi-$(CONFIG_ACPI_SYSTEM) += system.o event.o # # ACPI Bus and Device Drivers @@ -49,11 +54,9 @@ obj-$(CONFIG_ACPI_VIDEO) += video.o obj-$(CONFIG_ACPI_VIDEO) += video.o obj-y += pci_root.o pci_link.o pci_irq.o pci_bind.o obj-$(CONFIG_ACPI_PCI_SLOT) += pci_slot.o -obj-$(CONFIG_ACPI_POWER) += power.o obj-$(CONFIG_ACPI_PROCESSOR) += processor.o obj-$(CONFIG_ACPI_CONTAINER) += container.o obj-$(CONFIG_ACPI_THERMAL) += thermal.o -obj-$(CONFIG_ACPI_SYSTEM) += system.o event.o obj-$(CONFIG_ACPI_DEBUG) += debug.o obj-$(CONFIG_ACPI_NUMA) += numa.o obj-$(CONFIG_ACPI_WMI) += wmi.o diff -r 79f0d024aebc drivers/acpi/system.c --- a/drivers/acpi/system.c Fri Aug 15 11:23:34 2008 +1000 +++ b/drivers/acpi/system.c Sat Aug 16 15:55:10 2008 +1000 @@ -33,10 +33,6 @@ #define _COMPONENT ACPI_SYSTEM_COMPONENT ACPI_MODULE_NAME("system"); -#ifdef MODULE_PARAM_PREFIX -#undef MODULE_PARAM_PREFIX -#endif -#define MODULE_PARAM_PREFIX "acpi." #define ACPI_SYSTEM_CLASS "system" #define ACPI_SYSTEM_DEVICE_NAME "System" --
Tainted by some prior sysfs warn_on crapola. BUG: unable to handle kernel NULL pointer dereference at 0000000000000248 IP: [<ffffffffa001cb68>] do_cciss_intr+0x627/0xa6c [cciss] PGD 17eba6067 PUD 17eba7067 PMD 0 Oops: 0002 [1] SMP last sysfs file: /sys/block/ram15/dev CPU 2 Modules linked in: cciss(+) ehci_hcd ohci_hcd uhci_hcd Pid: 0, comm: swapper Tainted: G W 2.6.27-rc3-next-20080814 #1 RIP: 0010:[<ffffffffa001cb68>] [<ffffffffa001cb68>] do_cciss_intr+0x627/0xa6c [cciss] RSP: 0018:ffff88027f66bee8 EFLAGS: 00010007 RAX: 0000000000000000 RBX: ffff88007f840270 RCX: 000000000000000c RDX: 0000000000000000 RSI: ffff88027e560000 RDI: ffff88027e560000 RBP: ffff88027f66bf18 R08: 0000000000000000 R09: ffff88017fa99e88 R10: 0000000000000000 R11: 000000008022ec53 R12: ffff88027e560000 R13: 0000000000000000 R14: 00000000000001f7 R15: 0000000000000086 FS: 0000000000680850(0000) GS:ffff88017fc02b80(0000) knlGS:0000000000000000 CS: 0010 DS: 0018 ES: 0018 CR0: 000000008005003b CR2: 0000000000000248 CR3: 000000017eba0000 CR4: 00000000000006e0 DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000 DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000400 Process swapper (pid: 0, threadinfo ffff88017fa98000, task ffff88027f63d280) Stack: ffff88027f66bee0 ffff88027f64a800 0000000000000000 0000000000000000 00000000000001f7 0000000000000000 ffff88027f66bf48 ffffffff8026807a ffffffff8073a000 00000000000001f7 ffff88027f64a800 ffffffff8073a050 Call Trace: <IRQ> [<ffffffff8026807a>] handle_IRQ_event+0x27/0x57 [<ffffffff802698dc>] handle_edge_irq+0xed/0x12e [<ffffffff8020eaac>] do_IRQ+0xf7/0x16f [<ffffffff8020c471>] ret_from_intr+0x0/0xa <EOI> [<ffffffff802122ed>] ? default_idle+0x2b/0x40 [<ffffffff802124fb>] ? c1e_idle+0xd4/0xdb [<ffffffff8055d0bb>] ? atomic_notifier_call_chain+0xf/0x11 [<ffffffff8020ac61>] ? cpu_idle+0x5f/0x7d [<ffffffff80554f4f>] ? start_secondary+0x16c/0x171 Code: 8b 83 48 02 00 00 48 39 d8 74 37 49 39 9c 24 c0 00 01 00 75 ...
