* [PATCH 1/3] WDEV: ath5k, fix lock imbalance
@ 2008-02-15 20:58 Jiri Slaby
2008-02-15 20:58 ` [PATCH 2/3] WDEV, ath5k, don't return int from bool function Jiri Slaby
` (2 more replies)
0 siblings, 3 replies; 10+ messages in thread
From: Jiri Slaby @ 2008-02-15 20:58 UTC (permalink / raw)
To: linville
Cc: linux-kernel, linux-wireless, ath5k-devel, Jiri Slaby,
Nick Kossifidis, Luis R. Rodriguez
Omitted lock causes sparse warning
drivers/net/wireless/ath5k/base.c:1682:1: warning: context imbalance in 'ath5k_tasklet_rx' - different lock contexts for basic block
Add the lock to the guilty fail path.
Signed-off-by: Jiri Slaby <jirislaby@gmail.com>
Cc: Nick Kossifidis <mickflemm@gmail.com>
Cc: Luis R. Rodriguez <mcgrof@gmail.com>
---
drivers/net/wireless/ath5k/base.c | 1 +
1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/drivers/net/wireless/ath5k/base.c b/drivers/net/wireless/ath5k/base.c
index ddc8714..49d38e8 100644
--- a/drivers/net/wireless/ath5k/base.c
+++ b/drivers/net/wireless/ath5k/base.c
@@ -1715,6 +1715,7 @@ ath5k_tasklet_rx(unsigned long data)
break;
else if (unlikely(ret)) {
ATH5K_ERR(sc, "error in processing rx descriptor\n");
+ spin_unlock(&sc->rxbuflock);
return;
}
--
1.5.3.8
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH 2/3] WDEV, ath5k, don't return int from bool function 2008-02-15 20:58 [PATCH 1/3] WDEV: ath5k, fix lock imbalance Jiri Slaby @ 2008-02-15 20:58 ` Jiri Slaby 2008-02-15 22:03 ` Nick Kossifidis 2008-02-15 22:08 ` Luis R. Rodriguez 2008-02-15 20:58 ` [PATCH 3/3] WDEV: ath5k, typecheck on nonDEBUG Jiri Slaby 2008-02-15 22:00 ` [PATCH 1/3] WDEV: ath5k, fix lock imbalance Nick Kossifidis 2 siblings, 2 replies; 10+ messages in thread From: Jiri Slaby @ 2008-02-15 20:58 UTC (permalink / raw) To: linville Cc: linux-kernel, linux-wireless, ath5k-devel, Jiri Slaby, Nick Kossifidis, Luis R. Rodriguez sparse sees int -> bool cast as an error: hw.c:3754:10: warning: cast truncates bits from constant value (ffffffea becomes 0) Fix it by converting the rettype to int and check appropriately. Signed-off-by: Jiri Slaby <jirislaby@gmail.com> Cc: Nick Kossifidis <mickflemm@gmail.com> Cc: Luis R. Rodriguez <mcgrof@gmail.com> --- drivers/net/wireless/ath5k/ath5k.h | 2 +- drivers/net/wireless/ath5k/base.c | 5 ++++- drivers/net/wireless/ath5k/hw.c | 8 ++++---- 3 files changed, 9 insertions(+), 6 deletions(-) diff --git a/drivers/net/wireless/ath5k/ath5k.h b/drivers/net/wireless/ath5k/ath5k.h index c79066b..69dea33 100644 --- a/drivers/net/wireless/ath5k/ath5k.h +++ b/drivers/net/wireless/ath5k/ath5k.h @@ -1035,7 +1035,7 @@ struct ath5k_hw { unsigned int, unsigned int, enum ath5k_pkt_type, unsigned int, unsigned int, unsigned int, unsigned int, unsigned int, unsigned int, unsigned int, unsigned int); - bool (*ah_setup_xtx_desc)(struct ath5k_hw *, struct ath5k_desc *, + int (*ah_setup_xtx_desc)(struct ath5k_hw *, struct ath5k_desc *, unsigned int, unsigned int, unsigned int, unsigned int, unsigned int, unsigned int); int (*ah_proc_tx_desc)(struct ath5k_hw *, struct ath5k_desc *); diff --git a/drivers/net/wireless/ath5k/base.c b/drivers/net/wireless/ath5k/base.c index 49d38e8..59e5d56 100644 --- a/drivers/net/wireless/ath5k/base.c +++ b/drivers/net/wireless/ath5k/base.c @@ -668,7 +668,10 @@ ath5k_attach(struct pci_dev *pdev, struct ieee80211_hw *hw) * return false w/o doing anything. MAC's that do * support it will return true w/o doing anything. */ - if (ah->ah_setup_xtx_desc(ah, NULL, 0, 0, 0, 0, 0, 0)) + ret = ah->ah_setup_xtx_desc(ah, NULL, 0, 0, 0, 0, 0, 0); + if (ret < 0) + goto err; + if (ret > 0) __set_bit(ATH_STAT_MRRETRY, sc->status); /* diff --git a/drivers/net/wireless/ath5k/hw.c b/drivers/net/wireless/ath5k/hw.c index 93a75f2..463413a 100644 --- a/drivers/net/wireless/ath5k/hw.c +++ b/drivers/net/wireless/ath5k/hw.c @@ -45,7 +45,7 @@ static int ath5k_hw_setup_4word_tx_desc(struct ath5k_hw *, struct ath5k_desc *, unsigned int, unsigned int, enum ath5k_pkt_type, unsigned int, unsigned int, unsigned int, unsigned int, unsigned int, unsigned int, unsigned int, unsigned int); -static bool ath5k_hw_setup_xr_tx_desc(struct ath5k_hw *, struct ath5k_desc *, +static int ath5k_hw_setup_xr_tx_desc(struct ath5k_hw *, struct ath5k_desc *, unsigned int, unsigned int, unsigned int, unsigned int, unsigned int, unsigned int); static int ath5k_hw_proc_4word_tx_status(struct ath5k_hw *, struct ath5k_desc *); @@ -3733,7 +3733,7 @@ static int ath5k_hw_setup_4word_tx_desc(struct ath5k_hw *ah, /* * Initialize a 4-word multirate tx descriptor on 5212 */ -static bool +static int ath5k_hw_setup_xr_tx_desc(struct ath5k_hw *ah, struct ath5k_desc *desc, unsigned int tx_rate1, u_int tx_tries1, u_int tx_rate2, u_int tx_tries2, unsigned int tx_rate3, u_int tx_tries3) @@ -3773,10 +3773,10 @@ ath5k_hw_setup_xr_tx_desc(struct ath5k_hw *ah, struct ath5k_desc *desc, #undef _XTX_TRIES - return true; + return 1; } - return false; + return 0; } /* -- 1.5.3.8 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH 2/3] WDEV, ath5k, don't return int from bool function 2008-02-15 20:58 ` [PATCH 2/3] WDEV, ath5k, don't return int from bool function Jiri Slaby @ 2008-02-15 22:03 ` Nick Kossifidis 2008-02-15 22:08 ` Luis R. Rodriguez 1 sibling, 0 replies; 10+ messages in thread From: Nick Kossifidis @ 2008-02-15 22:03 UTC (permalink / raw) To: Jiri Slaby Cc: linville, linux-kernel, linux-wireless, ath5k-devel, Luis R. Rodriguez 2008/2/15, Jiri Slaby <jirislaby@gmail.com>: > sparse sees int -> bool cast as an error: > hw.c:3754:10: warning: cast truncates bits from constant value (ffffffea becomes 0) > Fix it by converting the rettype to int and check appropriately. > > Signed-off-by: Jiri Slaby <jirislaby@gmail.com> > Cc: Nick Kossifidis <mickflemm@gmail.com> > Cc: Luis R. Rodriguez <mcgrof@gmail.com> > --- > drivers/net/wireless/ath5k/ath5k.h | 2 +- > drivers/net/wireless/ath5k/base.c | 5 ++++- > drivers/net/wireless/ath5k/hw.c | 8 ++++---- > 3 files changed, 9 insertions(+), 6 deletions(-) > > diff --git a/drivers/net/wireless/ath5k/ath5k.h b/drivers/net/wireless/ath5k/ath5k.h > index c79066b..69dea33 100644 > --- a/drivers/net/wireless/ath5k/ath5k.h > +++ b/drivers/net/wireless/ath5k/ath5k.h > @@ -1035,7 +1035,7 @@ struct ath5k_hw { > unsigned int, unsigned int, enum ath5k_pkt_type, unsigned int, > unsigned int, unsigned int, unsigned int, unsigned int, > unsigned int, unsigned int, unsigned int); > - bool (*ah_setup_xtx_desc)(struct ath5k_hw *, struct ath5k_desc *, > + int (*ah_setup_xtx_desc)(struct ath5k_hw *, struct ath5k_desc *, > unsigned int, unsigned int, unsigned int, unsigned int, > unsigned int, unsigned int); > int (*ah_proc_tx_desc)(struct ath5k_hw *, struct ath5k_desc *); > diff --git a/drivers/net/wireless/ath5k/base.c b/drivers/net/wireless/ath5k/base.c > index 49d38e8..59e5d56 100644 > --- a/drivers/net/wireless/ath5k/base.c > +++ b/drivers/net/wireless/ath5k/base.c > @@ -668,7 +668,10 @@ ath5k_attach(struct pci_dev *pdev, struct ieee80211_hw *hw) > * return false w/o doing anything. MAC's that do > * support it will return true w/o doing anything. > */ > - if (ah->ah_setup_xtx_desc(ah, NULL, 0, 0, 0, 0, 0, 0)) > + ret = ah->ah_setup_xtx_desc(ah, NULL, 0, 0, 0, 0, 0, 0); > + if (ret < 0) > + goto err; > + if (ret > 0) > __set_bit(ATH_STAT_MRRETRY, sc->status); > > /* > diff --git a/drivers/net/wireless/ath5k/hw.c b/drivers/net/wireless/ath5k/hw.c > index 93a75f2..463413a 100644 > --- a/drivers/net/wireless/ath5k/hw.c > +++ b/drivers/net/wireless/ath5k/hw.c > @@ -45,7 +45,7 @@ static int ath5k_hw_setup_4word_tx_desc(struct ath5k_hw *, struct ath5k_desc *, > unsigned int, unsigned int, enum ath5k_pkt_type, unsigned int, > unsigned int, unsigned int, unsigned int, unsigned int, unsigned int, > unsigned int, unsigned int); > -static bool ath5k_hw_setup_xr_tx_desc(struct ath5k_hw *, struct ath5k_desc *, > +static int ath5k_hw_setup_xr_tx_desc(struct ath5k_hw *, struct ath5k_desc *, > unsigned int, unsigned int, unsigned int, unsigned int, unsigned int, > unsigned int); > static int ath5k_hw_proc_4word_tx_status(struct ath5k_hw *, struct ath5k_desc *); > @@ -3733,7 +3733,7 @@ static int ath5k_hw_setup_4word_tx_desc(struct ath5k_hw *ah, > /* > * Initialize a 4-word multirate tx descriptor on 5212 > */ > -static bool > +static int > ath5k_hw_setup_xr_tx_desc(struct ath5k_hw *ah, struct ath5k_desc *desc, > unsigned int tx_rate1, u_int tx_tries1, u_int tx_rate2, u_int tx_tries2, > unsigned int tx_rate3, u_int tx_tries3) > @@ -3773,10 +3773,10 @@ ath5k_hw_setup_xr_tx_desc(struct ath5k_hw *ah, struct ath5k_desc *desc, > > #undef _XTX_TRIES > > - return true; > + return 1; > } > > - return false; > + return 0; > } > > /* > Acked-by: Nick Kossifidis <mickflemm@gmail.com> -- GPG ID: 0xD21DB2DB As you read this post global entropy rises. Have Fun ;-) Nick ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/3] WDEV, ath5k, don't return int from bool function 2008-02-15 20:58 ` [PATCH 2/3] WDEV, ath5k, don't return int from bool function Jiri Slaby 2008-02-15 22:03 ` Nick Kossifidis @ 2008-02-15 22:08 ` Luis R. Rodriguez 2008-02-15 22:38 ` Jiri Slaby 1 sibling, 1 reply; 10+ messages in thread From: Luis R. Rodriguez @ 2008-02-15 22:08 UTC (permalink / raw) To: Jiri Slaby Cc: linville, linux-kernel, linux-wireless, ath5k-devel, Nick Kossifidis On Fri, Feb 15, 2008 at 3:58 PM, Jiri Slaby <jirislaby@gmail.com> wrote: > -static bool > +static int > ath5k_hw_setup_xr_tx_desc(struct ath5k_hw *ah, struct ath5k_desc *desc, > unsigned int tx_rate1, u_int tx_tries1, u_int tx_rate2, u_int tx_tries2, > unsigned int tx_rate3, u_int tx_tries3) > @@ -3773,10 +3773,10 @@ ath5k_hw_setup_xr_tx_desc(struct ath5k_hw *ah, struct ath5k_desc *desc, > > #undef _XTX_TRIES > > - return true; > + return 1; > } > > - return false; > + return 0; > } Shouldn't we then treat 0 as OK? Luis ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/3] WDEV, ath5k, don't return int from bool function 2008-02-15 22:08 ` Luis R. Rodriguez @ 2008-02-15 22:38 ` Jiri Slaby 2008-02-15 23:24 ` Luis R. Rodriguez 0 siblings, 1 reply; 10+ messages in thread From: Jiri Slaby @ 2008-02-15 22:38 UTC (permalink / raw) To: Luis R. Rodriguez Cc: linville, linux-kernel, linux-wireless, ath5k-devel, Nick Kossifidis On 02/15/2008 11:08 PM, Luis R. Rodriguez wrote: > On Fri, Feb 15, 2008 at 3:58 PM, Jiri Slaby <jirislaby@gmail.com> wrote: > >> -static bool >> +static int >> ath5k_hw_setup_xr_tx_desc(struct ath5k_hw *ah, struct ath5k_desc *desc, >> unsigned int tx_rate1, u_int tx_tries1, u_int tx_rate2, u_int tx_tries2, >> unsigned int tx_rate3, u_int tx_tries3) >> @@ -3773,10 +3773,10 @@ ath5k_hw_setup_xr_tx_desc(struct ath5k_hw *ah, struct ath5k_desc *desc, >> >> #undef _XTX_TRIES >> >> - return true; >> + return 1; >> } >> >> - return false; >> + return 0; >> } > > Shouldn't we then treat 0 as OK? Sorry, I don't understand you. There is return -EINVAL in the function above this too and we need to cope with another two states but the error: it is supported/it isn't. You mean to consider 0 as supported, -ENODEV/-EOPNOTSUPP as unsupported and the rest as error? ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/3] WDEV, ath5k, don't return int from bool function 2008-02-15 22:38 ` Jiri Slaby @ 2008-02-15 23:24 ` Luis R. Rodriguez 2008-02-15 23:29 ` Jiri Slaby 0 siblings, 1 reply; 10+ messages in thread From: Luis R. Rodriguez @ 2008-02-15 23:24 UTC (permalink / raw) To: Jiri Slaby Cc: linville, linux-kernel, linux-wireless, ath5k-devel, Nick Kossifidis On Fri, Feb 15, 2008 at 5:38 PM, Jiri Slaby <jirislaby@gmail.com> wrote: > > You mean to consider 0 as supported, -ENODEV/-EOPNOTSUPP as > unsupported and the rest as error? > Yeap, what do you think? Luis ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/3] WDEV, ath5k, don't return int from bool function 2008-02-15 23:24 ` Luis R. Rodriguez @ 2008-02-15 23:29 ` Jiri Slaby 0 siblings, 0 replies; 10+ messages in thread From: Jiri Slaby @ 2008-02-15 23:29 UTC (permalink / raw) To: Luis R. Rodriguez Cc: linville, linux-kernel, linux-wireless, ath5k-devel, Nick Kossifidis On 02/16/2008 12:24 AM, Luis R. Rodriguez wrote: > On Fri, Feb 15, 2008 at 5:38 PM, Jiri Slaby <jirislaby@gmail.com> wrote: >> You mean to consider 0 as supported, -ENODEV/-EOPNOTSUPP as >> unsupported and the rest as error? >> > > Yeap, what do you think? I have no opinion in this case, which one do you (all) like more? ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 3/3] WDEV: ath5k, typecheck on nonDEBUG 2008-02-15 20:58 [PATCH 1/3] WDEV: ath5k, fix lock imbalance Jiri Slaby 2008-02-15 20:58 ` [PATCH 2/3] WDEV, ath5k, don't return int from bool function Jiri Slaby @ 2008-02-15 20:58 ` Jiri Slaby 2008-02-15 22:32 ` Luis R. Rodriguez 2008-02-15 22:00 ` [PATCH 1/3] WDEV: ath5k, fix lock imbalance Nick Kossifidis 2 siblings, 1 reply; 10+ messages in thread From: Jiri Slaby @ 2008-02-15 20:58 UTC (permalink / raw) To: linville Cc: linux-kernel, linux-wireless, ath5k-devel, Jiri Slaby, Nick Kossifidis, Luis R. Rodriguez At least type check the ATH5K_TRACE paramter on !ATH5K_DEBUG configs. Signed-off-by: Jiri Slaby <jirislaby@gmail.com> Cc: Nick Kossifidis <mickflemm@gmail.com> Cc: Luis R. Rodriguez <mcgrof@gmail.com> --- drivers/net/wireless/ath5k/debug.h | 4 +++- 1 files changed, 3 insertions(+), 1 deletions(-) diff --git a/drivers/net/wireless/ath5k/debug.h b/drivers/net/wireless/ath5k/debug.h index c4fd8c4..44324c3 100644 --- a/drivers/net/wireless/ath5k/debug.h +++ b/drivers/net/wireless/ath5k/debug.h @@ -171,7 +171,9 @@ ath5k_debug_printtxbuf(struct ath5k_softc *sc, #else /* no debugging */ -#define ATH5K_TRACE(_sc) /* empty */ +#include <linux/compiler.h> + +#define ATH5K_TRACE(_sc) typecheck(struct ath5k_softc *, (_sc)) static inline void __attribute__ ((format (printf, 3, 4))) ATH5K_DBG(struct ath5k_softc *sc, unsigned int m, const char *fmt, ...) {} -- 1.5.3.8 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH 3/3] WDEV: ath5k, typecheck on nonDEBUG 2008-02-15 20:58 ` [PATCH 3/3] WDEV: ath5k, typecheck on nonDEBUG Jiri Slaby @ 2008-02-15 22:32 ` Luis R. Rodriguez 0 siblings, 0 replies; 10+ messages in thread From: Luis R. Rodriguez @ 2008-02-15 22:32 UTC (permalink / raw) To: Jiri Slaby Cc: linville, linux-kernel, linux-wireless, ath5k-devel, Nick Kossifidis On Fri, Feb 15, 2008 at 3:58 PM, Jiri Slaby <jirislaby@gmail.com> wrote: > At least type check the ATH5K_TRACE paramter on !ATH5K_DEBUG configs. That's pretty cool. > Signed-off-by: Jiri Slaby <jirislaby@gmail.com> > Cc: Nick Kossifidis <mickflemm@gmail.com> > Cc: Luis R. Rodriguez <mcgrof@gmail.com> Acked-by: Luis R. Rodriguez <mcgrof@winlab.rutgers.edu> Luis ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/3] WDEV: ath5k, fix lock imbalance 2008-02-15 20:58 [PATCH 1/3] WDEV: ath5k, fix lock imbalance Jiri Slaby 2008-02-15 20:58 ` [PATCH 2/3] WDEV, ath5k, don't return int from bool function Jiri Slaby 2008-02-15 20:58 ` [PATCH 3/3] WDEV: ath5k, typecheck on nonDEBUG Jiri Slaby @ 2008-02-15 22:00 ` Nick Kossifidis 2 siblings, 0 replies; 10+ messages in thread From: Nick Kossifidis @ 2008-02-15 22:00 UTC (permalink / raw) To: Jiri Slaby Cc: linville, linux-kernel, linux-wireless, ath5k-devel, Luis R. Rodriguez 2008/2/15, Jiri Slaby <jirislaby@gmail.com>: > Omitted lock causes sparse warning > drivers/net/wireless/ath5k/base.c:1682:1: warning: context imbalance in 'ath5k_tasklet_rx' - different lock contexts for basic block > > Add the lock to the guilty fail path. > > Signed-off-by: Jiri Slaby <jirislaby@gmail.com> > Cc: Nick Kossifidis <mickflemm@gmail.com> > Cc: Luis R. Rodriguez <mcgrof@gmail.com> > --- > drivers/net/wireless/ath5k/base.c | 1 + > 1 files changed, 1 insertions(+), 0 deletions(-) > > diff --git a/drivers/net/wireless/ath5k/base.c b/drivers/net/wireless/ath5k/base.c > index ddc8714..49d38e8 100644 > --- a/drivers/net/wireless/ath5k/base.c > +++ b/drivers/net/wireless/ath5k/base.c > @@ -1715,6 +1715,7 @@ ath5k_tasklet_rx(unsigned long data) > break; > else if (unlikely(ret)) { > ATH5K_ERR(sc, "error in processing rx descriptor\n"); > + spin_unlock(&sc->rxbuflock); > return; > } > > Acked-by: Nick Kossifidis <mickflemm@gmail.com> -- GPG ID: 0xD21DB2DB As you read this post global entropy rises. Have Fun ;-) Nick ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2008-02-15 23:29 UTC | newest] Thread overview: 10+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2008-02-15 20:58 [PATCH 1/3] WDEV: ath5k, fix lock imbalance Jiri Slaby 2008-02-15 20:58 ` [PATCH 2/3] WDEV, ath5k, don't return int from bool function Jiri Slaby 2008-02-15 22:03 ` Nick Kossifidis 2008-02-15 22:08 ` Luis R. Rodriguez 2008-02-15 22:38 ` Jiri Slaby 2008-02-15 23:24 ` Luis R. Rodriguez 2008-02-15 23:29 ` Jiri Slaby 2008-02-15 20:58 ` [PATCH 3/3] WDEV: ath5k, typecheck on nonDEBUG Jiri Slaby 2008-02-15 22:32 ` Luis R. Rodriguez 2008-02-15 22:00 ` [PATCH 1/3] WDEV: ath5k, fix lock imbalance Nick Kossifidis
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).