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/
This archive was generated by hypermail 2b29 : Sat Nov 30 2002 - 22:00:17 EST