Re: svn commit: r250609 - head/sys/dev/ath
On Wed, May 22, 2019 at 10:34:35AM -0600, Ian Lepore wrote: > On Wed, 2019-05-22 at 16:20 +, Alexey Dokuchaev wrote: > > On Mon, May 13, 2013 at 07:03:13PM +, Adrian Chadd wrote: > > > New Revision: 250609 > > > URL: http://svnweb.freebsd.org/changeset/base/250609 > > > > > > Log: > > > Since the node state is 100% back under the TX lock, just kill > > > the use > > > of atomics. > > > > > > Modified: > > > head/sys/dev/ath/if_ath.c > > > > > > @@ -6140,13 +6133,13 @@ ath_tx_update_tim(struct ath_softc *sc, > > > /* > > >* Don't bother grabbing the lock unless the queue is > > > empty. > > >*/ > > > - if (atomic_load_acq_int(&an->an_swq_depth) != 0) > > > + if (&an->an_swq_depth != 0) > > > return; > > > > > > if (an->an_is_powersave && > > > an->an_stack_psq == 0 && > > > an->an_tim_set == 1 && > > > - atomic_load_acq_int(&an->an_swq_depth) == 0) { > > > + an->an_swq_depth == 0) { > > > > PVS Studio complains here: warning: V560 A part of conditional > > expression > > is always true: an->an_swq_depth == 0. Which probably makes sense > > since > > you return earlier if it's != 0. > > You're replying to a six year old commit? Yeah, anything wrong with it? > It doesn't check earlier whether that value is 0, it checks whether the > address of that value is 0. That's probably a bug. No, the & is a vestige from the atomic_* conversion, it is not present in current code. The check is still there, however. ./danfe ___ svn-src-all@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/svn-src-all To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"
Re: svn commit: r250609 - head/sys/dev/ath
On Wed, 2019-05-22 at 16:20 +, Alexey Dokuchaev wrote: > On Mon, May 13, 2013 at 07:03:13PM +, Adrian Chadd wrote: > > New Revision: 250609 > > URL: http://svnweb.freebsd.org/changeset/base/250609 > > > > Log: > > Since the node state is 100% back under the TX lock, just kill > > the use > > of atomics. > > > > Modified: > > head/sys/dev/ath/if_ath.c > > > > @@ -6140,13 +6133,13 @@ ath_tx_update_tim(struct ath_softc *sc, > > /* > > * Don't bother grabbing the lock unless the queue is > > empty. > > */ > > - if (atomic_load_acq_int(&an->an_swq_depth) != 0) > > + if (&an->an_swq_depth != 0) > > return; > > > > if (an->an_is_powersave && > > an->an_stack_psq == 0 && > > an->an_tim_set == 1 && > > - atomic_load_acq_int(&an->an_swq_depth) == 0) { > > + an->an_swq_depth == 0) { > > PVS Studio complains here: warning: V560 A part of conditional > expression > is always true: an->an_swq_depth == 0. Which probably makes sense > since > you return earlier if it's != 0. > > ./danfe > You're replying to a six year old commit? It doesn't check earlier whether that value is 0, it checks whether the address of that value is 0. That's probably a bug. -- Ian ___ svn-src-all@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/svn-src-all To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"
Re: svn commit: r250609 - head/sys/dev/ath
On Mon, May 13, 2013 at 07:03:13PM +, Adrian Chadd wrote: > New Revision: 250609 > URL: http://svnweb.freebsd.org/changeset/base/250609 > > Log: > Since the node state is 100% back under the TX lock, just kill the use > of atomics. > > Modified: > head/sys/dev/ath/if_ath.c > > @@ -6140,13 +6133,13 @@ ath_tx_update_tim(struct ath_softc *sc, > /* >* Don't bother grabbing the lock unless the queue is empty. >*/ > - if (atomic_load_acq_int(&an->an_swq_depth) != 0) > + if (&an->an_swq_depth != 0) > return; > > if (an->an_is_powersave && > an->an_stack_psq == 0 && > an->an_tim_set == 1 && > - atomic_load_acq_int(&an->an_swq_depth) == 0) { > + an->an_swq_depth == 0) { PVS Studio complains here: warning: V560 A part of conditional expression is always true: an->an_swq_depth == 0. Which probably makes sense since you return earlier if it's != 0. ./danfe ___ svn-src-all@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/svn-src-all To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"
svn commit: r250609 - head/sys/dev/ath
Author: adrian Date: Mon May 13 19:03:12 2013 New Revision: 250609 URL: http://svnweb.freebsd.org/changeset/base/250609 Log: Since the node state is 100% back under the TX lock, just kill the use of atomics. I'll re-think this nonsense later. Modified: head/sys/dev/ath/if_ath.c head/sys/dev/ath/if_athvar.h Modified: head/sys/dev/ath/if_ath.c == --- head/sys/dev/ath/if_ath.c Mon May 13 19:02:22 2013(r250608) +++ head/sys/dev/ath/if_ath.c Mon May 13 19:03:12 2013(r250609) @@ -6028,7 +6028,7 @@ ath_node_set_tim(struct ieee80211_node * an->an_tim_set = 1; ATH_TX_UNLOCK(sc); changed = avp->av_set_tim(ni, enable); - } else if (atomic_load_acq_int(&an->an_swq_depth) == 0) { + } else if (an->an_swq_depth == 0) { /* disable */ DPRINTF(sc, ATH_DEBUG_NODE_PWRSAVE, "%s: an=%p, enable=%d, an_swq_depth == 0, disabling\n", @@ -6120,16 +6120,9 @@ ath_tx_update_tim(struct ath_softc *sc, ATH_TX_LOCK_ASSERT(sc); if (enable) { - /* -* Don't bother grabbing the lock unless the queue is not -* empty. -*/ - if (atomic_load_acq_int(&an->an_swq_depth) == 0) - return; - if (an->an_is_powersave && an->an_tim_set == 0 && - atomic_load_acq_int(&an->an_swq_depth) != 0) { + an->an_swq_depth != 0) { DPRINTF(sc, ATH_DEBUG_NODE_PWRSAVE, "%s: an=%p, swq_depth>0, tim_set=0, set!\n", __func__, an); @@ -6140,13 +6133,13 @@ ath_tx_update_tim(struct ath_softc *sc, /* * Don't bother grabbing the lock unless the queue is empty. */ - if (atomic_load_acq_int(&an->an_swq_depth) != 0) + if (&an->an_swq_depth != 0) return; if (an->an_is_powersave && an->an_stack_psq == 0 && an->an_tim_set == 1 && - atomic_load_acq_int(&an->an_swq_depth) == 0) { + an->an_swq_depth == 0) { DPRINTF(sc, ATH_DEBUG_NODE_PWRSAVE, "%s: an=%p, swq_depth=0, tim_set=1, psq_set=0," " clear!\n", Modified: head/sys/dev/ath/if_athvar.h == --- head/sys/dev/ath/if_athvar.hMon May 13 19:02:22 2013 (r250608) +++ head/sys/dev/ath/if_athvar.hMon May 13 19:03:12 2013 (r250609) @@ -415,17 +415,17 @@ struct ath_txq { #define ATH_TID_INSERT_HEAD(_tq, _elm, _field) do { \ TAILQ_INSERT_HEAD(&(_tq)->tid_q, (_elm), _field); \ (_tq)->axq_depth++; \ - atomic_add_rel_32( &((_tq)->an)->an_swq_depth, 1); \ + (_tq)->an->an_swq_depth++; \ } while (0) #define ATH_TID_INSERT_TAIL(_tq, _elm, _field) do { \ TAILQ_INSERT_TAIL(&(_tq)->tid_q, (_elm), _field); \ (_tq)->axq_depth++; \ - atomic_add_rel_32( &((_tq)->an)->an_swq_depth, 1); \ + (_tq)->an->an_swq_depth++; \ } while (0) #define ATH_TID_REMOVE(_tq, _elm, _field) do { \ TAILQ_REMOVE(&(_tq)->tid_q, _elm, _field); \ (_tq)->axq_depth--; \ - atomic_subtract_rel_32( &((_tq)->an)->an_swq_depth, 1); \ + (_tq)->an->an_swq_depth--; \ } while (0) #defineATH_TID_FIRST(_tq) TAILQ_FIRST(&(_tq)->tid_q) #defineATH_TID_LAST(_tq, _field) TAILQ_LAST(&(_tq)->tid_q, _field) @@ -436,17 +436,17 @@ struct ath_txq { #define ATH_TID_FILT_INSERT_HEAD(_tq, _elm, _field) do { \ TAILQ_INSERT_HEAD(&(_tq)->filtq.tid_q, (_elm), _field); \ (_tq)->axq_depth++; \ - atomic_add_rel_32( &((_tq)->an)->an_swq_depth, 1); \ + (_tq)->an->an_swq_depth++; \ } while (0) #define ATH_TID_FILT_INSERT_TAIL(_tq, _elm, _field) do { \ TAILQ_INSERT_TAIL(&(_tq)->filtq.tid_q, (_elm), _field); \ (_tq)->axq_depth++; \ - atomic_add_rel_32( &((_tq)->an)->an_swq_depth, 1); \ + (_tq)->an->an_swq_depth++; \ } while (0) #define ATH_TID_FILT_REMOVE(_tq, _elm, _field) do { \ TAILQ_REMOVE(&(_tq)->filtq.tid_q, _elm, _field); \ (_tq)->axq_depth--; \ - atomic_subtract_rel_32( &((_tq)->an)->an_swq_depth, 1); \ + (_tq)->an->an_swq_depth--; \ } while (0) #defineATH_TID_FILT_FIRST(_tq) TAILQ_FIRST(&(_tq)->filtq.tid_q) #defineATH_TID_FILT_LAST(_tq, _field) TAILQ_LAST(&(_tq)->filtq.tid_q,_field) ___ svn-src-all@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/svn-src-all To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"