[PATCH] bcache: writeback rate shouldn't artifically clamp

[PATCH] bcache: writeback rate shouldn't artifically clamp

[Michael Lyle]

The previous code artificially limited writeback rate to 1000000
blocks/second (NSEC_PER_MSEC), which is a rate that can be met on fast
hardware.  The rate limiting code works fine (though with decreased
precision) up to 3 orders of magnitude faster, so use NSEC_PER_SEC.

Additionally, ensure that uint32_t is used as a type for rate throughout
the rate management so that type checking/clamp_t can work properly.

bch_next_delay should be rewritten for increased precision and better
handling of high rates and long sleep periods, but this is adequate for
now.

Signed-off-by: Michael Lyle <mlyle@lyle.org>
Reported-by: Coly Li <colyli@suse.de>
---
 drivers/md/bcache/util.h      | 4 ++--
 drivers/md/bcache/writeback.c | 7 ++++---
 2 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/drivers/md/bcache/util.h b/drivers/md/bcache/util.h
index cb8d2ccbb6c6..8f509290bb02 100644
--- a/drivers/md/bcache/util.h
+++ b/drivers/md/bcache/util.h
@@ -441,10 +441,10 @@ struct bch_ratelimit {
 	uint64_t		next;
 
 	/*
-	 * Rate at which we want to do work, in units per nanosecond
+	 * Rate at which we want to do work, in units per second
 	 * The units here correspond to the units passed to bch_next_delay()
 	 */
-	unsigned		rate;
+	uint32_t		rate;
 };
 
 static inline void bch_ratelimit_reset(struct bch_ratelimit *d)
diff --git a/drivers/md/bcache/writeback.c b/drivers/md/bcache/writeback.c
index 3f8c4c6bee03..a1608c45bb12 100644
--- a/drivers/md/bcache/writeback.c
+++ b/drivers/md/bcache/writeback.c
@@ -46,7 +46,8 @@ static void __update_writeback_rate(struct cached_dev *dc)
 	int64_t error = dirty - target;
 	int64_t proportional_scaled =
 		div_s64(error, dc->writeback_rate_p_term_inverse);
-	int64_t integral_scaled, new_rate;
+	int64_t integral_scaled;
+	uint32_t new_rate;
 
 	if ((error < 0 && dc->writeback_rate_integral > 0) ||
 	    time_before64(local_clock(),
@@ -67,8 +68,8 @@ static void __update_writeback_rate(struct cached_dev *dc)
 	integral_scaled = div_s64(dc->writeback_rate_integral,
 			dc->writeback_rate_i_term_inverse);
 
-	new_rate = clamp_t(int64_t, (proportional_scaled + integral_scaled),
-			dc->writeback_rate_minimum, NSEC_PER_MSEC);
+	new_rate = clamp_t(uint32_t, (proportional_scaled + integral_scaled),
+			dc->writeback_rate_minimum, NSEC_PER_SEC);
 
 	dc->writeback_rate_proportional = proportional_scaled;
 	dc->writeback_rate_integral_scaled = integral_scaled;
-- 
2.11.0

Re: [PATCH] bcache: writeback rate shouldn't artifically clamp

[Coly Li]

On 2017/10/8 下午1:25, Michael Lyle wrote:
> The previous code artificially limited writeback rate to 1000000
> blocks/second (NSEC_PER_MSEC), which is a rate that can be met on fast
> hardware.  The rate limiting code works fine (though with decreased
> precision) up to 3 orders of magnitude faster, so use NSEC_PER_SEC.
> 
> Additionally, ensure that uint32_t is used as a type for rate throughout
> the rate management so that type checking/clamp_t can work properly.
> 
> bch_next_delay should be rewritten for increased precision and better
> handling of high rates and long sleep periods, but this is adequate for
> now.
> 
> Signed-off-by: Michael Lyle <mlyle@lyle.org>
> Reported-by: Coly Li <colyli@suse.de>

Added to for-test, I will update test result after the writeback
performance test done (hopefully in 2 days).

Thanks for fast fix.

Coly Li


[snip]

Re: [PATCH] bcache: writeback rate shouldn't artifically clamp

[Coly Li]

On 2017/10/8 下午3:50, Stefan Priebe - Profihost AG wrote:
> Is there a branch with all patches available?
> 
> Stefan

Hi Stefan,

Current I just organize patch files not a git tree branch. The reason
is, I send bcache patches by emails to Jens and Cc linux-bcache and
linux-block, in order to make more people notice the patches.

Currently the patches can be found here (today's change not pushed yet),
https://git.kernel.org/pub/scm/linux/kernel/git/colyli/bcache-patches.git/tree/

for-next: this directory contains patches under serious testing and for
next merge window
for-test: this directory contains all meaningful patches, I verify them
when I have time, they are candidate of for-next.

Michael is preparing a git tree at mean time, maybe we will take care
patches in form of git tree soon.

Thanks.

Coly Li

> 
> Excuse my typo sent from my mobile phone.
> 
> Am 08.10.2017 um 08:09 schrieb Coly Li <i@coly.li <mailto:i@coly.li>>:
> 
>> On 2017/10/8 下午1:25, Michael Lyle wrote:
>>> The previous code artificially limited writeback rate to 1000000
>>> blocks/second (NSEC_PER_MSEC), which is a rate that can be met on fast
>>> hardware.  The rate limiting code works fine (though with decreased
>>> precision) up to 3 orders of magnitude faster, so use NSEC_PER_SEC.
>>>
>>> Additionally, ensure that uint32_t is used as a type for rate throughout
>>> the rate management so that type checking/clamp_t can work properly.
>>>
>>> bch_next_delay should be rewritten for increased precision and better
>>> handling of high rates and long sleep periods, but this is adequate for
>>> now.
>>>
>>> Signed-off-by: Michael Lyle <mlyle@lyle.org <mailto:mlyle@lyle.org>>
>>> Reported-by: Coly Li <colyli@suse.de <mailto:colyli@suse.de>>
>>
>> Added to for-test, I will update test result after the writeback
>> performance test done (hopefully in 2 days).
>>
>> Thanks for fast fix.
>>
>> Coly Li
>>
>>
>> [snip]
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-bcache" in
>> the body of a message to majordomo@vger.kernel.org
>> <mailto:majordomo@vger.kernel.org>
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] bcache: writeback rate shouldn't artifically clamp

[Michael Lyle]

Stefan--

On Sun, Oct 8, 2017 at 12:50 AM, Stefan Priebe - Profihost AG
<s.priebe@profihost.ag> wrote:
> Is there a branch with all patches available?

I have a github repository up of linux-next, with commits currently
being staged for next on top, at https://github.com/mlyle/linux/ .

The specific branch contents can be seen at
https://github.com/mlyle/linux/commits/bcache-for-next ; the contents
are Coly's current list of patches for inclusion in next staged on top
of the present version of the next tree.  All the caveats that apply
to tracking linux-next apply to this branch.

This branch is ephemeral and used for staging and verification--
history will be rewritten (I'll rebase it on top of linux-next rather
than have lots of merge commits).  I'm getting together some
continuous integration infrastructure to aid automated test and
verification.

There will probably be other important branches on here.. e.g. a
"kitchensink" branch that includes all submitted patches, including
controversial ones / and ones with only light review.  Probably
sometimes it will make sense to be based on Linus's master, too.  I am
still figuring out my workflow.

> Stefan
>
> Excuse my typo sent from my mobile phone.

Mike