[15/23] btrfs: use ticketing for data space reservations
diff mbox series

Message ID 20200630135921.745612-16-josef@toxicpanda.com
State New
Headers show
Series
  • Change data reservations to use the ticketing infra
Related show

Commit Message

Josef Bacik June 30, 2020, 1:59 p.m. UTC
Now that we have all the infrastructure in place, use the ticketing
infrastructure to make data allocations.  This still maintains the exact
same flushing behavior, but now we're using tickets to get our
reservations satisfied.

Reviewed-by: Nikolay Borisov <nborisov@suse.com>
Tested-by: Nikolay Borisov <nborisov@suse.com>
Signed-off-by: Josef Bacik <josef@toxicpanda.com>
---
 fs/btrfs/space-info.c | 125 ++++++++++++++++++++++--------------------
 1 file changed, 67 insertions(+), 58 deletions(-)

Comments

Nikolay Borisov July 7, 2020, 2:46 p.m. UTC | #1
On 30.06.20 г. 16:59 ч., Josef Bacik wrote:
> Now that we have all the infrastructure in place, use the ticketing
> infrastructure to make data allocations.  This still maintains the exact
> same flushing behavior, but now we're using tickets to get our
> reservations satisfied.
> 
> Reviewed-by: Nikolay Borisov <nborisov@suse.com>
> Tested-by: Nikolay Borisov <nborisov@suse.com>
> Signed-off-by: Josef Bacik <josef@toxicpanda.com>
> ---
>  fs/btrfs/space-info.c | 125 ++++++++++++++++++++++--------------------
>  1 file changed, 67 insertions(+), 58 deletions(-)
> 
> diff --git a/fs/btrfs/space-info.c b/fs/btrfs/space-info.c
> index 799ee6090693..ee4747917b81 100644
> --- a/fs/btrfs/space-info.c
> +++ b/fs/btrfs/space-info.c
> @@ -1068,6 +1068,54 @@ static void priority_reclaim_metadata_space(struct btrfs_fs_info *fs_info,
>  	} while (flush_state < states_nr);
>  }
>  
> +static void priority_reclaim_data_space(struct btrfs_fs_info *fs_info,
> +					struct btrfs_space_info *space_info,
> +					struct reserve_ticket *ticket,
> +					const enum btrfs_flush_state *states,
> +					int states_nr)
> +{
> +	int flush_state = 0;
> +	int commit_cycles = 2;
> +
> +	while (!space_info->full) {
> +		flush_space(fs_info, space_info, U64_MAX, ALLOC_CHUNK_FORCE);
> +		spin_lock(&space_info->lock);
> +		if (ticket->bytes == 0) {
> +			spin_unlock(&space_info->lock);
> +			return;
> +		}
> +		spin_unlock(&space_info->lock);
> +	}
> +again:
> +	while (flush_state < states_nr) {
> +		u64 flush_bytes = U64_MAX;
> +
> +		if (!commit_cycles) {
> +			if (states[flush_state] == FLUSH_DELALLOC_WAIT) {
> +				flush_state++;
> +				continue;
> +			}
> +			if (states[flush_state] == COMMIT_TRANS)
> +				flush_bytes = ticket->bytes;
> +		}
> +
> +		flush_space(fs_info, space_info, flush_bytes,
> +			    states[flush_state]);
> +		spin_lock(&space_info->lock);
> +		if (ticket->bytes == 0) {
> +			spin_unlock(&space_info->lock);
> +			return;
> +		}
> +		spin_unlock(&space_info->lock);
> +		flush_state++;
> +	}
> +	if (commit_cycles) {
> +		commit_cycles--;
> +		flush_state = 0;
> +		goto again;
> +	}
> +}
> +
>  static void wait_reserve_ticket(struct btrfs_fs_info *fs_info,
>  				struct btrfs_space_info *space_info,
>  				struct reserve_ticket *ticket)
> @@ -1134,6 +1182,15 @@ static int handle_reserve_ticket(struct btrfs_fs_info *fs_info,
>  						evict_flush_states,
>  						ARRAY_SIZE(evict_flush_states));
>  		break;
> +	case BTRFS_RESERVE_FLUSH_DATA:
> +		priority_reclaim_data_space(fs_info, space_info, ticket,
> +					data_flush_states,
> +					ARRAY_SIZE(data_flush_states));
> +		break;
> +	case BTRFS_RESERVE_FLUSH_FREE_SPACE_INODE:
> +		priority_reclaim_data_space(fs_info, space_info, ticket,
> +					    NULL, 0);
> +		break;
>  	default:
>  		ASSERT(0);
>  		break;
> @@ -1341,78 +1398,30 @@ int btrfs_reserve_data_bytes(struct btrfs_fs_info *fs_info, u64 bytes,
>  			     enum btrfs_reserve_flush_enum flush)
>  {
>  	struct btrfs_space_info *data_sinfo = fs_info->data_sinfo;
> -	const enum btrfs_flush_state *states = NULL;
>  	u64 used;
> -	int states_nr = 0;
> -	int commit_cycles = 2;
>  	int ret = -ENOSPC;
>  
>  	ASSERT(!current->journal_info || flush != BTRFS_RESERVE_FLUSH_DATA);
>  
> -	if (flush == BTRFS_RESERVE_FLUSH_DATA) {
> -		states = data_flush_states;
> -		states_nr = ARRAY_SIZE(data_flush_states);
> -	}
> -
>  	spin_lock(&data_sinfo->lock);
> -again:
>  	used = btrfs_space_info_used(data_sinfo, true);
>  
>  	if (used + bytes > data_sinfo->total_bytes) {
> -		u64 prev_total_bytes = data_sinfo->total_bytes;
> -		int flush_state = 0;
> +		struct reserve_ticket ticket;
>  
> +		init_waitqueue_head(&ticket.wait);
> +		ticket.bytes = bytes;
> +		ticket.error = 0;
> +		list_add_tail(&ticket.list, &data_sinfo->priority_tickets);

nit: Shouldn't adding the ticket also be recorded in
spac_info->reclaim_size?
I see later that you are removing this code and relying on the existing
logic in __reserve_metadata_bytes( renamed to reserve_bytes) which
correctly modifies reclaim_size, but this just means this particular
patch is slightly broken.

>  		spin_unlock(&data_sinfo->lock);
>  
> -		/*
> -		 * Everybody can force chunk allocation, so try this first to
> -		 * see if we can just bail here and make our reservation.
> -		 */
> -		flush_space(fs_info, data_sinfo, bytes, ALLOC_CHUNK_FORCE);
> -		spin_lock(&data_sinfo->lock);
> -		if (prev_total_bytes < data_sinfo->total_bytes)
> -			goto again;
> +		ret = handle_reserve_ticket(fs_info, data_sinfo, &ticket,
> +					    flush);
> +	} else {
> +		btrfs_space_info_update_bytes_may_use(fs_info, data_sinfo, bytes);
> +		ret = 0;

<snip>
Josef Bacik July 7, 2020, 2:56 p.m. UTC | #2
On 7/7/20 10:46 AM, Nikolay Borisov wrote:
> 
> 
> On 30.06.20 г. 16:59 ч., Josef Bacik wrote:
>> Now that we have all the infrastructure in place, use the ticketing
>> infrastructure to make data allocations.  This still maintains the exact
>> same flushing behavior, but now we're using tickets to get our
>> reservations satisfied.
>>
>> Reviewed-by: Nikolay Borisov <nborisov@suse.com>
>> Tested-by: Nikolay Borisov <nborisov@suse.com>
>> Signed-off-by: Josef Bacik <josef@toxicpanda.com>
>> ---
>>   fs/btrfs/space-info.c | 125 ++++++++++++++++++++++--------------------
>>   1 file changed, 67 insertions(+), 58 deletions(-)
>>
>> diff --git a/fs/btrfs/space-info.c b/fs/btrfs/space-info.c
>> index 799ee6090693..ee4747917b81 100644
>> --- a/fs/btrfs/space-info.c
>> +++ b/fs/btrfs/space-info.c
>> @@ -1068,6 +1068,54 @@ static void priority_reclaim_metadata_space(struct btrfs_fs_info *fs_info,
>>   	} while (flush_state < states_nr);
>>   }
>>   
>> +static void priority_reclaim_data_space(struct btrfs_fs_info *fs_info,
>> +					struct btrfs_space_info *space_info,
>> +					struct reserve_ticket *ticket,
>> +					const enum btrfs_flush_state *states,
>> +					int states_nr)
>> +{
>> +	int flush_state = 0;
>> +	int commit_cycles = 2;
>> +
>> +	while (!space_info->full) {
>> +		flush_space(fs_info, space_info, U64_MAX, ALLOC_CHUNK_FORCE);
>> +		spin_lock(&space_info->lock);
>> +		if (ticket->bytes == 0) {
>> +			spin_unlock(&space_info->lock);
>> +			return;
>> +		}
>> +		spin_unlock(&space_info->lock);
>> +	}
>> +again:
>> +	while (flush_state < states_nr) {
>> +		u64 flush_bytes = U64_MAX;
>> +
>> +		if (!commit_cycles) {
>> +			if (states[flush_state] == FLUSH_DELALLOC_WAIT) {
>> +				flush_state++;
>> +				continue;
>> +			}
>> +			if (states[flush_state] == COMMIT_TRANS)
>> +				flush_bytes = ticket->bytes;
>> +		}
>> +
>> +		flush_space(fs_info, space_info, flush_bytes,
>> +			    states[flush_state]);
>> +		spin_lock(&space_info->lock);
>> +		if (ticket->bytes == 0) {
>> +			spin_unlock(&space_info->lock);
>> +			return;
>> +		}
>> +		spin_unlock(&space_info->lock);
>> +		flush_state++;
>> +	}
>> +	if (commit_cycles) {
>> +		commit_cycles--;
>> +		flush_state = 0;
>> +		goto again;
>> +	}
>> +}
>> +
>>   static void wait_reserve_ticket(struct btrfs_fs_info *fs_info,
>>   				struct btrfs_space_info *space_info,
>>   				struct reserve_ticket *ticket)
>> @@ -1134,6 +1182,15 @@ static int handle_reserve_ticket(struct btrfs_fs_info *fs_info,
>>   						evict_flush_states,
>>   						ARRAY_SIZE(evict_flush_states));
>>   		break;
>> +	case BTRFS_RESERVE_FLUSH_DATA:
>> +		priority_reclaim_data_space(fs_info, space_info, ticket,
>> +					data_flush_states,
>> +					ARRAY_SIZE(data_flush_states));
>> +		break;
>> +	case BTRFS_RESERVE_FLUSH_FREE_SPACE_INODE:
>> +		priority_reclaim_data_space(fs_info, space_info, ticket,
>> +					    NULL, 0);
>> +		break;
>>   	default:
>>   		ASSERT(0);
>>   		break;
>> @@ -1341,78 +1398,30 @@ int btrfs_reserve_data_bytes(struct btrfs_fs_info *fs_info, u64 bytes,
>>   			     enum btrfs_reserve_flush_enum flush)
>>   {
>>   	struct btrfs_space_info *data_sinfo = fs_info->data_sinfo;
>> -	const enum btrfs_flush_state *states = NULL;
>>   	u64 used;
>> -	int states_nr = 0;
>> -	int commit_cycles = 2;
>>   	int ret = -ENOSPC;
>>   
>>   	ASSERT(!current->journal_info || flush != BTRFS_RESERVE_FLUSH_DATA);
>>   
>> -	if (flush == BTRFS_RESERVE_FLUSH_DATA) {
>> -		states = data_flush_states;
>> -		states_nr = ARRAY_SIZE(data_flush_states);
>> -	}
>> -
>>   	spin_lock(&data_sinfo->lock);
>> -again:
>>   	used = btrfs_space_info_used(data_sinfo, true);
>>   
>>   	if (used + bytes > data_sinfo->total_bytes) {
>> -		u64 prev_total_bytes = data_sinfo->total_bytes;
>> -		int flush_state = 0;
>> +		struct reserve_ticket ticket;
>>   
>> +		init_waitqueue_head(&ticket.wait);
>> +		ticket.bytes = bytes;
>> +		ticket.error = 0;
>> +		list_add_tail(&ticket.list, &data_sinfo->priority_tickets);
> 
> nit: Shouldn't adding the ticket also be recorded in
> spac_info->reclaim_size?
> I see later that you are removing this code and relying on the existing
> logic in __reserve_metadata_bytes( renamed to reserve_bytes) which
> correctly modifies reclaim_size, but this just means this particular
> patch is slightly broken.

Yup I'll fix this up, thanks.  Thanks,

Josef

Patch
diff mbox series

diff --git a/fs/btrfs/space-info.c b/fs/btrfs/space-info.c
index 799ee6090693..ee4747917b81 100644
--- a/fs/btrfs/space-info.c
+++ b/fs/btrfs/space-info.c
@@ -1068,6 +1068,54 @@  static void priority_reclaim_metadata_space(struct btrfs_fs_info *fs_info,
 	} while (flush_state < states_nr);
 }
 
+static void priority_reclaim_data_space(struct btrfs_fs_info *fs_info,
+					struct btrfs_space_info *space_info,
+					struct reserve_ticket *ticket,
+					const enum btrfs_flush_state *states,
+					int states_nr)
+{
+	int flush_state = 0;
+	int commit_cycles = 2;
+
+	while (!space_info->full) {
+		flush_space(fs_info, space_info, U64_MAX, ALLOC_CHUNK_FORCE);
+		spin_lock(&space_info->lock);
+		if (ticket->bytes == 0) {
+			spin_unlock(&space_info->lock);
+			return;
+		}
+		spin_unlock(&space_info->lock);
+	}
+again:
+	while (flush_state < states_nr) {
+		u64 flush_bytes = U64_MAX;
+
+		if (!commit_cycles) {
+			if (states[flush_state] == FLUSH_DELALLOC_WAIT) {
+				flush_state++;
+				continue;
+			}
+			if (states[flush_state] == COMMIT_TRANS)
+				flush_bytes = ticket->bytes;
+		}
+
+		flush_space(fs_info, space_info, flush_bytes,
+			    states[flush_state]);
+		spin_lock(&space_info->lock);
+		if (ticket->bytes == 0) {
+			spin_unlock(&space_info->lock);
+			return;
+		}
+		spin_unlock(&space_info->lock);
+		flush_state++;
+	}
+	if (commit_cycles) {
+		commit_cycles--;
+		flush_state = 0;
+		goto again;
+	}
+}
+
 static void wait_reserve_ticket(struct btrfs_fs_info *fs_info,
 				struct btrfs_space_info *space_info,
 				struct reserve_ticket *ticket)
@@ -1134,6 +1182,15 @@  static int handle_reserve_ticket(struct btrfs_fs_info *fs_info,
 						evict_flush_states,
 						ARRAY_SIZE(evict_flush_states));
 		break;
+	case BTRFS_RESERVE_FLUSH_DATA:
+		priority_reclaim_data_space(fs_info, space_info, ticket,
+					data_flush_states,
+					ARRAY_SIZE(data_flush_states));
+		break;
+	case BTRFS_RESERVE_FLUSH_FREE_SPACE_INODE:
+		priority_reclaim_data_space(fs_info, space_info, ticket,
+					    NULL, 0);
+		break;
 	default:
 		ASSERT(0);
 		break;
@@ -1341,78 +1398,30 @@  int btrfs_reserve_data_bytes(struct btrfs_fs_info *fs_info, u64 bytes,
 			     enum btrfs_reserve_flush_enum flush)
 {
 	struct btrfs_space_info *data_sinfo = fs_info->data_sinfo;
-	const enum btrfs_flush_state *states = NULL;
 	u64 used;
-	int states_nr = 0;
-	int commit_cycles = 2;
 	int ret = -ENOSPC;
 
 	ASSERT(!current->journal_info || flush != BTRFS_RESERVE_FLUSH_DATA);
 
-	if (flush == BTRFS_RESERVE_FLUSH_DATA) {
-		states = data_flush_states;
-		states_nr = ARRAY_SIZE(data_flush_states);
-	}
-
 	spin_lock(&data_sinfo->lock);
-again:
 	used = btrfs_space_info_used(data_sinfo, true);
 
 	if (used + bytes > data_sinfo->total_bytes) {
-		u64 prev_total_bytes = data_sinfo->total_bytes;
-		int flush_state = 0;
+		struct reserve_ticket ticket;
 
+		init_waitqueue_head(&ticket.wait);
+		ticket.bytes = bytes;
+		ticket.error = 0;
+		list_add_tail(&ticket.list, &data_sinfo->priority_tickets);
 		spin_unlock(&data_sinfo->lock);
 
-		/*
-		 * Everybody can force chunk allocation, so try this first to
-		 * see if we can just bail here and make our reservation.
-		 */
-		flush_space(fs_info, data_sinfo, bytes, ALLOC_CHUNK_FORCE);
-		spin_lock(&data_sinfo->lock);
-		if (prev_total_bytes < data_sinfo->total_bytes)
-			goto again;
+		ret = handle_reserve_ticket(fs_info, data_sinfo, &ticket,
+					    flush);
+	} else {
+		btrfs_space_info_update_bytes_may_use(fs_info, data_sinfo, bytes);
+		ret = 0;
 		spin_unlock(&data_sinfo->lock);
-
-		/*
-		 * Cycle through the rest of the flushing options for our flush
-		 * type, then try again.
-		 */
-		while (flush_state < states_nr) {
-			u64 flush_bytes = U64_MAX;
-
-			/*
-			 * Previously we unconditionally committed the
-			 * transaction twice before finally checking against
-			 * pinned space before committing the final time.  We
-			 * also skipped flushing delalloc the final pass
-			 * through.
-			 */
-			if (!commit_cycles) {
-				if (states[flush_state] == FLUSH_DELALLOC_WAIT) {
-					flush_state++;
-					continue;
-				}
-				if (states[flush_state] == COMMIT_TRANS)
-					flush_bytes = bytes;
-			}
-
-			flush_space(fs_info, data_sinfo, flush_bytes,
-				    states[flush_state]);
-			flush_state++;
-		}
-
-		if (!commit_cycles)
-			goto out;
-
-		commit_cycles--;
-		spin_lock(&data_sinfo->lock);
-		goto again;
 	}
-	btrfs_space_info_update_bytes_may_use(fs_info, data_sinfo, bytes);
-	ret = 0;
-	spin_unlock(&data_sinfo->lock);
-out:
 	if (ret)
 		trace_btrfs_space_reservation(fs_info,
 					      "space_info:enospc",