| Message ID | 20328.16366.166909.255035@gargle.gargle.HOWL |
|---|---|
| State | Not Applicable, archived |
| Delegated to: | David Miller |
| Headers | show |
Hi Sujith/Justin, On Tue, Mar 20, 2012 at 1:59 PM, Sujith Manoharan <c_manoha@qca.qualcomm.com> wrote: > Justin P. Mattock wrote: >> yeah this works: >> >> eading symbols from >> /home/kernel/linux-next/drivers/net/wireless/ath/ath9k/ath9k.o...done. >> (gdb) l *(ath_tx_start+0x284) >> 0xcad4 is in ath_tx_start (drivers/net/wireless/ath/ath9k/xmit.c:1878). >> 1873 ieee80211_is_data_qos(hdr->frame_control)) { >> 1874 tidno = ieee80211_get_qos_ctl(hdr)[0] & >> 1875 IEEE80211_QOS_CTL_TID_MASK; >> 1876 tid = ATH_AN_2_TID(txctl->an, tidno); >> 1877 >> 1878 WARN_ON(tid->ac->txq != txctl->txq); >> 1879 } >> 1880 >> 1881 if ((tx_info->flags & IEEE80211_TX_CTL_AMPDU) && tid) { >> 1882 /* >> (gdb) > > Can you try this patch ? just found out that 'ht_supported' may not be set, if assoc response does not has ht_cap IE (or) if we could not parse it (why), then the driver won't initialize those tid related structures ath_tx_node_init, while we later access them in ath_tx_start. so this should fix the issue. > > From: Sujith Manoharan <c_manoha@qca.qualcomm.com> > Date: Tue, 20 Mar 2012 13:51:26 +0530 > Subject: [PATCH] ath9k: Use HW HT capabilites properly > > The commit "ath9k: Remove aggregation flags" changed how > nodes were being initialized. Use the HW HT cap bits > to initialize/de-initialize nodes, else we would be > accessing an uninitialized entry during a suspend/resume cycle, > resulting in a panic. > > Reported-by: Justin P. Mattock <justinmattock@gmail.com> > Signed-off-by: Sujith Manoharan <c_manoha@qca.qualcomm.com> > --- > drivers/net/wireless/ath/ath9k/main.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/net/wireless/ath/ath9k/main.c b/drivers/net/wireless/ath/ath9k/main.c > index 3879485..215eb25 100644 > --- a/drivers/net/wireless/ath/ath9k/main.c > +++ b/drivers/net/wireless/ath/ath9k/main.c > @@ -640,7 +640,7 @@ static void ath_node_attach(struct ath_softc *sc, struct ieee80211_sta *sta, > an->sta = sta; > an->vif = vif; > > - if (sta->ht_cap.ht_supported) { > + if (sc->sc_ah->caps.hw_caps & ATH9K_HW_CAP_HT) { > ath_tx_node_init(sc, an); > an->maxampdu = 1 << (IEEE80211_HT_MAX_AMPDU_FACTOR + > sta->ht_cap.ampdu_factor); > @@ -659,7 +659,7 @@ static void ath_node_detach(struct ath_softc *sc, struct ieee80211_sta *sta) > an->sta = NULL; > #endif > > - if (sta->ht_cap.ht_supported) > + if (sc->sc_ah->caps.hw_caps & ATH9K_HW_CAP_HT) > ath_tx_node_cleanup(sc, an); > } > > -- > 1.7.9.4 >
On Tue, Mar 20, 2012 at 3:07 PM, Mohammed Shafi <shafi.wireless@gmail.com> wrote: > Hi Sujith/Justin, > > On Tue, Mar 20, 2012 at 1:59 PM, Sujith Manoharan > <c_manoha@qca.qualcomm.com> wrote: >> Justin P. Mattock wrote: >>> yeah this works: >>> >>> eading symbols from >>> /home/kernel/linux-next/drivers/net/wireless/ath/ath9k/ath9k.o...done. >>> (gdb) l *(ath_tx_start+0x284) >>> 0xcad4 is in ath_tx_start (drivers/net/wireless/ath/ath9k/xmit.c:1878). >>> 1873 ieee80211_is_data_qos(hdr->frame_control)) { >>> 1874 tidno = ieee80211_get_qos_ctl(hdr)[0] & >>> 1875 IEEE80211_QOS_CTL_TID_MASK; >>> 1876 tid = ATH_AN_2_TID(txctl->an, tidno); >>> 1877 >>> 1878 WARN_ON(tid->ac->txq != txctl->txq); >>> 1879 } >>> 1880 >>> 1881 if ((tx_info->flags & IEEE80211_TX_CTL_AMPDU) && tid) { >>> 1882 /* >>> (gdb) >> >> Can you try this patch ? > > just found out that 'ht_supported' may not be set, if assoc response > does not has ht_cap IE (or) if we could not parse it (why), then the > driver won't initialize those tid related structures ath_tx_node_init, > while we later access them in ath_tx_start. so this should fix the > issue. now i was able to quite easily recreate this issue in wireless testing even without suspend resume and with the patch the issue seems to get fixed. i was using 2ghz channel 6 and TKIP (no HT!). > >> >> From: Sujith Manoharan <c_manoha@qca.qualcomm.com> >> Date: Tue, 20 Mar 2012 13:51:26 +0530 >> Subject: [PATCH] ath9k: Use HW HT capabilites properly >> >> The commit "ath9k: Remove aggregation flags" changed how >> nodes were being initialized. Use the HW HT cap bits >> to initialize/de-initialize nodes, else we would be >> accessing an uninitialized entry during a suspend/resume cycle, >> resulting in a panic. >> >> Reported-by: Justin P. Mattock <justinmattock@gmail.com> >> Signed-off-by: Sujith Manoharan <c_manoha@qca.qualcomm.com> >> --- >> drivers/net/wireless/ath/ath9k/main.c | 4 ++-- >> 1 file changed, 2 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/net/wireless/ath/ath9k/main.c b/drivers/net/wireless/ath/ath9k/main.c >> index 3879485..215eb25 100644 >> --- a/drivers/net/wireless/ath/ath9k/main.c >> +++ b/drivers/net/wireless/ath/ath9k/main.c >> @@ -640,7 +640,7 @@ static void ath_node_attach(struct ath_softc *sc, struct ieee80211_sta *sta, >> an->sta = sta; >> an->vif = vif; >> >> - if (sta->ht_cap.ht_supported) { >> + if (sc->sc_ah->caps.hw_caps & ATH9K_HW_CAP_HT) { >> ath_tx_node_init(sc, an); >> an->maxampdu = 1 << (IEEE80211_HT_MAX_AMPDU_FACTOR + >> sta->ht_cap.ampdu_factor); >> @@ -659,7 +659,7 @@ static void ath_node_detach(struct ath_softc *sc, struct ieee80211_sta *sta) >> an->sta = NULL; >> #endif >> >> - if (sta->ht_cap.ht_supported) >> + if (sc->sc_ah->caps.hw_caps & ATH9K_HW_CAP_HT) >> ath_tx_node_cleanup(sc, an); >> } >> >> -- >> 1.7.9.4 >> > > > > -- > thanks, > shafi
On Tue, Mar 20, 2012 at 4:45 PM, Mohammed Shafi <shafi.wireless@gmail.com> wrote: > On Tue, Mar 20, 2012 at 3:07 PM, Mohammed Shafi > <shafi.wireless@gmail.com> wrote: >> Hi Sujith/Justin, >> >> On Tue, Mar 20, 2012 at 1:59 PM, Sujith Manoharan >> <c_manoha@qca.qualcomm.com> wrote: >>> Justin P. Mattock wrote: >>>> yeah this works: >>>> >>>> eading symbols from >>>> /home/kernel/linux-next/drivers/net/wireless/ath/ath9k/ath9k.o...done. >>>> (gdb) l *(ath_tx_start+0x284) >>>> 0xcad4 is in ath_tx_start (drivers/net/wireless/ath/ath9k/xmit.c:1878). >>>> 1873 ieee80211_is_data_qos(hdr->frame_control)) { >>>> 1874 tidno = ieee80211_get_qos_ctl(hdr)[0] & >>>> 1875 IEEE80211_QOS_CTL_TID_MASK; >>>> 1876 tid = ATH_AN_2_TID(txctl->an, tidno); >>>> 1877 >>>> 1878 WARN_ON(tid->ac->txq != txctl->txq); >>>> 1879 } >>>> 1880 >>>> 1881 if ((tx_info->flags & IEEE80211_TX_CTL_AMPDU) && tid) { >>>> 1882 /* >>>> (gdb) >>> >>> Can you try this patch ? >> >> just found out that 'ht_supported' may not be set, if assoc response >> does not has ht_cap IE (or) if we could not parse it (why), then the >> driver won't initialize those tid related structures ath_tx_node_init, >> while we later access them in ath_tx_start. so this should fix the >> issue. > > now i was able to quite easily recreate this issue in wireless testing > even without suspend resume and with the patch the issue seems to get > fixed. > i was using 2ghz channel 6 and TKIP (no HT!). i was keep on trying with 5ghz TKIP and it seems HT enable even with TKIP (need to see why this happens), thats why even i could not recreate this issue. mac80211 seems to disable HT, ht_supported false while in driver we go with ATH9K_HW_CAP HT > > >> >>> >>> From: Sujith Manoharan <c_manoha@qca.qualcomm.com> >>> Date: Tue, 20 Mar 2012 13:51:26 +0530 >>> Subject: [PATCH] ath9k: Use HW HT capabilites properly >>> >>> The commit "ath9k: Remove aggregation flags" changed how >>> nodes were being initialized. Use the HW HT cap bits >>> to initialize/de-initialize nodes, else we would be >>> accessing an uninitialized entry during a suspend/resume cycle, >>> resulting in a panic. >>> >>> Reported-by: Justin P. Mattock <justinmattock@gmail.com> >>> Signed-off-by: Sujith Manoharan <c_manoha@qca.qualcomm.com> >>> --- >>> drivers/net/wireless/ath/ath9k/main.c | 4 ++-- >>> 1 file changed, 2 insertions(+), 2 deletions(-) >>> >>> diff --git a/drivers/net/wireless/ath/ath9k/main.c b/drivers/net/wireless/ath/ath9k/main.c >>> index 3879485..215eb25 100644 >>> --- a/drivers/net/wireless/ath/ath9k/main.c >>> +++ b/drivers/net/wireless/ath/ath9k/main.c >>> @@ -640,7 +640,7 @@ static void ath_node_attach(struct ath_softc *sc, struct ieee80211_sta *sta, >>> an->sta = sta; >>> an->vif = vif; >>> >>> - if (sta->ht_cap.ht_supported) { >>> + if (sc->sc_ah->caps.hw_caps & ATH9K_HW_CAP_HT) { >>> ath_tx_node_init(sc, an); >>> an->maxampdu = 1 << (IEEE80211_HT_MAX_AMPDU_FACTOR + >>> sta->ht_cap.ampdu_factor); >>> @@ -659,7 +659,7 @@ static void ath_node_detach(struct ath_softc *sc, struct ieee80211_sta *sta) >>> an->sta = NULL; >>> #endif >>> >>> - if (sta->ht_cap.ht_supported) >>> + if (sc->sc_ah->caps.hw_caps & ATH9K_HW_CAP_HT) >>> ath_tx_node_cleanup(sc, an); >>> } >>> >>> -- >>> 1.7.9.4 >>> >> >> >> >> -- >> thanks, >> shafi > > > > -- > thanks, > shafi
this should also fix https://bugzilla.kernel.org/show_bug.cgi?id=42903
On 03/20/2012 01:29 AM, Sujith Manoharan wrote: > Justin P. Mattock wrote: >> yeah this works: >> >> eading symbols from >> /home/kernel/linux-next/drivers/net/wireless/ath/ath9k/ath9k.o...done. >> (gdb) l *(ath_tx_start+0x284) >> 0xcad4 is in ath_tx_start (drivers/net/wireless/ath/ath9k/xmit.c:1878). >> 1873 ieee80211_is_data_qos(hdr->frame_control)) { >> 1874 tidno = ieee80211_get_qos_ctl(hdr)[0]& >> 1875 IEEE80211_QOS_CTL_TID_MASK; >> 1876 tid = ATH_AN_2_TID(txctl->an, tidno); >> 1877 >> 1878 WARN_ON(tid->ac->txq != txctl->txq); >> 1879 } >> 1880 >> 1881 if ((tx_info->flags& IEEE80211_TX_CTL_AMPDU)&& tid) { >> 1882 /* >> (gdb) > > Can you try this patch ? > > From: Sujith Manoharan<c_manoha@qca.qualcomm.com> > Date: Tue, 20 Mar 2012 13:51:26 +0530 > Subject: [PATCH] ath9k: Use HW HT capabilites properly > > The commit "ath9k: Remove aggregation flags" changed how > nodes were being initialized. Use the HW HT cap bits > to initialize/de-initialize nodes, else we would be > accessing an uninitialized entry during a suspend/resume cycle, > resulting in a panic. > > Reported-by: Justin P. Mattock<justinmattock@gmail.com> > Signed-off-by: Sujith Manoharan<c_manoha@qca.qualcomm.com> > --- > drivers/net/wireless/ath/ath9k/main.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/net/wireless/ath/ath9k/main.c b/drivers/net/wireless/ath/ath9k/main.c > index 3879485..215eb25 100644 > --- a/drivers/net/wireless/ath/ath9k/main.c > +++ b/drivers/net/wireless/ath/ath9k/main.c > @@ -640,7 +640,7 @@ static void ath_node_attach(struct ath_softc *sc, struct ieee80211_sta *sta, > an->sta = sta; > an->vif = vif; > > - if (sta->ht_cap.ht_supported) { > + if (sc->sc_ah->caps.hw_caps& ATH9K_HW_CAP_HT) { > ath_tx_node_init(sc, an); > an->maxampdu = 1<< (IEEE80211_HT_MAX_AMPDU_FACTOR + > sta->ht_cap.ampdu_factor); > @@ -659,7 +659,7 @@ static void ath_node_detach(struct ath_softc *sc, struct ieee80211_sta *sta) > an->sta = NULL; > #endif > > - if (sta->ht_cap.ht_supported) > + if (sc->sc_ah->caps.hw_caps& ATH9K_HW_CAP_HT) > ath_tx_node_cleanup(sc, an); > } > alright! both mohammed and sujith Thank you for the patches for this issue! I really appreciate this.. I went and applied the debug printk and added ath_node_attach as well to my linux-next clone, will run for a few days to see if I get anything.. full dmesg is here..: http://fpaste.org/jHSX/ Justin P. Mattock -- 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
Justin P. Mattock wrote: > both mohammed and sujith Thank you for the patches for this issue! > I really appreciate this.. > > I went and applied the debug printk and added ath_node_attach as well to > my linux-next clone, will run for a few days to see if I get anything.. > full dmesg is here..: http://fpaste.org/jHSX/ Thanks for testing and please report if the issue is fixed. Sujith -- 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 03/20/2012 08:22 AM, Sujith Manoharan wrote: > Justin P. Mattock wrote: >> both mohammed and sujith Thank you for the patches for this issue! >> I really appreciate this.. >> >> I went and applied the debug printk and added ath_node_attach as well to >> my linux-next clone, will run for a few days to see if I get anything.. >> full dmesg is here..: http://fpaste.org/jHSX/ > > Thanks for testing and please report if the issue is fixed. > > Sujith > no worries.. hopefully we can get this bug fixed.. will continue running these patches and will send out anything I see.. Justin P. Mattock -- 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 03/20/2012 01:29 AM, Sujith Manoharan wrote: > Justin P. Mattock wrote: >> yeah this works: >> >> eading symbols from >> /home/kernel/linux-next/drivers/net/wireless/ath/ath9k/ath9k.o...done. >> (gdb) l *(ath_tx_start+0x284) >> 0xcad4 is in ath_tx_start (drivers/net/wireless/ath/ath9k/xmit.c:1878). >> 1873 ieee80211_is_data_qos(hdr->frame_control)) { >> 1874 tidno = ieee80211_get_qos_ctl(hdr)[0]& >> 1875 IEEE80211_QOS_CTL_TID_MASK; >> 1876 tid = ATH_AN_2_TID(txctl->an, tidno); >> 1877 >> 1878 WARN_ON(tid->ac->txq != txctl->txq); >> 1879 } >> 1880 >> 1881 if ((tx_info->flags& IEEE80211_TX_CTL_AMPDU)&& tid) { >> 1882 /* >> (gdb) > > Can you try this patch ? > > From: Sujith Manoharan<c_manoha@qca.qualcomm.com> > Date: Tue, 20 Mar 2012 13:51:26 +0530 > Subject: [PATCH] ath9k: Use HW HT capabilites properly > > The commit "ath9k: Remove aggregation flags" changed how > nodes were being initialized. Use the HW HT cap bits > to initialize/de-initialize nodes, else we would be > accessing an uninitialized entry during a suspend/resume cycle, > resulting in a panic. > > Reported-by: Justin P. Mattock<justinmattock@gmail.com> > Signed-off-by: Sujith Manoharan<c_manoha@qca.qualcomm.com> > --- > drivers/net/wireless/ath/ath9k/main.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/net/wireless/ath/ath9k/main.c b/drivers/net/wireless/ath/ath9k/main.c > index 3879485..215eb25 100644 > --- a/drivers/net/wireless/ath/ath9k/main.c > +++ b/drivers/net/wireless/ath/ath9k/main.c > @@ -640,7 +640,7 @@ static void ath_node_attach(struct ath_softc *sc, struct ieee80211_sta *sta, > an->sta = sta; > an->vif = vif; > > - if (sta->ht_cap.ht_supported) { > + if (sc->sc_ah->caps.hw_caps& ATH9K_HW_CAP_HT) { > ath_tx_node_init(sc, an); > an->maxampdu = 1<< (IEEE80211_HT_MAX_AMPDU_FACTOR + > sta->ht_cap.ampdu_factor); > @@ -659,7 +659,7 @@ static void ath_node_detach(struct ath_softc *sc, struct ieee80211_sta *sta) > an->sta = NULL; > #endif > > - if (sta->ht_cap.ht_supported) > + if (sc->sc_ah->caps.hw_caps& ATH9K_HW_CAP_HT) > ath_tx_node_cleanup(sc, an); > } > I would have to say this patch above does get rid of this crash I was seeing. as a quick test I simply connect to a WPA network, then connect to an open network going back and forth triggers this freeze for me after applying this I am able to toggle back and forth without a freeze. Thanks! Justin P. Mattock -- 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
Justin P. Mattock wrote: > I would have to say this patch above does get rid of this crash I was > seeing. as a quick test I simply connect to a WPA network, then connect > to an open network going back and forth triggers this freeze for me > after applying this I am able to toggle back and forth without a freeze. Thanks for verifying. Sujith -- 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
diff --git a/drivers/net/wireless/ath/ath9k/main.c b/drivers/net/wireless/ath/ath9k/main.c index 3879485..215eb25 100644 --- a/drivers/net/wireless/ath/ath9k/main.c +++ b/drivers/net/wireless/ath/ath9k/main.c @@ -640,7 +640,7 @@ static void ath_node_attach(struct ath_softc *sc, struct ieee80211_sta *sta, an->sta = sta; an->vif = vif; - if (sta->ht_cap.ht_supported) { + if (sc->sc_ah->caps.hw_caps & ATH9K_HW_CAP_HT) { ath_tx_node_init(sc, an); an->maxampdu = 1 << (IEEE80211_HT_MAX_AMPDU_FACTOR + sta->ht_cap.ampdu_factor); @@ -659,7 +659,7 @@ static void ath_node_detach(struct ath_softc *sc, struct ieee80211_sta *sta) an->sta = NULL; #endif - if (sta->ht_cap.ht_supported) + if (sc->sc_ah->caps.hw_caps & ATH9K_HW_CAP_HT) ath_tx_node_cleanup(sc, an); }