| Message ID | 1254076075.6583.6.camel@johannes.local |
|---|---|
| State | Deferred, archived |
| Delegated to: | David Miller |
| Headers | show |
On Sun, Sep 27, 2009 at 11:27 AM, Johannes Berg <johannes@sipsolutions.net> wrote: > The point we assign dev->wireless_handlers at is too > late, we need to do that before netdev_register_kobject() > gets called, so use the new NETDEV_PRE_INIT notifier. > The result of adding wireless_handlers too late is the > disappearance of /sys/class/net/wlan0/wireless which a > bunch of distro scripts still require. > > Signed-off-by: Johannes Berg <johannes@sipsolutions.net> > --- > This should fix the regression Hugh reported (of course requires the > other patch which unfortunately I forgot to CC you, Hugh, I'll send you > a copy in private). Are these stable fixes? Luis -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Sun, 27 Sep 2009, Johannes Berg wrote: > The point we assign dev->wireless_handlers at is too > late, we need to do that before netdev_register_kobject() > gets called, so use the new NETDEV_PRE_INIT notifier. > The result of adding wireless_handlers too late is the > disappearance of /sys/class/net/wlan0/wireless which a > bunch of distro scripts still require. > > Signed-off-by: Johannes Berg <johannes@sipsolutions.net> This 2/2 patch does indeed work: many thanks. But... > --- > This should fix the regression Hugh reported (of course requires the > other patch which unfortunately I forgot to CC you, Hugh, I'll send you > a copy in private). ... the 1/2 patch has a couple of problems. The trivial problem is that include/linux/notifier.h in Linus's git has #define NETDEV_PRE_UP 0x000D #define NETDEV_BONDING_OLDTYPE 0x000E #define NETDEV_BONDING_NEWTYPE 0x000F So it rejects the patch you posted: I changed it to add #define NETDEV_PRE_INIT 0x0010 just after the NETDEV_BONDING_NEWTYPE line. The more serious problem is that it stops both my machines from booting, too early for framebuffer to show any messages, but vga=normal shows a long stacktrace scrolling offscreen, with some notifier stuff in there. I've experimented by moving your NETDEV_PRE_INIT hunk later in the sequence, just before the netdev_initialize_kobject(dev) (so I also changed the "goto out" to "goto err_uninit"): both* machines then boot correctly, and this mail leaves me wirelessly. I'll now experiment to see how early I can move that hunk. Hugh * I wrote that hoping the Aspire One build would complete soon, but it's still going: assume it's fine unless I say otherwise. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Sun, 27 Sep 2009, Hugh Dickins wrote: > > I've experimented by moving your NETDEV_PRE_INIT hunk later in the > sequence, just before the netdev_initialize_kobject(dev) (so I also > changed the "goto out" to "goto err_uninit"): both* machines then boot > correctly, and this mail leaves me wirelessly. > > I'll now experiment to see how early I can move that hunk. Both machines boot (and do wireless) correctly with your NETDEV_PRE_INIT hunk placed just after the ndo_init block, instead of just before where you placed it. That's i386 kernels on both. But curiouser and curiouser... the laptop can do 64-bit, so I built my 64-bit kernel, and went through the motions to reproduce the early boot crash with the patch as you had it: but the x86_64 kernel boots (and does wireless) correctly with the NETDEV_PRE_INIT hunk just where you placed it, before the ndo_init block. Perhaps there's a difference in the 32- and 64-bit startup sequence with respect to notifiers; or perhaps your change tickles another bug. I don't know, I'm turning off now. Hugh -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Sun, 2009-09-27 at 21:58 +0100, Hugh Dickins wrote: > This 2/2 patch does indeed work: many thanks. But... ... it won't fix anything by itself. > > This should fix the regression Hugh reported (of course requires the > > other patch which unfortunately I forgot to CC you, Hugh, I'll send you > > a copy in private). > > ... the 1/2 patch has a couple of problems. > > The trivial problem is that include/linux/notifier.h in Linus's git has > #define NETDEV_PRE_UP 0x000D > #define NETDEV_BONDING_OLDTYPE 0x000E > #define NETDEV_BONDING_NEWTYPE 0x000F > > So it rejects the patch you posted: I changed it to add > #define NETDEV_PRE_INIT 0x0010 > just after the NETDEV_BONDING_NEWTYPE line. Right, that's fixable, I was evidently working against the wrong tree. > The more serious problem is that it stops both my machines from booting, > too early for framebuffer to show any messages, but vga=normal shows a > long stacktrace scrolling offscreen, with some notifier stuff in there. Ahrg. That's strange. > I've experimented by moving your NETDEV_PRE_INIT hunk later in the > sequence, just before the netdev_initialize_kobject(dev) (so I also > changed the "goto out" to "goto err_uninit"): both* machines then boot > correctly, and this mail leaves me wirelessly. > > I'll now experiment to see how early I can move that hunk. Interesting. Thanks for that; must be some notifier that doesn't expect to be run. For all I care, it can be right before the kobject stuff, so maybe that makes more sense as it's a fairly last-minute fix now. Of course then it should be POST_INIT or PRE_REGISTER I suppose. johannes
On Sun, 2009-09-27 at 22:50 +0100, Hugh Dickins wrote: > On Sun, 27 Sep 2009, Hugh Dickins wrote: > > > > I've experimented by moving your NETDEV_PRE_INIT hunk later in the > > sequence, just before the netdev_initialize_kobject(dev) (so I also > > changed the "goto out" to "goto err_uninit"): both* machines then boot > > correctly, and this mail leaves me wirelessly. > > > > I'll now experiment to see how early I can move that hunk. > > Both machines boot (and do wireless) correctly with your NETDEV_PRE_INIT > hunk placed just after the ndo_init block, instead of just before where > you placed it. That's i386 kernels on both. > > But curiouser and curiouser... the laptop can do 64-bit, so I built > my 64-bit kernel, and went through the motions to reproduce the > early boot crash with the patch as you had it: but the x86_64 kernel > boots (and does wireless) correctly with the NETDEV_PRE_INIT hunk > just where you placed it, before the ndo_init block. > > Perhaps there's a difference in the 32- and 64-bit startup sequence > with respect to notifiers; or perhaps your change tickles another bug. > I don't know, I'm turning off now. Interesting, thanks for taking the time to test. I'll post an updated patchset that calls it POST_INIT and moves it to there. johannes
--- wireless-testing.orig/net/wireless/core.c 2009-09-27 15:12:20.000000000 +0200 +++ wireless-testing/net/wireless/core.c 2009-09-27 15:12:54.000000000 +0200 @@ -641,6 +641,12 @@ static int cfg80211_netdev_notifier_call WARN_ON(wdev->iftype == NL80211_IFTYPE_UNSPECIFIED); switch (state) { + case NETDEV_PRE_INIT: +#ifdef CONFIG_WIRELESS_EXT + if (!dev->wireless_handlers) + dev->wireless_handlers = &cfg80211_wext_handler; +#endif + break; case NETDEV_REGISTER: /* * NB: cannot take rdev->mtx here because this may be @@ -666,8 +672,6 @@ static int cfg80211_netdev_notifier_call wdev->sme_state = CFG80211_SME_IDLE; mutex_unlock(&rdev->devlist_mtx); #ifdef CONFIG_WIRELESS_EXT - if (!dev->wireless_handlers) - dev->wireless_handlers = &cfg80211_wext_handler; wdev->wext.default_key = -1; wdev->wext.default_mgmt_key = -1; wdev->wext.connect.auth_type = NL80211_AUTHTYPE_AUTOMATIC;
The point we assign dev->wireless_handlers at is too late, we need to do that before netdev_register_kobject() gets called, so use the new NETDEV_PRE_INIT notifier. The result of adding wireless_handlers too late is the disappearance of /sys/class/net/wlan0/wireless which a bunch of distro scripts still require. Signed-off-by: Johannes Berg <johannes@sipsolutions.net> --- This should fix the regression Hugh reported (of course requires the other patch which unfortunately I forgot to CC you, Hugh, I'll send you a copy in private). net/wireless/core.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html