From: Simon Wunderlich <simon.wunderlich@s2003.tu-chemnitz.de>
To: ath9k-devel@lists.ath9k.org
Subject: [ath9k-devel] [PATCH] ath9k: Disable spectral scan code to fix crash on rmmod.
Date: Thu, 9 May 2013 20:18:27 +0200 [thread overview]
Message-ID: <20130509181827.GA22871@pandem0nium> (raw)
In-Reply-To: <20875.13834.615920.337442@gargle.gargle.HOWL>
On Thu, May 09, 2013 at 11:07:14AM +0530, Sujith Manoharan wrote:
> greearb at candelatech.com wrote:
> > From: Ben Greear <greearb@candelatech.com>
> >
> > With CONFIG_ATH9K_DEBUGFS enabled, and slub memory poisoning
> > enabled, I see this crash on rmmod of ath9k. I'm not sure how
> > to fix this properly, but in the meantime, this patch to disable
> > the spectral scan code works around the problem for me.
> >
> > With memory poisoning and the verify_mem_not_deleted code
> > below added, the crash looks as follows... The dentry
> > is not *always* freed at this point, probably because rcu
> > callbacks haven't completed. You still get a crash soon
> > after, however.
>
> The relay file should probably be closed before calling ieee80211_unregister_hw().
> The ath9k debugfs directory is created inside the phy#/ directory and that would
> get cleaned up when the wiphy is unregistered.
>
> Does this help ?
Looks good to me, moving the closing of the relayfs file before the unregistering. Thanks
a lot for fixing my bugs, Sujith. :)
Acked-by: Simon Wunderlich <siwu@hrz.tu-chemnitz.de>
(BTW, would you mind sending this as proper [PATCH] again?)
>
> diff --git a/drivers/net/wireless/ath/ath9k/debug.c b/drivers/net/wireless/ath/ath9k/debug.c
> index 4101c4a..cecbe1f 100644
> --- a/drivers/net/wireless/ath/ath9k/debug.c
> +++ b/drivers/net/wireless/ath/ath9k/debug.c
> @@ -1684,6 +1684,14 @@ void ath9k_get_et_stats(struct ieee80211_hw *hw,
> WARN_ON(i != ATH9K_SSTATS_LEN);
> }
>
> +void ath9k_deinit_debug(struct ath_softc *sc)
> +{
> + if (config_enabled(CONFIG_ATH9K_DEBUGFS) && sc->rfs_chan_spec_scan) {
> + relay_close(sc->rfs_chan_spec_scan);
> + sc->rfs_chan_spec_scan = NULL;
> + }
> +}
> +
> int ath9k_init_debug(struct ath_hw *ah)
> {
> struct ath_common *common = ath9k_hw_common(ah);
> diff --git a/drivers/net/wireless/ath/ath9k/debug.h b/drivers/net/wireless/ath/ath9k/debug.h
> index 62da19c..223418d 100644
> --- a/drivers/net/wireless/ath/ath9k/debug.h
> +++ b/drivers/net/wireless/ath/ath9k/debug.h
> @@ -297,6 +297,7 @@ struct ath9k_debug {
> };
>
> int ath9k_init_debug(struct ath_hw *ah);
> +void ath9k_deinit_debug(struct ath_softc *sc);
>
> void ath_debug_stat_interrupt(struct ath_softc *sc, enum ath9k_int status);
> void ath_debug_stat_tx(struct ath_softc *sc, struct ath_buf *bf,
> @@ -332,6 +333,10 @@ static inline int ath9k_init_debug(struct ath_hw *ah)
> return 0;
> }
>
> +static inline void ath9k_deinit_debug(struct ath_softc *sc)
> +{
> +}
> +
> static inline void ath_debug_stat_interrupt(struct ath_softc *sc,
> enum ath9k_int status)
> {
> diff --git a/drivers/net/wireless/ath/ath9k/init.c b/drivers/net/wireless/ath/ath9k/init.c
> index c7b888f..c0aa4ff 100644
> --- a/drivers/net/wireless/ath/ath9k/init.c
> +++ b/drivers/net/wireless/ath/ath9k/init.c
> @@ -903,7 +903,7 @@ int ath9k_init_device(u16 devid, struct ath_softc *sc,
> if (!ath_is_world_regd(reg)) {
> error = regulatory_hint(hw->wiphy, reg->alpha2);
> if (error)
> - goto unregister;
> + goto debug_cleanup;
> }
>
> ath_init_leds(sc);
> @@ -911,6 +911,8 @@ int ath9k_init_device(u16 devid, struct ath_softc *sc,
>
> return 0;
>
> +debug_cleanup:
> + ath9k_deinit_debug(sc);
> unregister:
> ieee80211_unregister_hw(hw);
> rx_cleanup:
> @@ -939,11 +941,6 @@ static void ath9k_deinit_softc(struct ath_softc *sc)
> sc->dfs_detector->exit(sc->dfs_detector);
>
> ath9k_eeprom_release(sc);
> -
> - if (config_enabled(CONFIG_ATH9K_DEBUGFS) && sc->rfs_chan_spec_scan) {
> - relay_close(sc->rfs_chan_spec_scan);
> - sc->rfs_chan_spec_scan = NULL;
> - }
> }
>
> void ath9k_deinit_device(struct ath_softc *sc)
> @@ -957,6 +954,7 @@ void ath9k_deinit_device(struct ath_softc *sc)
>
> ath9k_ps_restore(sc);
>
> + ath9k_deinit_debug(sc);
> ieee80211_unregister_hw(hw);
> ath_rx_cleanup(sc);
> ath9k_deinit_softc(sc);
> --
> To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
Url : http://lists.ath9k.org/pipermail/ath9k-devel/attachments/20130509/7f3c119f/attachment.pgp
WARNING: multiple messages have this Message-ID (diff)
From: Simon Wunderlich <simon.wunderlich@s2003.tu-chemnitz.de>
To: Sujith Manoharan <sujith@msujith.org>
Cc: greearb@candelatech.com, linux-wireless@vger.kernel.org,
ath9k-devel@venema.h4ckr.net
Subject: Re: [PATCH] ath9k: Disable spectral scan code to fix crash on rmmod.
Date: Thu, 9 May 2013 20:18:27 +0200 [thread overview]
Message-ID: <20130509181827.GA22871@pandem0nium> (raw)
In-Reply-To: <20875.13834.615920.337442@gargle.gargle.HOWL>
[-- Attachment #1: Type: text/plain, Size: 4245 bytes --]
On Thu, May 09, 2013 at 11:07:14AM +0530, Sujith Manoharan wrote:
> greearb@candelatech.com wrote:
> > From: Ben Greear <greearb@candelatech.com>
> >
> > With CONFIG_ATH9K_DEBUGFS enabled, and slub memory poisoning
> > enabled, I see this crash on rmmod of ath9k. I'm not sure how
> > to fix this properly, but in the meantime, this patch to disable
> > the spectral scan code works around the problem for me.
> >
> > With memory poisoning and the verify_mem_not_deleted code
> > below added, the crash looks as follows... The dentry
> > is not *always* freed at this point, probably because rcu
> > callbacks haven't completed. You still get a crash soon
> > after, however.
>
> The relay file should probably be closed before calling ieee80211_unregister_hw().
> The ath9k debugfs directory is created inside the phy#/ directory and that would
> get cleaned up when the wiphy is unregistered.
>
> Does this help ?
Looks good to me, moving the closing of the relayfs file before the unregistering. Thanks
a lot for fixing my bugs, Sujith. :)
Acked-by: Simon Wunderlich <siwu@hrz.tu-chemnitz.de>
(BTW, would you mind sending this as proper [PATCH] again?)
>
> diff --git a/drivers/net/wireless/ath/ath9k/debug.c b/drivers/net/wireless/ath/ath9k/debug.c
> index 4101c4a..cecbe1f 100644
> --- a/drivers/net/wireless/ath/ath9k/debug.c
> +++ b/drivers/net/wireless/ath/ath9k/debug.c
> @@ -1684,6 +1684,14 @@ void ath9k_get_et_stats(struct ieee80211_hw *hw,
> WARN_ON(i != ATH9K_SSTATS_LEN);
> }
>
> +void ath9k_deinit_debug(struct ath_softc *sc)
> +{
> + if (config_enabled(CONFIG_ATH9K_DEBUGFS) && sc->rfs_chan_spec_scan) {
> + relay_close(sc->rfs_chan_spec_scan);
> + sc->rfs_chan_spec_scan = NULL;
> + }
> +}
> +
> int ath9k_init_debug(struct ath_hw *ah)
> {
> struct ath_common *common = ath9k_hw_common(ah);
> diff --git a/drivers/net/wireless/ath/ath9k/debug.h b/drivers/net/wireless/ath/ath9k/debug.h
> index 62da19c..223418d 100644
> --- a/drivers/net/wireless/ath/ath9k/debug.h
> +++ b/drivers/net/wireless/ath/ath9k/debug.h
> @@ -297,6 +297,7 @@ struct ath9k_debug {
> };
>
> int ath9k_init_debug(struct ath_hw *ah);
> +void ath9k_deinit_debug(struct ath_softc *sc);
>
> void ath_debug_stat_interrupt(struct ath_softc *sc, enum ath9k_int status);
> void ath_debug_stat_tx(struct ath_softc *sc, struct ath_buf *bf,
> @@ -332,6 +333,10 @@ static inline int ath9k_init_debug(struct ath_hw *ah)
> return 0;
> }
>
> +static inline void ath9k_deinit_debug(struct ath_softc *sc)
> +{
> +}
> +
> static inline void ath_debug_stat_interrupt(struct ath_softc *sc,
> enum ath9k_int status)
> {
> diff --git a/drivers/net/wireless/ath/ath9k/init.c b/drivers/net/wireless/ath/ath9k/init.c
> index c7b888f..c0aa4ff 100644
> --- a/drivers/net/wireless/ath/ath9k/init.c
> +++ b/drivers/net/wireless/ath/ath9k/init.c
> @@ -903,7 +903,7 @@ int ath9k_init_device(u16 devid, struct ath_softc *sc,
> if (!ath_is_world_regd(reg)) {
> error = regulatory_hint(hw->wiphy, reg->alpha2);
> if (error)
> - goto unregister;
> + goto debug_cleanup;
> }
>
> ath_init_leds(sc);
> @@ -911,6 +911,8 @@ int ath9k_init_device(u16 devid, struct ath_softc *sc,
>
> return 0;
>
> +debug_cleanup:
> + ath9k_deinit_debug(sc);
> unregister:
> ieee80211_unregister_hw(hw);
> rx_cleanup:
> @@ -939,11 +941,6 @@ static void ath9k_deinit_softc(struct ath_softc *sc)
> sc->dfs_detector->exit(sc->dfs_detector);
>
> ath9k_eeprom_release(sc);
> -
> - if (config_enabled(CONFIG_ATH9K_DEBUGFS) && sc->rfs_chan_spec_scan) {
> - relay_close(sc->rfs_chan_spec_scan);
> - sc->rfs_chan_spec_scan = NULL;
> - }
> }
>
> void ath9k_deinit_device(struct ath_softc *sc)
> @@ -957,6 +954,7 @@ void ath9k_deinit_device(struct ath_softc *sc)
>
> ath9k_ps_restore(sc);
>
> + ath9k_deinit_debug(sc);
> ieee80211_unregister_hw(hw);
> ath_rx_cleanup(sc);
> ath9k_deinit_softc(sc);
> --
> To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]
next prev parent reply other threads:[~2013-05-09 18:18 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-05-08 22:08 [ath9k-devel] [PATCH] ath9k: Disable spectral scan code to fix crash on rmmod greearb at candelatech.com
2013-05-08 22:08 ` greearb
2013-05-09 5:37 ` [ath9k-devel] " Sujith Manoharan
2013-05-09 5:37 ` Sujith Manoharan
2013-05-09 17:40 ` [ath9k-devel] " Ben Greear
2013-05-09 17:40 ` Ben Greear
2013-05-09 18:18 ` Simon Wunderlich [this message]
2013-05-09 18:18 ` Simon Wunderlich
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20130509181827.GA22871@pandem0nium \
--to=simon.wunderlich@s2003.tu-chemnitz.de \
--cc=ath9k-devel@lists.ath9k.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.