On Tue, Oct 15, 2019 at 03:12:17PM +0200, David Sterba wrote: > On Mon, Oct 07, 2019 at 04:17:39PM -0400, Dennis Zhou wrote: > > The number of discardable extents will serve as the rate limiting metric > > for how often we should discard. This keeps track of discardable extents > > in the free space caches by maintaining deltas and propagating them to > > the global count. > > > > This also setups up a discard directory in btrfs sysfs and exports the > > total discard_extents count. > > Please put the discard directory under debug/ for now. >
Just double checking, but you mean to have it be: /sys/fs/btrfs/<uuid>/debug/discard/*? > > Signed-off-by: Dennis Zhou <[email protected]> > > --- > > fs/btrfs/ctree.h | 2 + > > fs/btrfs/discard.c | 2 + > > fs/btrfs/discard.h | 19 ++++++++ > > fs/btrfs/free-space-cache.c | 93 ++++++++++++++++++++++++++++++++++--- > > fs/btrfs/free-space-cache.h | 2 + > > fs/btrfs/sysfs.c | 33 +++++++++++++ > > 6 files changed, 144 insertions(+), 7 deletions(-) > > > > diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h > > index c328d2e85e4d..43e515939b9c 100644 > > --- a/fs/btrfs/ctree.h > > +++ b/fs/btrfs/ctree.h > > @@ -447,6 +447,7 @@ struct btrfs_discard_ctl { > > spinlock_t lock; > > struct btrfs_block_group_cache *cache; > > struct list_head discard_list[BTRFS_NR_DISCARD_LISTS]; > > + atomic_t discard_extents; > > At the end of the series this becomes > > 452 atomic_t discard_extents; > 453 atomic64_t discardable_bytes; > 454 atomic_t delay; > 455 atomic_t iops_limit; > 456 atomic64_t bps_limit; > 457 atomic64_t discard_extent_bytes; > 458 atomic64_t discard_bitmap_bytes; > 459 atomic64_t discard_bytes_saved; > > raising many eyebrows. What's the reason to use so many atomics? As this > is purely for accounting and perhaps not contended, add one spinlock > protecting all of them. > > None of delay, bps_limit and iops_limit use the atomict_t semantics at > all, it's just _set and _read. > > As this seem to cascade to all other patches, I'll postpone my review > until I see V2. Yeah... I think the following 3 would be nice to keep as atomics as the first two are propagated per block group and are protected via the free_space_ctl's lock. Then multiple allocations can go through concurrently. discard_bytes_saved is also something that can be incremented by multiple block groups at once for the same reason, so an atomic makes life simple. > 452 atomic_t discard_extents; > 453 atomic64_t discardable_bytes; > 459 atomic64_t discard_bytes_saved; The others I'll flip over to the proper type. Thanks, Dennis
