Here are two fixes to the PID rc. The first one improves PID
performance by quite a bit on my test setup, and seems correct,
but I am not an expert on the algo so comments welcome. The
other is just a minor cleanup. With the change, Minstrel is
still a bit better in my tests.
Bob Copeland (2):
mac80211: fix sign error in pid controller
mac80211: pid: replace open-coded msecs_to_jiffies
net/mac80211/rc80211_pid_algo.c | 10 +++-------
1 files changed, 3 insertions(+), 7 deletions(-)
On Sat, 16 Jan 2010 14:36:52 -0500
Bob Copeland <[email protected]> wrote:
> While testing the pid rate controller in mac80211_hwsim, I noticed
> that once the controller reached 54 Mbit rates, it would fail to
> lower the rate when necessary. The debug log shows:
>
> 1945 186786 pf_sample 50 3534 3577 50
>
> My interpretation is that the fixed point scaling of the target
> error value (pf) is incorrect: the error value of 50 compared to
> a target of 14 case should result in a scaling value of
> (14-50) = -36 * 256 or -9216, but instead it is (14 * 256)-50, or
> 3534.
Good catch! I actually wonder why nobody hit this before. One possible
explanation is that the proportional error alone, in most cases, isn't
significant "enough" with regard to the other ones.
Mattias, could you please double check this anyway?
> Cc: [email protected]
> Cc: [email protected]
> Signed-off-by: Bob Copeland <[email protected]>
Acked-by: Stefano Brivio <[email protected]>
--
Ciao
Stefano
Code directly scaling by HZ and rounding can be more efficiently
and clearly performed with msecs_to_jiffies.
Signed-off-by: Bob Copeland <[email protected]>
---
net/mac80211/rc80211_pid_algo.c | 8 ++------
1 files changed, 2 insertions(+), 6 deletions(-)
diff --git a/net/mac80211/rc80211_pid_algo.c b/net/mac80211/rc80211_pid_algo.c
index 29bc4c5..2652a37 100644
--- a/net/mac80211/rc80211_pid_algo.c
+++ b/net/mac80211/rc80211_pid_algo.c
@@ -157,9 +157,7 @@ static void rate_control_pid_sample(struct rc_pid_info *pinfo,
/* In case nothing happened during the previous control interval, turn
* the sharpening factor on. */
- period = (HZ * pinfo->sampling_period + 500) / 1000;
- if (!period)
- period = 1;
+ period = msecs_to_jiffies(pinfo->sampling_period);
if (jiffies - spinfo->last_sample > 2 * period)
spinfo->sharp_cnt = pinfo->sharpen_duration;
@@ -252,9 +250,7 @@ static void rate_control_pid_tx_status(void *priv, struct ieee80211_supported_ba
}
/* Update PID controller state. */
- period = (HZ * pinfo->sampling_period + 500) / 1000;
- if (!period)
- period = 1;
+ period = msecs_to_jiffies(pinfo->sampling_period);
if (time_after(jiffies, spinfo->last_sample + period))
rate_control_pid_sample(pinfo, sband, sta, spinfo);
}
--
1.6.3.3
On Sat, 2010-01-16 at 14:36 -0500, Bob Copeland wrote:
> While testing the pid rate controller in mac80211_hwsim, I noticed
> that once the controller reached 54 Mbit rates, it would fail to
> lower the rate when necessary. The debug log shows:
>
> 1945 186786 pf_sample 50 3534 3577 50
>
> My interpretation is that the fixed point scaling of the target
> error value (pf) is incorrect: the error value of 50 compared to
> a target of 14 case should result in a scaling value of
> (14-50) = -36 * 256 or -9216, but instead it is (14 * 256)-50, or
> 3534.
>
> Correct this by doing fixed point scaling after subtraction.
>
> Cc: [email protected]
> Cc: [email protected]
> Signed-off-by: Bob Copeland <[email protected]>
I took a quick look and found that you're right. pf is an unscaled
value, so we should subtract it before scaling.
Acked-by: Mattias Nissler <[email protected]>
> ---
>
> Mattias / Stefano, please advise if I missed something.
>
> net/mac80211/rc80211_pid_algo.c | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/net/mac80211/rc80211_pid_algo.c b/net/mac80211/rc80211_pid_algo.c
> index 699d3ed..29bc4c5 100644
> --- a/net/mac80211/rc80211_pid_algo.c
> +++ b/net/mac80211/rc80211_pid_algo.c
> @@ -190,7 +190,7 @@ static void rate_control_pid_sample(struct rc_pid_info *pinfo,
> rate_control_pid_normalize(pinfo, sband->n_bitrates);
>
> /* Compute the proportional, integral and derivative errors. */
> - err_prop = (pinfo->target << RC_PID_ARITH_SHIFT) - pf;
> + err_prop = (pinfo->target - pf) << RC_PID_ARITH_SHIFT;
>
> err_avg = spinfo->err_avg_sc >> pinfo->smoothing_shift;
> spinfo->err_avg_sc = spinfo->err_avg_sc - err_avg + err_prop;
While testing the pid rate controller in mac80211_hwsim, I noticed
that once the controller reached 54 Mbit rates, it would fail to
lower the rate when necessary. The debug log shows:
1945 186786 pf_sample 50 3534 3577 50
My interpretation is that the fixed point scaling of the target
error value (pf) is incorrect: the error value of 50 compared to
a target of 14 case should result in a scaling value of
(14-50) = -36 * 256 or -9216, but instead it is (14 * 256)-50, or
3534.
Correct this by doing fixed point scaling after subtraction.
Cc: [email protected]
Cc: [email protected]
Signed-off-by: Bob Copeland <[email protected]>
---
Mattias / Stefano, please advise if I missed something.
net/mac80211/rc80211_pid_algo.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/net/mac80211/rc80211_pid_algo.c b/net/mac80211/rc80211_pid_algo.c
index 699d3ed..29bc4c5 100644
--- a/net/mac80211/rc80211_pid_algo.c
+++ b/net/mac80211/rc80211_pid_algo.c
@@ -190,7 +190,7 @@ static void rate_control_pid_sample(struct rc_pid_info *pinfo,
rate_control_pid_normalize(pinfo, sband->n_bitrates);
/* Compute the proportional, integral and derivative errors. */
- err_prop = (pinfo->target << RC_PID_ARITH_SHIFT) - pf;
+ err_prop = (pinfo->target - pf) << RC_PID_ARITH_SHIFT;
err_avg = spinfo->err_avg_sc >> pinfo->smoothing_shift;
spinfo->err_avg_sc = spinfo->err_avg_sc - err_avg + err_prop;
--
1.6.3.3