On 11/15/18 11:08 AM, Shenghui Wang wrote: > A fresh backing device is not attached to any cache_set, and > has no writeback kthread created until first attached to some > cache_set. > > But bch_cached_dev_writeback_init run > " > dc->writeback_running = true; > WARN_ON(test_and_clear_bit(BCACHE_DEV_WB_RUNNING, > &dc->disk.flags)); > " > for any newly formatted backing devices. > > For a fresh standalone backing device, we can get something like > following even if no writeback kthread created: > ------------------------ > /sys/block/bcache0/bcache# cat writeback_running > 1 > /sys/block/bcache0/bcache# cat writeback_rate_debug > rate: 512.0k/sec > dirty: 0.0k > target: 0.0k > proportional: 0.0k > integral: 0.0k > change: 0.0k/sec > next io: -15427384ms > > The none ZERO fields are misleading as no alive writeback kthread yet. > > Set dc->writeback_running to false as no writeback thread created in > bch_cached_dev_writeback_init(). > We have writeback thread created and woken up in bch_cached_dev_writeback > _start(). Set dc->writeback_running to true after bch_writeback_queue() > called. > > After the change, we can get the following output for a fresh standalone > backing device: > ----------------------- > /sys/block/bcache0/bcache$ cat writeback_running > 0 > /sys/block/bcache0/bcache# cat writeback_rate_debug > rate: 0.0k/sec > dirty: 0.0k > target: 0.0k > proportional: 0.0k > integral: 0.0k > change: 0.0k/sec > next io: 0ms > > Signed-off-by: Shenghui Wang <shhuiw@xxxxxxxxxxx> > --- > drivers/md/bcache/writeback.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/drivers/md/bcache/writeback.c b/drivers/md/bcache/writeback.c > index 08c3a9f9676c..81263bbc1608 100644 > --- a/drivers/md/bcache/writeback.c > +++ b/drivers/md/bcache/writeback.c > @@ -777,7 +777,7 @@ void bch_cached_dev_writeback_init(struct cached_dev *dc) > bch_keybuf_init(&dc->writeback_keys); > > dc->writeback_metadata = true; > - dc->writeback_running = true; > + dc->writeback_running = false; > dc->writeback_percent = 10; > dc->writeback_delay = 30; > atomic_long_set(&dc->writeback_rate.rate, 1024); > @@ -811,6 +811,7 @@ int bch_cached_dev_writeback_start(struct cached_dev *dc) > dc->writeback_rate_update_seconds * HZ); > > bch_writeback_queue(dc); > + dc->writeback_running = true; > Hi Shenghui, I see a potential locking issue from the above change. dc->writeback_running should be set to true before bch_writeback_queue() called. I am happy with the idea of the patches, but still suggest you doing more test for the changes. Thanks. Coly Li > return 0; > } >
