Re: [PATCH 11/19] btrfs: add bps discard rate limit

[Josef Bacik <josef@toxicpanda.com>]

On Mon, Oct 07, 2019 at 04:17:42PM -0400, Dennis Zhou wrote:
> Provide an ability to rate limit based on mbps in addition to the iops
> delay calculated from number of discardable extents.
> 
> Signed-off-by: Dennis Zhou <dennis@kernel.org>
> ---
>  fs/btrfs/ctree.h   |  2 ++
>  fs/btrfs/discard.c | 11 +++++++++++
>  fs/btrfs/sysfs.c   | 30 ++++++++++++++++++++++++++++++
>  3 files changed, 43 insertions(+)
> 
> diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
> index b0823961d049..e81f699347e0 100644
> --- a/fs/btrfs/ctree.h
> +++ b/fs/btrfs/ctree.h
> @@ -447,10 +447,12 @@ struct btrfs_discard_ctl {
>  	spinlock_t lock;
>  	struct btrfs_block_group_cache *cache;
>  	struct list_head discard_list[BTRFS_NR_DISCARD_LISTS];
> +	u64 prev_discard;
>  	atomic_t discard_extents;
>  	atomic64_t discardable_bytes;
>  	atomic_t delay;
>  	atomic_t iops_limit;
> +	atomic64_t bps_limit;
>  };
>  
>  /* delayed seq elem */
> diff --git a/fs/btrfs/discard.c b/fs/btrfs/discard.c
> index c7afb5f8240d..072c73f48297 100644
> --- a/fs/btrfs/discard.c
> +++ b/fs/btrfs/discard.c
> @@ -176,6 +176,13 @@ void btrfs_discard_schedule_work(struct btrfs_discard_ctl *discard_ctl,
>  	cache = find_next_cache(discard_ctl, now);
>  	if (cache) {
>  		u64 delay = atomic_read(&discard_ctl->delay);
> +		s64 bps_limit = atomic64_read(&discard_ctl->bps_limit);
> +
> +		if (bps_limit)
> +			delay = max_t(u64, delay,
> +				      msecs_to_jiffies(MSEC_PER_SEC *
> +						discard_ctl->prev_discard /
> +						bps_limit));

I forget, are we allowed to do 0 / some value?  I feel like I did this at some
point with io.latency and it panic'ed and was very confused.  Maybe I'm just
misremembering.

And a similar nit, maybe we just do

u64 delay = atomic_read(&discard_ctl->delay);
u64 bps_delay = atomic64_read(&discard_ctl->bps_limit);
if (bps_delay)
	bps_delay = msecs_to_jiffies(MSEC_PER_SEC * blah)

delay = max(delay, bps_delay);

Or something else.  Thanks,

Josef