Messages in this thread | | | Date | Wed, 27 Nov 2002 11:05:58 -0600 (CST) | From | Kai Germaschewski <> | Subject | Re: Modules with list |
| |
On Tue, 26 Nov 2002, Adam J. Richter wrote:
> >No. That means dangling pointers everywhere. Remember dev_exit_p() > >and why it was introduced. > > First of all, let's understand how small this problem is. > dev_exit_p was introduceed to allow a debugging using a feature of > newer versions of ld. It was never essential. devexit_p() is only > relevant to non-hotplug systems.
That's wrong. It's essential, since the following happens:
static void __devexit my_remove() { }
static struct pci_driver my_driver = { .name = "my", .probe = my_probe, .remove = __devexit_p(my_remove), };
For CONFIG_HOTPLUG set, __devexit == "" and __devexit_p(x) == x, so everything's obviously fine.
For CONFIG_HOTPLUT not set, my_remove gets discarded from vmlinux, so we cannot set my_driver.remove = my_remove, since that symbol doesn't even exist anymore. Older binutils quietly accepted this, but recent ones correctly barf. That's why in that case __devexit_p(x) == NULL, so my_remove isn't referenced. It's not a debugging feature of ld, it's just correct behavior and it's not possible to turn it off.
> devexit_p() is currently is used only for static > initialization of driver->remove(), although I suspect that that ld > debugging feature has probably exposed some bugs that have been fixed. > So, in practice, all of the dangling pointers that it currently > avoids could be avoided by adding a few lines in places like > driver_register(): > > #ifndef CONFIG_HOTPLUG > if (!driver->module->removable) > driver->remove = invoke_bug; > #endif
Since in that case driver->remove == NULL, that gives you a nice oops when trying to invoke it, no need for an explicit invoke_bug ;)
> It's probably overkill, but, in the longer term, we could > eliminate CONFIG_HOTPLUG, have .devexit{,data} sections, and build yet > another ELF section listing the devexit_p pointers, by defining > devexit_p like so. > > #define devexit_p(symbol) ( \ > asm (".pushsection .devexit_p_refs\n" \ > ".long " #symbol "\n" \ > ".popsection\n"); \ > symbol )
Again, that doesn't fix the problem that we have a reference to a symbol which doesn't exist since we just discarded it.
One way to fix this is to make my_remove a weak symbol, so that the linker just silently puts in NULL when it's not defined elsewhere, which would avoid the ugly __devexit_p(). Not sure if that doesn't end up in even uglier hacks, though.
Another way to fix this is of course to always have CONFIG_HOTPLUG=y, which may become necessary anyway when properly shutting down devices at shutdown/reboot.
--Kai
- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |