On Wed, Feb 12, 2025 at 03:41:54PM +0800, Julian Sun wrote:
> Hi, mingzhe
>
> Thanks for your review and comments.
>
> On Wed, Feb 12, 2025 at 3:06 PM 邹明哲 <mingzhe.zou@xxxxxxxxxxxx> wrote:
> >
> > Original:
> > From：Julian Sun <sunjunchao2870@xxxxxxxxx>
> > Date：2025-02-12 13:51:26(中国 (GMT+08:00))
> > To：linux-bcache<linux-bcache@xxxxxxxxxxxxxxx>
> > Cc：colyli<colyli@xxxxxxxxxx> , kent.overstreet<kent.overstreet@xxxxxxxxx> , Julian Sun <sunjunchao2870@xxxxxxxxx>
> > Subject：[PATCH] bcache: Use the lastest writeback_delay value when writeback thread is woken up
> > When users reset writeback_delay value and woke up writeback
> > thread via sysfs interface, expect the writeback thread
> > to do actual writeback work, but in reality, the writeback
> > thread probably continue to sleep.
> >
> > For example the following script set writeback_delay to 0 and
> > wake up writeback thread, but writeback thread just continue to
> > sleep:
> > echo 0 > /sys/block/bcache0/bcache/writeback_delay
> > echo 1 > /sys/block/bcache0/bcache/writeback_running
> >
> > Using the lastest value when writeback thread is woken up can
> > urge it to do actual writeback work.
> >
> > Signed-off-by: Julian Sun <sunjunchao2870@xxxxxxxxx>
> > ---
> > drivers/md/bcache/writeback.c | 4 +++-
> > 1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/md/bcache/writeback.c b/drivers/md/bcache/writeback.c
> > index c1d28e365910..0d2d06aaacfe 100644
> > --- a/drivers/md/bcache/writeback.c
> > +++ b/drivers/md/bcache/writeback.c
> > @@ -825,8 +825,10 @@ static int bch_writeback_thread(void *arg)
> >
> > Hi, Julian Sun:
> >
> > We should first understand the role of writable_delay.
> >
> > The writeback thread only sleep when searched_full_index is True,
> > which means that there are very few dirty keys at this time, all
> > dirty keys are refilled at once.
> >
> > while (delay &&
> > !kthread_should_stop() &&
> > !test_bit(CACHE_SET_IO_DISABLE, &c->flags) &&
> > - !test_bit(BCACHE_DEV_DETACHING, &dc->disk.flags))
> > + !test_bit(BCACHE_DEV_DETACHING, &dc->disk.flags)) {
> > delay = schedule_timeout_interruptible(delay);
> > + delay = min(delay, dc->writeback_delay * HZ);
> > + }
> >
> >
> > > So, I don't think it is necessary to immediately adjust the sleep time
> > > unless the writeback_delay is set very large. We need to set a reasonable
> > > value for writable_delay at startup, rather than adjusting it at runtime.
> > >
>
> I understand your point, but I still believe this is important.
> IMO, since /sys/block/bcacheX/bcache/writeback_delay allows adjusting
> the writeback_delay value at runtime, bcache should ideally support
> this functionality. Otherwise, the current behavior may be confusing
> for users: "I've adjusted it, but why does it seem ineffective?" :)
>
Hi Julian,
Correct me if I am wrong. I feel maybe that you misunderstodd the
functionality of sysfs parameter writeback_delay. It is NOT used to
control the writeback rate.
You may want to check Documentation/admin-guide/bcache.rst at,
468 writeback_delay
469 When dirty data is written to the cache and it previously did not contain
470 any, waits some number of seconds before initiating writeback. Defaults to
471 30.
Thanks.
--
Coly Li
