diff mbox series

[4/9] btrfs: check reclaim_size in need_preemptive_reclaim

Message ID 855a8376fa0d8e63e066ac323a985fe7bc1e562f.1601495426.git.josef@toxicpanda.com
State New, archived
Headers show
Series Improve preemptive ENOSPC flushing | expand

Commit Message

Josef Bacik Sept. 30, 2020, 8:01 p.m. UTC
If we're flushing space for tickets then we have
space_info->reclaim_size set and we do not need to do background
reclaim.

Signed-off-by: Josef Bacik <josef@toxicpanda.com>
---
 fs/btrfs/space-info.c | 7 +++++++
 1 file changed, 7 insertions(+)

Comments

Nikolay Borisov Oct. 1, 2020, 1:23 p.m. UTC | #1
On 30.09.20 г. 23:01 ч., Josef Bacik wrote:
> If we're flushing space for tickets then we have
> space_info->reclaim_size set and we do not need to do background
> reclaim.
> 
> Signed-off-by: Josef Bacik <josef@toxicpanda.com>

I'm fine with this but check my remak below.

Reviewed-by: Nikolay Borisov <nborisov@suse.com>
> ---
>  fs/btrfs/space-info.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/fs/btrfs/space-info.c b/fs/btrfs/space-info.c
> index 98207ea57a3d..518749093bc5 100644
> --- a/fs/btrfs/space-info.c
> +++ b/fs/btrfs/space-info.c
> @@ -805,6 +805,13 @@ static inline int need_preemptive_reclaim(struct btrfs_fs_info *fs_info,
>  	if ((space_info->bytes_used + space_info->bytes_reserved) >= thresh)
>  		return 0;
>  
> +	/*
> +	 * We have tickets queued, bail so we don't compete with the async
> +	 * flushers.
> +	 */
> +	if (space_info->reclaim_size)
> +		return 0;
> +

nit: So where do we draw the line if this check should be in this
function or in __reserve_bytes so that we eliminate the case where a
preemptive reclaim is scheduled only to return instantly because of
tickets. Though the 'if' in __reserve_bytes is getting slitghly out of
hand :)

>  	if (!btrfs_calc_reclaim_metadata_size(fs_info, space_info))
>  		return 0;
>  
>
Josef Bacik Oct. 1, 2020, 9:36 p.m. UTC | #2
On 10/1/20 9:23 AM, Nikolay Borisov wrote:
> 
> 
> On 30.09.20 г. 23:01 ч., Josef Bacik wrote:
>> If we're flushing space for tickets then we have
>> space_info->reclaim_size set and we do not need to do background
>> reclaim.
>>
>> Signed-off-by: Josef Bacik <josef@toxicpanda.com>
> 
> I'm fine with this but check my remak below.
> 
> Reviewed-by: Nikolay Borisov <nborisov@suse.com>
>> ---
>>   fs/btrfs/space-info.c | 7 +++++++
>>   1 file changed, 7 insertions(+)
>>
>> diff --git a/fs/btrfs/space-info.c b/fs/btrfs/space-info.c
>> index 98207ea57a3d..518749093bc5 100644
>> --- a/fs/btrfs/space-info.c
>> +++ b/fs/btrfs/space-info.c
>> @@ -805,6 +805,13 @@ static inline int need_preemptive_reclaim(struct btrfs_fs_info *fs_info,
>>   	if ((space_info->bytes_used + space_info->bytes_reserved) >= thresh)
>>   		return 0;
>>   
>> +	/*
>> +	 * We have tickets queued, bail so we don't compete with the async
>> +	 * flushers.
>> +	 */
>> +	if (space_info->reclaim_size)
>> +		return 0;
>> +
> 
> nit: So where do we draw the line if this check should be in this
> function or in __reserve_bytes so that we eliminate the case where a
> preemptive reclaim is scheduled only to return instantly because of
> tickets. Though the 'if' in __reserve_bytes is getting slitghly out of
> hand :)
> 

Keep in mind this is also used by the background flushing thread to tell when 
it's time to stop flushing, which is why the check is in here.  Thanks,

Josef
diff mbox series

Patch

diff --git a/fs/btrfs/space-info.c b/fs/btrfs/space-info.c
index 98207ea57a3d..518749093bc5 100644
--- a/fs/btrfs/space-info.c
+++ b/fs/btrfs/space-info.c
@@ -805,6 +805,13 @@  static inline int need_preemptive_reclaim(struct btrfs_fs_info *fs_info,
 	if ((space_info->bytes_used + space_info->bytes_reserved) >= thresh)
 		return 0;
 
+	/*
+	 * We have tickets queued, bail so we don't compete with the async
+	 * flushers.
+	 */
+	if (space_info->reclaim_size)
+		return 0;
+
 	if (!btrfs_calc_reclaim_metadata_size(fs_info, space_info))
 		return 0;