Kernel/linux
1
0
Fork 0
mirror of https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git synced 2025-01-06 05:13:18 +00:00
Code Issues Actions Packages Projects Releases Wiki Activity

bcache: fix set_at_max_writeback_rate() for multiple attached devices

Browse Source 
Inside set_at_max_writeback_rate() the calculation in following if()
check is wrong,
	if (atomic_inc_return(&c->idle_counter) <
	    atomic_read(&c->attached_dev_nr) * 6)

Because each attached backing device has its own writeback thread
running and increasing c->idle_counter, the counter increates much
faster than expected. The correct calculation should be,
	(counter / dev_nr) < dev_nr * 6
which equals to,
	counter < dev_nr * dev_nr * 6

This patch fixes the above mistake with correct calculation, and helper
routine idle_counter_exceeded() is added to make code be more clear.

Reported-by: Mingzhe Zou <mingzhe.zou@easystack.cn>
Signed-off-by: Coly Li <colyli@suse.de>
Acked-by: Mingzhe Zou <mingzhe.zou@easystack.cn>
Link: https://lore.kernel.org/r/20220919161647.81238-6-colyli@suse.de
Signed-off-by: Jens Axboe <axboe@kernel.dk>
This commit is contained in:
Coly Li 2022-09-20 00:16:47 +08:00 committed by Jens Axboe
parent 6dd3be6923
commit d2d05b8803
1 changed files with 52 additions and 21 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

73
drivers/md/bcache/writeback.c
View File

@ -157,6 +157,53 @@ static void __update_writeback_rate(struct cached_dev *dc)
dc->writeback_rate_target = target;
}
static bool idle_counter_exceeded(struct cache_set *c)
{
int counter, dev_nr;
/*
* If c->idle_counter is overflow (idel for really long time),
* reset as 0 and not set maximum rate this time for code
* simplicity.
*/
counter = atomic_inc_return(&c->idle_counter);
if (counter <= 0) {
atomic_set(&c->idle_counter, 0);
return false;
}
dev_nr = atomic_read(&c->attached_dev_nr);
if (dev_nr == 0)
return false;
/*
* c->idle_counter is increased by writeback thread of all
* attached backing devices, in order to represent a rough
* time period, counter should be divided by dev_nr.
* Otherwise the idle time cannot be larger with more backing
* device attached.
* The following calculation equals to checking
* (counter / dev_nr) < (dev_nr * 6)
*/
if (counter < (dev_nr * dev_nr * 6))
return false;
return true;
}
/*
* Idle_counter is increased every time when update_writeback_rate() is
* called. If all backing devices attached to the same cache set have
* identical dc->writeback_rate_update_seconds values, it is about 6
* rounds of update_writeback_rate() on each backing device before
* c->at_max_writeback_rate is set to 1, and then max wrteback rate set
* to each dc->writeback_rate.rate.
* In order to avoid extra locking cost for counting exact dirty cached
* devices number, c->attached_dev_nr is used to calculate the idle
* throushold. It might be bigger if not all cached device are in write-
* back mode, but it still works well with limited extra rounds of
* update_writeback_rate().
*/
static bool set_at_max_writeback_rate(struct cache_set *c,
struct cached_dev *dc)
{
@ -167,21 +214,8 @@ static bool set_at_max_writeback_rate(struct cache_set *c,
/* Don't set max writeback rate if gc is running */
if (!c->gc_mark_valid)
return false;
/*
* Idle_counter is increased everytime when update_writeback_rate() is
* called. If all backing devices attached to the same cache set have
* identical dc->writeback_rate_update_seconds values, it is about 6
* rounds of update_writeback_rate() on each backing device before
* c->at_max_writeback_rate is set to 1, and then max wrteback rate set
* to each dc->writeback_rate.rate.
* In order to avoid extra locking cost for counting exact dirty cached
* devices number, c->attached_dev_nr is used to calculate the idle
* throushold. It might be bigger if not all cached device are in write-
* back mode, but it still works well with limited extra rounds of
* update_writeback_rate().
*/
if (atomic_inc_return(&c->idle_counter) <
atomic_read(&c->attached_dev_nr) * 6)
if (!idle_counter_exceeded(c))
return false;
if (atomic_read(&c->at_max_writeback_rate) != 1)
@ -195,13 +229,10 @@ static bool set_at_max_writeback_rate(struct cache_set *c,
dc->writeback_rate_change = 0;
/*
* Check c->idle_counter and c->at_max_writeback_rate agagain in case
* new I/O arrives during before set_at_max_writeback_rate() returns.
* Then the writeback rate is set to 1, and its new value should be
* decided via __update_writeback_rate().
* In case new I/O arrives during before
* set_at_max_writeback_rate() returns.
*/
if ((atomic_read(&c->idle_counter) <
atomic_read(&c->attached_dev_nr) * 6) ||
if (!idle_counter_exceeded(c) ||
!atomic_read(&c->at_max_writeback_rate))
return false;