Re: [PATCH v2 09/11] btrfs: implement space clamping for preemptive flushing

From: Nikolay Borisov <nborisov@suse.com>
To: Josef Bacik <josef@toxicpanda.com>,
	linux-btrfs@vger.kernel.org, kernel-team@fb.com
Subject: Re: [PATCH v2 09/11] btrfs: implement space clamping for preemptive flushing
Date: Fri, 9 Oct 2020 15:22:45 +0300	[thread overview]
Message-ID: <f5156e78-08c5-caae-435b-3e182292460b@suse.com> (raw)
In-Reply-To: <501bfeaa3dc6f4c59dd6062f6108bab974316b85.1602189832.git.josef@toxicpanda.com>


On 8.10.20 г. 23:48 ч., Josef Bacik wrote:
> Starting preemptive flushing at 50% of available free space is a good
> start, but some workloads are particularly abusive and can quickly
> overwhelm the preemptive flushing code and drive us into using tickets.
> 
> Handle this by clamping down on our threshold for starting and
> continuing to run preemptive flushing.  This is particularly important
> for our overcommit case, as we can really drive the file system into
> overages and then it's more difficult to pull it back as we start to
> actually fill up the file system.
> 
> The clamping is essentially 2^CLAMP, but we start at 1 so whatever we
> calculate for overcommit is the baseline.
> 

<snip>

> +static inline void maybe_clamp_preempt(struct btrfs_fs_info *fs_info,
> +				       struct btrfs_space_info *space_info)
> +{
> +	u64 ordered, delalloc;
> +
> +	ordered = percpu_counter_sum_positive(&fs_info->ordered_bytes);
> +	delalloc = percpu_counter_sum_positive(&fs_info->delalloc_bytes);

nit: This can easily be rewritten to :

 u64 ordered = percpu_counter_sum_positive(&fs_info->ordered_bytes);
 u64 delalloc =  percpu_counter_sum_positive(&fs_info->delalloc_bytes);

as it doesn't cross the 80 chars boundary, no need for the extra 2 lines :)

> +
> +	/*
> +	 * If we're heavy on ordered operations then clamping won't help us.  We
> +	 * need to clamp specifically to keep up with dirty'ing buffered
> +	 * writers, because there's not a 1:1 correlation of writing delalloc
> +	 * and freeing space, like there is with flushing delayed refs or
> +	 * delayed nodes.  If we're already more ordered than delalloc then
> +	 * we're keeping up, otherwise we aren't and should probably clamp.
> +	 */
> +	if (ordered < delalloc)
> +		space_info->clamp = min(space_info->clamp + 1, 8);
> +}

<snip>