| Message ID | 20160923195911.4572-4-toke@toke.dk (mailing list archive) |
|---|---|
| State | Accepted |
| Delegated to: | Johannes Berg |
| Headers | show |
On Fri, 2016-09-23 at 21:59 +0200, Toke Høiland-Jørgensen wrote: > Small devices can run out of memory from queueing too many packets. > If VHT is not supported by the PHY, having more than 4 MBytes of > total queue in the TXQ intermediate queues is not needed, and so we > can safely limit the memory usage in these cases and avoid OOM. I kinda see the logic here - we really don't need to queue as much if we can't possibly transmit it out quickly - but it seems to me we should also throw in some kind of limit that's relative to the amount of memory you have on the system? I've applied these anyway though. I just don't like your assumption (b) much as the rationale for it. johannes
Johannes Berg <johannes@sipsolutions.net> writes: > On Fri, 2016-09-23 at 21:59 +0200, Toke Høiland-Jørgensen wrote: >> Small devices can run out of memory from queueing too many packets. >> If VHT is not supported by the PHY, having more than 4 MBytes of >> total queue in the TXQ intermediate queues is not needed, and so we >> can safely limit the memory usage in these cases and avoid OOM. > > I kinda see the logic here - we really don't need to queue as much if > we can't possibly transmit it out quickly - but it seems to me we > should also throw in some kind of limit that's relative to the amount > of memory you have on the system? Yes, ideally. That goes for FQ-CoDel as well, BTW. LEDE currently carries a patch for that which just changes the hard-coded default to another hard-coded default. Not sure how to get a good value to use, though; and deciding on how large a fraction of memory to use for packets starts smelling an awful lot like setting policy in the kernel, doesn't it? > I've applied these anyway though. I just don't like your assumption (b) > much as the rationale for it. Right, thanks. I'll come up with a better rationale next time ;) -Toke
> > I kinda see the logic here - we really don't need to queue as much > > if we can't possibly transmit it out quickly - but it seems to me > > we should also throw in some kind of limit that's relative to the > > amount of memory you have on the system? > > Yes, ideally. That goes for FQ-CoDel as well, BTW. LEDE currently > carries a patch for that which just changes the hard-coded default to > another hard-coded default. Not sure how to get a good value to use, > though; and deciding on how large a fraction of memory to use for > packets starts smelling an awful lot like setting policy in the > kernel, doesn't it? Yeah, I agree it does seem awkward. Perhaps we should instead pick a low limit and let users change it more easily (i.e. not debugfs)? I don't know a good answer to this either. johannes
Johannes Berg <johannes@sipsolutions.net> writes: >> > I kinda see the logic here - we really don't need to queue as much >> > if we can't possibly transmit it out quickly - but it seems to me >> > we should also throw in some kind of limit that's relative to the >> > amount of memory you have on the system? >> >> Yes, ideally. That goes for FQ-CoDel as well, BTW. LEDE currently >> carries a patch for that which just changes the hard-coded default to >> another hard-coded default. Not sure how to get a good value to use, >> though; and deciding on how large a fraction of memory to use for >> packets starts smelling an awful lot like setting policy in the >> kernel, doesn't it? > > Yeah, I agree it does seem awkward. > > Perhaps we should instead pick a low limit and let users change it > more easily (i.e. not debugfs)? I don't know a good answer to this > either. Hmm, I'll talk it over with some of the LEDE people who are more used to dealing with these sorts of memory-constrained devices than I am. Will send a patch if we come up with a good solution :) -Toke
diff --git a/net/mac80211/tx.c b/net/mac80211/tx.c index 1ff08be..82f41fc 100644 --- a/net/mac80211/tx.c +++ b/net/mac80211/tx.c @@ -1434,6 +1434,8 @@ int ieee80211_txq_setup_flows(struct ieee80211_local *local) struct fq *fq = &local->fq; int ret; int i; + bool supp_vht = false; + enum nl80211_band band; if (!local->ops->wake_tx_queue) return 0; @@ -1442,6 +1444,22 @@ int ieee80211_txq_setup_flows(struct ieee80211_local *local) if (ret) return ret; + /* + * If the hardware doesn't support VHT, it is safe to limit the maximum + * queue size. 4 Mbytes is 64 max-size aggregates in 802.11n. + */ + for (band = 0; band < NUM_NL80211_BANDS; band++) { + struct ieee80211_supported_band *sband; + + sband = local->hw.wiphy->bands[band]; + if (!sband) + continue; + + supp_vht = supp_vht || sband->vht_cap.vht_supported; + } + if (!supp_vht) + fq->memory_limit = 4 << 20; /* 4 Mbytes */ + codel_params_init(&local->cparams); local->cparams.interval = MS2TIME(100); local->cparams.target = MS2TIME(20);
Small devices can run out of memory from queueing too many packets. If VHT is not supported by the PHY, having more than 4 MBytes of total queue in the TXQ intermediate queues is not needed, and so we can safely limit the memory usage in these cases and avoid OOM. Signed-off-by: Toke Høiland-Jørgensen <toke@toke.dk> --- net/mac80211/tx.c | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+)