On Fri, 9 Jun 2017, tang.junhui@xxxxxxxxxx wrote:
> Hello Eric,
>
>
> ＞ It would be nice if ZERO_RATE_DELAY_NS was configurable via sysfs, as some
> ＞ might want a longer delay than 30s.
>
> How about we reuse the dc-＞writeback_delay configuration for the delay time?
That seems reasonable.
For small writeback caches where the cache could be exhausted while in
schedule_timeout(), will IOs stall until schedule_timeout() allows
writeback_thread to proceed?
> ＞ Also, if zero-rate has been hit, why not have a full-writeback of
> whatever ＞ remains so we only use the ZERO_RATE_DELAY_NS when
> dirty_data==0? No ＞ sense in leaving it slightly dirty if there is
> only a block or two still ＞ remaining.
>
> I think it's risky, write IO maybe in continue, we should to accumulate
> more dirty data
ok, agreed---also I note that writeback is still 10% dirty by default, so
it could be a large amount of data. The cache is always seeking to hit
dirty_ratio and this is what the 1-sector writeback fix is targetd for.
>
> to make writeback sequentially, otherwise, the full-writeback would be
> inefficient when there
>
> is only a little dirty, as it is small and random IOs, and the write IO
> are in continue.
>
>
> So, Eric, would you agree to just modify this patch to reuse
> dc-＞writeback_delay as
>
> the delay time?
Yes that seems OK, unless someone else says otherwise.
>
> Acctually, we do many optimizations for writeback in our product, such
> as the issue that the dirty device never get clean (we start
> full-writeback when we find there is no more write IOs in comming),
> these modifications make bcache works better.
I would be interested in seeing other optimization patches if you have
any. Thank you for your work on this!
-Eric
>
>
> Regards,
>
> Tang Junhui
>
>
>
>
> 原始邮件
> 发件人： ＜bcache@xxxxxxxxxxxxxxxxxx＞;
> 收件人： ＜i@xxxxxxx＞;
> 抄送人：唐军辉10074136; ＜kent.overstreet@xxxxxxxxx＞; ＜linux-bcache@xxxxxxxxxxxxxxx＞;
> 日 期 ：2017年06月09日 09:43
> 主 题 ：Re: [PATCH] bcache: fix issue of writeback rate at minimum 1 blockper second
>
>
> On Fri, 9 Jun 2017, Coly Li wrote:
>
> ＞ On 2017/6/8 下午5:34, tang.junhui@xxxxxxxxxx wrote:
> ＞ ＞ From: Tang Junhui ＜tang.junhui@xxxxxxxxxx＞
> ＞ ＞
> ＞ ＞ When there is not enough data in writeback cache, let the writeback rate
> ＞ ＞ to be 0, and delay 30 seconds in read_dirty().
> ＞ ＞
> ＞
> ＞ I feel uncomfortable to the above idea. Is the purpose of this change is
> ＞ to make writeback more efficient with more dirty data ? Or some other
> ＞ reason that you want to make this change?
> ＞
> ＞ When data writing back to the backing device, most of the writes are
> ＞ random. If the backing device is a hard disk, the write back rate will
> ＞ be very slow. Such a longer delay only makes more data accumulated in
> ＞ writeback cache, does not help the writeback rate indeed.
> ＞
> ＞ Do you observe performance improvement by this patch ? If yes, I'd like
> ＞ to see the performance number.
>
> Its not a performance issue, it is a power saving issue. Laptop users
> have reported that zero-rate writeback writes 512-bytes every second and
> never lets the HDD spin down and several users on the list have asked for
> such a bugfix.
>
> It would be nice if ZERO_RATE_DELAY_NS was configurable via sysfs, as some
> might want a longer delay than 30s.
>
> Also, if zero-rate has been hit, why not have a full-writeback of whatever
> remains so we only use the ZERO_RATE_DELAY_NS when dirty_data==0? No
> sense in leaving it slightly dirty if there is only a block or two still
> remaining.
>
>
> --
> Eric Wheeler
>
>
>
> ＞
> ＞ Thanks.
> ＞
> ＞ Coly
> ＞
> ＞
> ＞ ＞ Signed-off-by: Tang Junhui ＜tang.junhui@xxxxxxxxxx＞
> ＞ ＞ ---
> ＞ ＞ drivers/md/bcache/util.c | 17 ++++++++++++-----
> ＞ ＞ drivers/md/bcache/writeback.c | 2 +-
> ＞ ＞ 2 files changed, 13 insertions(+), 6 deletions(-)
> ＞ ＞
> ＞ ＞ diff --git a/drivers/md/bcache/util.c b/drivers/md/bcache/util.c
> ＞ ＞ index dde6172..cf750df 100644
> ＞ ＞ --- a/drivers/md/bcache/util.c
> ＞ ＞ +++ b/drivers/md/bcache/util.c
> ＞ ＞ @@ -15,6 +15,8 @@
> ＞ ＞
> ＞ ＞ #include "util.h"
> ＞ ＞
> ＞ ＞ +#define ZERO_RATE_DELAY_NS 30*NSEC_PER_SEC
> ＞ ＞ +
> ＞ ＞ #define simple_strtoint(c, end, base) simple_strtol(c, end, base)
> ＞ ＞ #define simple_strtouint(c, end, base) simple_strtoul(c, end, base)
> ＞ ＞
> ＞ ＞ @@ -209,13 +211,18 @@ uint64_t bch_next_delay(struct bch_ratelimit *d, uint64_t done)
> ＞ ＞ {
> ＞ ＞ uint64_t now = local_clock();
> ＞ ＞
> ＞ ＞ - d-＞next += div_u64(done * NSEC_PER_SEC, d-＞rate);
> ＞ ＞ + if (!d-＞rate) {
> ＞ ＞ + d-＞next = now + ZERO_RATE_DELAY_NS;
> ＞ ＞ + }
> ＞ ＞ + else {
> ＞ ＞ + d-＞next += div_u64(done * NSEC_PER_SEC, d-＞rate);
> ＞ ＞
> ＞ ＞ - if (time_before64(now + NSEC_PER_SEC, d-＞next))
> ＞ ＞ - d-＞next = now + NSEC_PER_SEC;
> ＞ ＞ + if (time_before64(now + NSEC_PER_SEC, d-＞next))
> ＞ ＞ + d-＞next = now + NSEC_PER_SEC;
> ＞ ＞
> ＞ ＞ - if (time_after64(now - NSEC_PER_SEC * 2, d-＞next))
> ＞ ＞ - d-＞next = now - NSEC_PER_SEC * 2;
> ＞ ＞ + if (time_after64(now - NSEC_PER_SEC * 2, d-＞next))
> ＞ ＞ + d-＞next = now - NSEC_PER_SEC * 2;
> ＞ ＞ + }
> ＞ ＞
> ＞ ＞ return time_after64(d-＞next, now)
> ＞ ＞ ? div_u64(d-＞next - now, NSEC_PER_SEC / HZ)
> ＞ ＞ diff --git a/drivers/md/bcache/writeback.c b/drivers/md/bcache/writeback.c
> ＞ ＞ index 69e1ae5..8fac280 100644
> ＞ ＞ --- a/drivers/md/bcache/writeback.c
> ＞ ＞ +++ b/drivers/md/bcache/writeback.c
> ＞ ＞ @@ -60,7 +60,7 @@ static void __update_writeback_rate(struct cached_dev *dc)
> ＞ ＞
> ＞ ＞ dc-＞writeback_rate.rate =
> ＞ ＞ clamp_t(int64_t, (int64_t) dc-＞writeback_rate.rate + change,
> ＞ ＞ - 1, NSEC_PER_MSEC);
> ＞ ＞ + 0, NSEC_PER_MSEC);
> ＞ ＞
> ＞ ＞ dc-＞writeback_rate_proportional = proportional;
> ＞ ＞ dc-＞writeback_rate_derivative = derivative;
> ＞ ＞
> ＞ --
> ＞ To unsubscribe from this list: send the line "unsubscribe linux-bcache" in
> ＞ the body of a message to majordomo@xxxxxxxxxxxxxxx
> ＞ More majordomo info at http://vger.kernel.org/majordomo-info.html
> ＞
>
>
>
>
>
