* [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 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.