| Message ID | 1467201146-6844-1-git-send-email-michal.kazior@tieto.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 4ca1807815aa6801aaced7fdefa9edacc2521767 |
| Delegated to: | Kalle Valo |
| Headers | show |
Michal Kazior <michal.kazior@tieto.com> writes: > Ideally wake_tx_queue should be used regardless as > it is a requirement for reducing bufferbloat and > implementing airtime fairness in the future. > > However some setups (typically low-end platforms > hosting QCA988X) suffer performance regressions > with the current wake_tx_queue implementation. > Therefore disable it unless it is really > beneficial with current codebase (which is when > firmware supports smart pull-push tx scheduling). > > Signed-off-by: Michal Kazior <michal.kazior@tieto.com> I think it's too late to send this to 4.7 anymore (and this due to my vacation). So I'm planning to queue this to 4.8, but if the feedback is positive we can always send this to a 4.7 stable release.
Michal Kazior <michal.kazior@tieto.com> wrote: > Ideally wake_tx_queue should be used regardless as > it is a requirement for reducing bufferbloat and > implementing airtime fairness in the future. > > However some setups (typically low-end platforms > hosting QCA988X) suffer performance regressions > with the current wake_tx_queue implementation. > Therefore disable it unless it is really > beneficial with current codebase (which is when > firmware supports smart pull-push tx scheduling). > > Signed-off-by: Michal Kazior <michal.kazior@tieto.com> Thanks, 1 patch applied to ath-next branch of ath.git: 4ca1807815aa ath10k: disable wake_tx_queue for older devices
On 7 July 2016 at 19:30, Valo, Kalle <kvalo@qca.qualcomm.com> wrote: > Michal Kazior <michal.kazior@tieto.com> writes: > >> Ideally wake_tx_queue should be used regardless as >> it is a requirement for reducing bufferbloat and >> implementing airtime fairness in the future. >> >> However some setups (typically low-end platforms >> hosting QCA988X) suffer performance regressions >> with the current wake_tx_queue implementation. >> Therefore disable it unless it is really >> beneficial with current codebase (which is when >> firmware supports smart pull-push tx scheduling). >> >> Signed-off-by: Michal Kazior <michal.kazior@tieto.com> > > I think it's too late to send this to 4.7 anymore (and this due to my > vacation). So I'm planning to queue this to 4.8, but if the feedback is > positive we can always send this to a 4.7 stable release. > Sorry guys, drowned. So, yes, applying this patch does the job. That is gets me to the results similar to https://lists.openwrt.org/pipermail/openwrt-devel/2016-May/041448.html Going to try latest code on same system... Regards, Roman
On Mon, Aug 1, 2016 at 1:35 AM, Roman Yeryomin <leroi.lists@gmail.com> wrote: > On 7 July 2016 at 19:30, Valo, Kalle <kvalo@qca.qualcomm.com> wrote: >> Michal Kazior <michal.kazior@tieto.com> writes: >> >>> Ideally wake_tx_queue should be used regardless as >>> it is a requirement for reducing bufferbloat and >>> implementing airtime fairness in the future. >>> >>> However some setups (typically low-end platforms >>> hosting QCA988X) suffer performance regressions >>> with the current wake_tx_queue implementation. >>> Therefore disable it unless it is really >>> beneficial with current codebase (which is when >>> firmware supports smart pull-push tx scheduling). >>> >>> Signed-off-by: Michal Kazior <michal.kazior@tieto.com> >> >> I think it's too late to send this to 4.7 anymore (and this due to my >> vacation). So I'm planning to queue this to 4.8, but if the feedback is >> positive we can always send this to a 4.7 stable release. >> > > Sorry guys, drowned. > So, yes, applying this patch does the job. That is gets me to the > results similar to > https://lists.openwrt.org/pipermail/openwrt-devel/2016-May/041448.html > > Going to try latest code on same system... Can you try increasing the quantum to 1514, and reducing the codel target to 5ms? (without this patch?) > > Regards, > Roman > > _______________________________________________ > ath10k mailing list > ath10k@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/ath10k
On 1 August 2016 at 12:04, Dave Taht <dave.taht@gmail.com> wrote: > On Mon, Aug 1, 2016 at 1:35 AM, Roman Yeryomin <leroi.lists@gmail.com> wrote: >> On 7 July 2016 at 19:30, Valo, Kalle <kvalo@qca.qualcomm.com> wrote: >>> Michal Kazior <michal.kazior@tieto.com> writes: >>> >>>> Ideally wake_tx_queue should be used regardless as >>>> it is a requirement for reducing bufferbloat and >>>> implementing airtime fairness in the future. >>>> >>>> However some setups (typically low-end platforms >>>> hosting QCA988X) suffer performance regressions >>>> with the current wake_tx_queue implementation. >>>> Therefore disable it unless it is really >>>> beneficial with current codebase (which is when >>>> firmware supports smart pull-push tx scheduling). >>>> >>>> Signed-off-by: Michal Kazior <michal.kazior@tieto.com> >>> >>> I think it's too late to send this to 4.7 anymore (and this due to my >>> vacation). So I'm planning to queue this to 4.8, but if the feedback is >>> positive we can always send this to a 4.7 stable release. >>> >> >> Sorry guys, drowned. >> So, yes, applying this patch does the job. That is gets me to the >> results similar to >> https://lists.openwrt.org/pipermail/openwrt-devel/2016-May/041448.html >> >> Going to try latest code on same system... > > Can you try increasing the quantum to 1514, and reducing the codel > target to 5ms? (without this patch?) > So it was 1514 already... Regards, Roman
On Thu, Aug 4, 2016 at 12:07 PM, Roman Yeryomin <leroi.lists@gmail.com> wrote: > On 1 August 2016 at 12:04, Dave Taht <dave.taht@gmail.com> wrote: >> On Mon, Aug 1, 2016 at 1:35 AM, Roman Yeryomin <leroi.lists@gmail.com> wrote: >>> On 7 July 2016 at 19:30, Valo, Kalle <kvalo@qca.qualcomm.com> wrote: >>>> Michal Kazior <michal.kazior@tieto.com> writes: >>>> >>>>> Ideally wake_tx_queue should be used regardless as >>>>> it is a requirement for reducing bufferbloat and >>>>> implementing airtime fairness in the future. >>>>> >>>>> However some setups (typically low-end platforms >>>>> hosting QCA988X) suffer performance regressions >>>>> with the current wake_tx_queue implementation. >>>>> Therefore disable it unless it is really >>>>> beneficial with current codebase (which is when >>>>> firmware supports smart pull-push tx scheduling). >>>>> >>>>> Signed-off-by: Michal Kazior <michal.kazior@tieto.com> >>>> >>>> I think it's too late to send this to 4.7 anymore (and this due to my >>>> vacation). So I'm planning to queue this to 4.8, but if the feedback is >>>> positive we can always send this to a 4.7 stable release. >>>> >>> >>> Sorry guys, drowned. >>> So, yes, applying this patch does the job. That is gets me to the >>> results similar to >>> https://lists.openwrt.org/pipermail/openwrt-devel/2016-May/041448.html >>> >>> Going to try latest code on same system... >> >> Can you try increasing the quantum to 1514, and reducing the codel >> target to 5ms? (without this patch?) >> > > So it was 1514 already... based on some testing of 20, codel target should be 5ms and isn't. https://github.com/torvalds/linux/commit/5caa328e3811b7cfa33fd02c93280ffa622deb0e > Regards, > Roman > > _______________________________________________ > ath10k mailing list > ath10k@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/ath10k
diff --git a/drivers/net/wireless/ath/ath10k/core.h b/drivers/net/wireless/ath/ath10k/core.h index 3da18c9dbd7a..de580af2e5ef 100644 --- a/drivers/net/wireless/ath/ath10k/core.h +++ b/drivers/net/wireless/ath/ath10k/core.h @@ -667,6 +667,7 @@ struct ath10k_fw_components { struct ath10k { struct ath_common ath_common; struct ieee80211_hw *hw; + struct ieee80211_ops *ops; struct device *dev; u8 mac_addr[ETH_ALEN]; diff --git a/drivers/net/wireless/ath/ath10k/mac.c b/drivers/net/wireless/ath/ath10k/mac.c index d4b7a168f7c0..958bc6c91aac 100644 --- a/drivers/net/wireless/ath/ath10k/mac.c +++ b/drivers/net/wireless/ath/ath10k/mac.c @@ -7471,21 +7471,32 @@ static const struct ieee80211_channel ath10k_5ghz_channels[] = { struct ath10k *ath10k_mac_create(size_t priv_size) { struct ieee80211_hw *hw; + struct ieee80211_ops *ops; struct ath10k *ar; - hw = ieee80211_alloc_hw(sizeof(struct ath10k) + priv_size, &ath10k_ops); - if (!hw) + ops = kmemdup(&ath10k_ops, sizeof(ath10k_ops), GFP_KERNEL); + if (!ops) return NULL; + hw = ieee80211_alloc_hw(sizeof(struct ath10k) + priv_size, ops); + if (!hw) { + kfree(ops); + return NULL; + } + ar = hw->priv; ar->hw = hw; + ar->ops = ops; return ar; } void ath10k_mac_destroy(struct ath10k *ar) { + struct ieee80211_ops *ops = ar->ops; + ieee80211_free_hw(ar->hw); + kfree(ops); } static const struct ieee80211_iface_limit ath10k_if_limits[] = { @@ -7919,6 +7930,15 @@ int ath10k_mac_register(struct ath10k *ar) ath10k_warn(ar, "failed to initialise DFS pattern detector\n"); } + /* Current wake_tx_queue implementation imposes a significant + * performance penalty in some setups. The tx scheduling code needs + * more work anyway so disable the wake_tx_queue unless firmware + * supports the pull-push mechanism. + */ + if (!test_bit(ATH10K_FW_FEATURE_PEER_FLOW_CONTROL, + ar->running_fw->fw_file.fw_features)) + ar->ops->wake_tx_queue = NULL; + ret = ath_regd_init(&ar->ath_common.regulatory, ar->hw->wiphy, ath10k_reg_notifier); if (ret) {
Ideally wake_tx_queue should be used regardless as it is a requirement for reducing bufferbloat and implementing airtime fairness in the future. However some setups (typically low-end platforms hosting QCA988X) suffer performance regressions with the current wake_tx_queue implementation. Therefore disable it unless it is really beneficial with current codebase (which is when firmware supports smart pull-push tx scheduling). Signed-off-by: Michal Kazior <michal.kazior@tieto.com> --- Notes: v1: - improve commit log drivers/net/wireless/ath/ath10k/core.h | 1 + drivers/net/wireless/ath/ath10k/mac.c | 24 ++++++++++++++++++++++-- 2 files changed, 23 insertions(+), 2 deletions(-)