Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 81247850 authored by Coly Li's avatar Coly Li Committed by Greg Kroah-Hartman
Browse files

bcache: fix set_at_max_writeback_rate() for multiple attached devices 



[ Upstream commit d2d05b88035d2d51a5bb6c5afec88a0880c73df4 ]

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: default avatarMingzhe Zou <mingzhe.zou@easystack.cn>
Signed-off-by: default avatarColy Li <colyli@suse.de>
Acked-by: default avatarMingzhe Zou <mingzhe.zou@easystack.cn>
Link: https://lore.kernel.org/r/20220919161647.81238-6-colyli@suse.de


Signed-off-by: default avatarJens Axboe <axboe@kernel.dk>
Signed-off-by: default avatarSasha Levin <sashal@kernel.org>
parent 7cfc77f4

drivers/md/bcache/writeback.c

+52 −21
Original line number Diff line number Diff line
@@ -119,12 +119,40 @@ static void __update_writeback_rate(struct cached_dev *dc) 
	dc->writeback_rate_target = target;

}



static bool set_at_max_writeback_rate(struct cache_set *c,

				       struct cached_dev *dc)

static bool idle_counter_exceeded(struct cache_set *c)

{

	/* Don't set max writeback rate if gc is running */

	if (!c->gc_mark_valid)

	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
@@ -138,8 +166,14 @@ static bool set_at_max_writeback_rate(struct cache_set *c, 
 * 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)

static bool set_at_max_writeback_rate(struct cache_set *c,

				       struct cached_dev *dc)

{

	/* Don't set max writeback rate if gc is running */

	if (!c->gc_mark_valid)

		return false;



	if (!idle_counter_exceeded(c))

		return false;



	if (atomic_read(&c->at_max_writeback_rate) != 1)
@@ -153,13 +187,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;