Hi Mingzhe,
On Tue, Aug 22, 2023 at 06:19:58PM +0800, Mingzhe Zou wrote:
> We found that after long run, the dirty_data of the bcache device
> will have errors. This error cannot be eliminated unless re-register.
Could you explain what the error was?
>
> We also found that reattach after detach, this error can accumulate.
>
Could you elaborate how the error can accumulate?
> In bch_sectors_dirty_init(), all inode <= d->id keys will be recounted
> again. This is wrong, we only need to count the keys of the current
> device.
>
> Fixes: b144e45fc576 ("bcache: make bch_sectors_dirty_init() to be multithreaded")
> Signed-off-by: Mingzhe Zou <mingzhe.zou@xxxxxxxxxxxx>
> ---
> drivers/md/bcache/writeback.c | 7 ++++++-
> 1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/md/bcache/writeback.c b/drivers/md/bcache/writeback.c
> index 24c049067f61..71d0dabcbf9d 100644
> --- a/drivers/md/bcache/writeback.c
> +++ b/drivers/md/bcache/writeback.c
> @@ -983,6 +983,8 @@ void bch_sectors_dirty_init(struct bcache_device *d)
> struct cache_set *c = d->c;
> struct bch_dirty_init_state state;
>
> + atomic_long_set(&d->dirty_sectors, 0);
> +
The above change is not upstreamed yet, if you are fixing upstream code please
avoid to use d->dirty_sectors here.
> /* Just count root keys if no leaf node */
> rw_lock(0, c->root, c->root->level);
> if (c->root->level == 0) {
> @@ -991,8 +993,11 @@ void bch_sectors_dirty_init(struct bcache_device *d)
> op.count = 0;
>
> for_each_key_filter(&c->root->keys,
> - k, &iter, bch_ptr_invalid)
> + k, &iter, bch_ptr_invalid) {
> + if (KEY_INODE(k) != op.inode)
> + continue;
> sectors_dirty_init_fn(&op.op, c->root, k);
> + }
>
Nice catch! IMHO if I take the above change, setting d->dirty_sectors by 0
might be unncessary in ideal condition, am I right?
Thanks for the fixup.
> rw_unlock(0, c->root);
> return;
> --
> 2.17.1.windows.2
>
--
Coly Li
