On Thu, 13 Jul 2017, Coly Li wrote:
> On 2017/7/13 下午12:12, Eric Wheeler wrote:
> > On Tue, 11 Jul 2017, tang.junhui@xxxxxxxxxx wrote:
> >
> >>> Based on the above implementation, non-dirty space from flash only
> >>> bcache device will mislead writeback rate calculation too. So I suggest
> >>> to subtract all buckets size from all flash only bcache devices. Then it
> >>> might be something like,
> >>
> >> what is non-dirty space from flash only bcache device?
> >> Where is non-dirty space from flash only bcache device?
> >
> > Hi Tang, Coly:
> >
> > Waas there more discussion on this thread, or is the patch good to go?
> >
> > Please send your ack if you're happy with it so I can queue it up.
>
> I discussed with Tang offline, this patch is correct. But the patch
> commit log should be improved. Now I help to work on it, should be done
> quite soon.
Has an updated commit log been made? I've not seen this in the commit
stream yet.
--
Eric Wheeler
>
> Coly
>
> >>
> >>
> >> 发件人: Coly Li <i@xxxxxxx>
> >> 收件人: Tang Junhui <tang.junhui@xxxxxxxxxx>,
> >> 抄送: bcache@xxxxxxxxxxxxxxxxxx, linux-block@xxxxxxxxxxxxxxx, linux-bcache@xxxxxxxxxxxxxxx,
> >> hch@xxxxxxxxxxxxx, axboe@xxxxxxxxx, stable@xxxxxxxxxxxxxxx
> >> 日期: 2017/07/11 02:11
> >> 主题: Re: [PATCH 11/19] bcache: Subtract dirty sectors of thin flash from cache_sectors in calculating
> >> writeback rate
> >> 发件人: linux-bcache-owner@xxxxxxxxxxxxxxx
> >>
> >> _________________________________________________________________________________________________________________
> >>
> >>
> >>
> >> On 2017/7/1 上午4:43, bcache@xxxxxxxxxxxxxxxxxx wrote:
> >>> From: Tang Junhui <tang.junhui@xxxxxxxxxx>
> >>>
> >>> Since dirty sectors of thin flash cannot be used to cache data for backend
> >>> device, so we should subtract it in calculating writeback rate.
> >>>
> >>
> >> I see you want to get ride of the noise of flash only cache device for
> >> writeback rate calculation. It makes sense, because flash only cache
> >> device won't have write back happen at all.
> >>
> >>
> >>> Signed-off-by: Tang Junhui <tang.junhui@xxxxxxxxxx>
> >>> Cc: stable@xxxxxxxxxxxxxxx
> >>> ---
> >>> drivers/md/bcache/writeback.c | 2 +-
> >>> drivers/md/bcache/writeback.h | 19 +++++++++++++++++++
> >>> 2 files changed, 20 insertions(+), 1 deletion(-)
> >>>
> >>> diff --git a/drivers/md/bcache/writeback.c b/drivers/md/bcache/writeback.c
> >>> index 4ac8b13..25289e4 100644
> >>> --- a/drivers/md/bcache/writeback.c
> >>> +++ b/drivers/md/bcache/writeback.c
> >>> @@ -21,7 +21,7 @@
> >>> static void __update_writeback_rate(struct cached_dev *dc)
> >>> {
> >>> struct cache_set *c = dc->disk.c;
> >>> - uint64_t cache_sectors = c->nbuckets * c->sb.bucket_size;
> >>> + uint64_t cache_sectors = c->nbuckets * c->sb.bucket_size -
> >> bcache_flash_devs_sectors_dirty(c);
> >>
> >> See flash_dev_run(), the flash volume is created per struct
> >> bcache_device of a cache set. That means, all data allocated for the
> >> flash volume will be from a flash only bcache device. Regular dirty data
> >> won't mixed allocating with flash volume dirty data on identical struct
> >> bcache device.
> >>
> >> Based on the above implementation, non-dirty space from flash only
> >> bcache device will mislead writeback rate calculation too. So I suggest
> >> to subtract all buckets size from all flash only bcache devices. Then it
> >> might be something like,
> >>
> >> uint64_t cache_sectors = c->nbuckets * c->sb.bucket_size -
> >> bcache_flash_devs_nbuckets(c);
> >>
> >>
> >>
> >> Just FYI. Thanks.
> >>
> >> Coly
> >>
> >>> uint64_t cache_dirty_target =
> >>> div_u64(cache_sectors * dc->writeback_percent, 100);
> >>>
> >>> diff --git a/drivers/md/bcache/writeback.h b/drivers/md/bcache/writeback.h
> >>> index c2ab4b4..24ff589 100644
> >>> --- a/drivers/md/bcache/writeback.h
> >>> +++ b/drivers/md/bcache/writeback.h
> >>> @@ -14,6 +14,25 @@ static inline uint64_t bcache_dev_sectors_dirty(struct bcache_device *d)
> >>> return ret;
> >>> }
> >>>
> >>> +static inline uint64_t bcache_flash_devs_sectors_dirty(struct cache_set *c)
> >>> +{
> >>> + uint64_t i, ret = 0;
> >>> +
> >>> + mutex_lock(&bch_register_lock);
> >>> +
> >>> + for (i = 0; i < c->nr_uuids; i++) {
> >>> + struct bcache_device *d = c->devices[i];
> >>> +
> >>> + if (!d || !UUID_FLASH_ONLY(&c->uuids[i]))
> >>> + continue;
> >>> + ret += bcache_dev_sectors_dirty(d);
> >>> + }
> >>> +
> >>> + mutex_unlock(&bch_register_lock);
> >>> +
> >>> + return ret;
> >>> +}
> >>> +
> >>> static inline unsigned offset_to_stripe(struct bcache_device *d,
> >>> uint64_t offset)
> >>> {
> >>>
> >> --
> >> To unsubscribe from this list: send the line "unsubscribe linux-bcache" in
> >> the body of a message to majordomo@xxxxxxxxxxxxxxx
> >> More majordomo info at http://vger.kernel.org/majordomo-info.html
> >>
> >>
> >>
>
>
> --
> Coly Li
> --
> To unsubscribe from this list: send the line "unsubscribe linux-bcache" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>