| Message ID | 0ad278def3875fc2c60b4898daa3f0d53288c168.1608975795.git.lorenzo@kernel.org |
|---|---|
| State | New |
| Headers | show |
| Series | [mac80211-next] mac80211: introduce aql_enable node in debugfs | expand |
Lorenzo Bianconi <lorenzo@kernel.org> writes: > Introduce aql_enable node in debugfs in order to enable/disable aql. > This is useful for debugging purpose. Don't mind having a switch, although I wonder if it would be better to overload the existing debugfs file (e.g., a threshold of 0 could disable everything?) so as not to clutter up debugfs too much? -Toke
> > Lorenzo Bianconi <lorenzo@kernel.org> writes: > > > Introduce aql_enable node in debugfs in order to enable/disable aql. > > This is useful for debugging purpose. > > Don't mind having a switch, although I wonder if it would be better to > overload the existing debugfs file (e.g., a threshold of 0 could disable > everything?) so as not to clutter up debugfs too much? > > -Toke > You mean to consider 0 as a special value to disable aql, right? I would prefer to have a dedicated switch for it since I guess it is clearer for users (but I can live with it :) ) Regards, Lorenzo
Lorenzo Bianconi <lorenzo.bianconi@redhat.com> writes: >> >> Lorenzo Bianconi <lorenzo@kernel.org> writes: >> >> > Introduce aql_enable node in debugfs in order to enable/disable aql. >> > This is useful for debugging purpose. >> >> Don't mind having a switch, although I wonder if it would be better to >> overload the existing debugfs file (e.g., a threshold of 0 could disable >> everything?) so as not to clutter up debugfs too much? >> >> -Toke >> > > You mean to consider 0 as a special value to disable aql, right? I > would prefer to have a dedicated switch for it since I guess it is > clearer for users (but I can live with it :) ) Yeah, maybe a bit clearer but at the cost of clutter. I dunno, not a strong preference either way; I guess Johannes can make the call :) -Toke
On Mon, 2021-01-04 at 14:47 +0100, Toke Høiland-Jørgensen wrote: > Lorenzo Bianconi <lorenzo.bianconi@redhat.com> writes: > > > > Lorenzo Bianconi <lorenzo@kernel.org> writes: > > > > > > > Introduce aql_enable node in debugfs in order to enable/disable aql. > > > > This is useful for debugging purpose. > > > > > > Don't mind having a switch, although I wonder if it would be better to > > > overload the existing debugfs file (e.g., a threshold of 0 could disable > > > everything?) so as not to clutter up debugfs too much? > > > > > > -Toke > > > > > > > You mean to consider 0 as a special value to disable aql, right? I > > would prefer to have a dedicated switch for it since I guess it is > > clearer for users (but I can live with it :) ) > > Yeah, maybe a bit clearer but at the cost of clutter. I dunno, not a > strong preference either way; I guess Johannes can make the call :) I'm not sure I care about an extra debugfs file - but I do wonder about the extra check at runtime that would basically never be true since the default is enable ... Maybe that should use a static_branch() or something? johannes
diff --git a/net/mac80211/debugfs.c b/net/mac80211/debugfs.c index 48f144f107d5..898ad57bebd0 100644 --- a/net/mac80211/debugfs.c +++ b/net/mac80211/debugfs.c @@ -581,6 +581,7 @@ void debugfs_hw_add(struct ieee80211_local *local) DEBUGFS_ADD(aql_txq_limit); debugfs_create_u32("aql_threshold", 0600, phyd, &local->aql_threshold); + debugfs_create_bool("aql_enable", 0600, phyd, &local->aql_enable); statsd = debugfs_create_dir("statistics", phyd); diff --git a/net/mac80211/ieee80211_i.h b/net/mac80211/ieee80211_i.h index 8bf9c0e974d6..8c9cce373010 100644 --- a/net/mac80211/ieee80211_i.h +++ b/net/mac80211/ieee80211_i.h @@ -1162,6 +1162,7 @@ struct ieee80211_local { u32 aql_txq_limit_low[IEEE80211_NUM_ACS]; u32 aql_txq_limit_high[IEEE80211_NUM_ACS]; u32 aql_threshold; + bool aql_enable; atomic_t aql_total_pending_airtime; const struct ieee80211_ops *ops; diff --git a/net/mac80211/main.c b/net/mac80211/main.c index dee88ec566ad..b3bec68943c8 100644 --- a/net/mac80211/main.c +++ b/net/mac80211/main.c @@ -700,6 +700,7 @@ struct ieee80211_hw *ieee80211_alloc_hw_nm(size_t priv_data_len, local->airtime_flags = AIRTIME_USE_TX | AIRTIME_USE_RX; local->aql_threshold = IEEE80211_AQL_THRESHOLD; + local->aql_enable = true; atomic_set(&local->aql_total_pending_airtime, 0); INIT_LIST_HEAD(&local->chanctx_list); diff --git a/net/mac80211/tx.c b/net/mac80211/tx.c index 6422da6690f7..86503d47d86e 100644 --- a/net/mac80211/tx.c +++ b/net/mac80211/tx.c @@ -3832,6 +3832,9 @@ bool ieee80211_txq_airtime_check(struct ieee80211_hw *hw, if (!wiphy_ext_feature_isset(local->hw.wiphy, NL80211_EXT_FEATURE_AQL)) return true; + if (!local->aql_enable) + return true; + if (!txq->sta) return true;
Introduce aql_enable node in debugfs in order to enable/disable aql. This is useful for debugging purpose. Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org> --- net/mac80211/debugfs.c | 1 + net/mac80211/ieee80211_i.h | 1 + net/mac80211/main.c | 1 + net/mac80211/tx.c | 3 +++ 4 files changed, 6 insertions(+)