| Message ID | 071dbbbdfecaebf9e850e622c52dd591969e21ab.1606617087.git.baolin.wang@linux.alibaba.com |
|---|---|
| State | New, archived |
| Headers | show |
| Series |
|
| Related | show |
On Sun, Nov 29, 2020 at 10:37:18AM +0800, Baolin Wang wrote: > The ioc_refreash_vrate() will only be called in ioc_timer_fn() after > starting a new period or stopping the period. > > So when starting a new period, the variable 'pleft' in ioc_refreash_vrate() > is always the period's time, which means if the abs(ioc->vtime_err) > is less than the period's time, the vcomp is 0, and we do not need > compensate the vtime_rate in this case, just set it as the base vrate > and return. > > When stopping the period, the ioc->vtime_err will be cleared to 0, > and we also do not need to compensate the vtime_rate, just set it as > the base vrate and return. Before, the function did something which is conceptually discrete and describable. After, its operation is intertwined with when it's called. I don't think this sort of micro optimizations are beneficial in cold paths. Thanks.
在 2020/12/3 4:32, Tejun Heo 写道: > On Sun, Nov 29, 2020 at 10:37:18AM +0800, Baolin Wang wrote: >> The ioc_refreash_vrate() will only be called in ioc_timer_fn() after >> starting a new period or stopping the period. >> >> So when starting a new period, the variable 'pleft' in ioc_refreash_vrate() >> is always the period's time, which means if the abs(ioc->vtime_err) >> is less than the period's time, the vcomp is 0, and we do not need >> compensate the vtime_rate in this case, just set it as the base vrate >> and return. >> >> When stopping the period, the ioc->vtime_err will be cleared to 0, >> and we also do not need to compensate the vtime_rate, just set it as >> the base vrate and return. > > Before, the function did something which is conceptually discrete and > describable. After, its operation is intertwined with when it's called. I > don't think this sort of micro optimizations are beneficial in cold paths. OK. I understood your concern. Thanks for your input.
diff --git a/block/blk-iocost.c b/block/blk-iocost.c index 8348db4..58c9533 100644 --- a/block/blk-iocost.c +++ b/block/blk-iocost.c @@ -943,30 +943,29 @@ static bool ioc_refresh_params(struct ioc *ioc, bool force) */ static void ioc_refresh_vrate(struct ioc *ioc, struct ioc_now *now) { - s64 pleft = ioc->period_at + ioc->period_us - now->now; s64 vperiod = ioc->period_us * ioc->vtime_base_rate; s64 vcomp, vcomp_min, vcomp_max; lockdep_assert_held(&ioc->lock); - /* we need some time left in this period */ - if (pleft <= 0) - goto done; + if (abs(ioc->vtime_err) < ioc->period_us) { + atomic64_set(&ioc->vtime_rate, ioc->vtime_base_rate); + return; + } /* * Calculate how much vrate should be adjusted to offset the error. * Limit the amount of adjustment and deduct the adjusted amount from * the error. */ - vcomp = -div64_s64(ioc->vtime_err, pleft); + vcomp = -div64_s64(ioc->vtime_err, ioc->period_us); vcomp_min = -(ioc->vtime_base_rate >> 1); vcomp_max = ioc->vtime_base_rate; vcomp = clamp(vcomp, vcomp_min, vcomp_max); - ioc->vtime_err += vcomp * pleft; + ioc->vtime_err += vcomp * ioc->period_us; atomic64_set(&ioc->vtime_rate, ioc->vtime_base_rate + vcomp); -done: /* bound how much error can accumulate */ ioc->vtime_err = clamp(ioc->vtime_err, -vperiod, vperiod); }
The ioc_refreash_vrate() will only be called in ioc_timer_fn() after starting a new period or stopping the period. So when starting a new period, the variable 'pleft' in ioc_refreash_vrate() is always the period's time, which means if the abs(ioc->vtime_err) is less than the period's time, the vcomp is 0, and we do not need compensate the vtime_rate in this case, just set it as the base vrate and return. When stopping the period, the ioc->vtime_err will be cleared to 0, and we also do not need to compensate the vtime_rate, just set it as the base vrate and return. Signed-off-by: Baolin Wang <baolin.wang@linux.alibaba.com> --- block/blk-iocost.c | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-)