| Message ID | 1363727997-1554-2-git-send-email-greearb@candelatech.com (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
On Tue, 2013-03-19 at 14:19 -0700, greearb@candelatech.com wrote: > From: Ben Greear <greearb@candelatech.com> > > After that, print it out with net_ratelimit. We saw a system > continually hit this warning, for reasons unknown, and it > seems it bogged the system down enough to make it go OOM. I'm not really sure I like this ... that points to a deeper problem, and this just papers over it while causing more cost in the TX path for all the different checks. johannes -- 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
On 03/22/2013 03:28 AM, Johannes Berg wrote: > On Tue, 2013-03-19 at 14:19 -0700, greearb@candelatech.com wrote: >> From: Ben Greear <greearb@candelatech.com> >> >> After that, print it out with net_ratelimit. We saw a system >> continually hit this warning, for reasons unknown, and it >> seems it bogged the system down enough to make it go OOM. > > I'm not really sure I like this ... that points to a deeper problem, and > this just papers over it while causing more cost in the TX path for all > the different checks. If I add an 'unlikely' to the initial check, that gets back to the original TX path cost, or are you worried about something else? I think in most cases we should be using some variation of WARN_ONCE in all the places that splat a warning... Thanks, Ben
On Fri, 2013-03-22 at 08:59 -0700, Ben Greear wrote: > On 03/22/2013 03:28 AM, Johannes Berg wrote: > > On Tue, 2013-03-19 at 14:19 -0700, greearb@candelatech.com wrote: > >> From: Ben Greear <greearb@candelatech.com> > >> > >> After that, print it out with net_ratelimit. We saw a system > >> continually hit this warning, for reasons unknown, and it > >> seems it bogged the system down enough to make it go OOM. > > > > I'm not really sure I like this ... that points to a deeper problem, and > > this just papers over it while causing more cost in the TX path for all > > the different checks. > > If I add an 'unlikely' to the initial check, that gets back to the original > TX path cost, or are you worried about something else? > > I think in most cases we should be using some variation of WARN_ONCE > in all the places that splat a warning... Let's just make this WARN_ONCE() then maybe? johannes -- 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
diff --git a/net/mac80211/tx.c b/net/mac80211/tx.c index e03a8e9..2eab7d8 100644 --- a/net/mac80211/tx.c +++ b/net/mac80211/tx.c @@ -672,14 +672,26 @@ ieee80211_tx_h_rate_ctrl(struct ieee80211_tx_data *tx) * Lets not bother rate control if we're associated and cannot * talk to the sta. This should not happen. */ - if (WARN(test_bit(SCAN_SW_SCANNING, &tx->local->scanning) && assoc && - !rate_usable_index_exists(sband, &tx->sta->sta), - "%s: Dropped data frame as no usable bitrate found while " - "scanning and associated. Target station: " - "%pM on %d GHz band\n", - tx->sdata->name, hdr->addr1, - info->band ? 5 : 2)) + if (test_bit(SCAN_SW_SCANNING, &tx->local->scanning) && assoc && + !rate_usable_index_exists(sband, &tx->sta->sta)) { + static bool do_once = true; + if (do_once) { + WARN(1, "%s: Dropped data frame as no usable bitrate found while " + "scanning and associated. Target station: " + "%pM on %d GHz band\n", + tx->sdata->name, hdr->addr1, + info->band ? 5 : 2); + do_once = false; + } + else { + net_info_ratelimited("%s: Dropped data frame as no usable bitrate found while " + "scanning and associated. Target station: " + "%pM on %d GHz band\n", + tx->sdata->name, hdr->addr1, + info->band ? 5 : 2); + } return TX_DROP; + } /* * If we're associated with the sta at this point we know we can at