[1/8] btrfs: do not allow reservations if we have pending tickets
diff mbox series

Message ID 20190816141952.19369-2-josef@toxicpanda.com
State New
Headers show
Series
  • Rework reserve ticket handling
Related show

Commit Message

Josef Bacik Aug. 16, 2019, 2:19 p.m. UTC
If we already have tickets on the list we don't want to steal their
reservations.  This is a preparation patch for upcoming changes,
technically this shouldn't happen today because of the way we add bytes
to tickets before adding them to the space_info in most cases.

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

Comments

Nikolay Borisov Aug. 19, 2019, 12:54 p.m. UTC | #1
On 16.08.19 г. 17:19 ч., Josef Bacik wrote:
> If we already have tickets on the list we don't want to steal their
> reservations.  This is a preparation patch for upcoming changes,
> technically this shouldn't happen today because of the way we add bytes
> to tickets before adding them to the space_info in most cases.

nit: IMO this changelog should be a bit more explicit since this commit
really makes reservations in FIFO order. Have you also quantified what's
the latency impact as I suspect this will introduce such latencies?


> 
> Signed-off-by: Josef Bacik <josef@toxicpanda.com>
> ---
>  fs/btrfs/space-info.c | 10 +++++++---
>  1 file changed, 7 insertions(+), 3 deletions(-)
> 
> diff --git a/fs/btrfs/space-info.c b/fs/btrfs/space-info.c
> index e9406b2133d1..d671d6476eed 100644
> --- a/fs/btrfs/space-info.c
> +++ b/fs/btrfs/space-info.c
> @@ -938,6 +938,7 @@ static int __reserve_metadata_bytes(struct btrfs_fs_info *fs_info,
>  	u64 used;
>  	u64 reclaim_bytes = 0;
>  	int ret = 0;
> +	bool pending_tickets;
>  
>  	ASSERT(orig_bytes);
>  	ASSERT(!current->journal_info || flush != BTRFS_RESERVE_FLUSH_ALL);
> @@ -945,14 +946,17 @@ static int __reserve_metadata_bytes(struct btrfs_fs_info *fs_info,
>  	spin_lock(&space_info->lock);
>  	ret = -ENOSPC;
>  	used = btrfs_space_info_used(space_info, true);
> +	pending_tickets = !list_empty(&space_info->tickets) ||
> +		!list_empty(&space_info->priority_tickets);
>  
>  	/*
>  	 * Carry on if we have enough space (short-circuit) OR call
>  	 * can_overcommit() to ensure we can overcommit to continue.
>  	 */
> -	if ((used + orig_bytes <= space_info->total_bytes) ||
> -	    can_overcommit(fs_info, space_info, orig_bytes, flush,
> -			   system_chunk)) {
> +	if (!pending_tickets &&
> +	    ((used + orig_bytes <= space_info->total_bytes) ||
> +	     can_overcommit(fs_info, space_info, orig_bytes, flush,
> +			   system_chunk))) {
>  		btrfs_space_info_update_bytes_may_use(fs_info, space_info,
>  						      orig_bytes);
>  		trace_btrfs_space_reservation(fs_info, "space_info",
>
Josef Bacik Aug. 19, 2019, 12:57 p.m. UTC | #2
On Mon, Aug 19, 2019 at 03:54:29PM +0300, Nikolay Borisov wrote:
> 
> 
> On 16.08.19 г. 17:19 ч., Josef Bacik wrote:
> > If we already have tickets on the list we don't want to steal their
> > reservations.  This is a preparation patch for upcoming changes,
> > technically this shouldn't happen today because of the way we add bytes
> > to tickets before adding them to the space_info in most cases.
> 
> nit: IMO this changelog should be a bit more explicit since this commit
> really makes reservations in FIFO order. Have you also quantified what's
> the latency impact as I suspect this will introduce such latencies?

Reservations were already in FIFO order because we would add new space to
existing tickets.  This doesn't change the behavior, just makes it so we get the
same behavior without refilling.  Thanks,

Josef

Patch
diff mbox series

diff --git a/fs/btrfs/space-info.c b/fs/btrfs/space-info.c
index e9406b2133d1..d671d6476eed 100644
--- a/fs/btrfs/space-info.c
+++ b/fs/btrfs/space-info.c
@@ -938,6 +938,7 @@  static int __reserve_metadata_bytes(struct btrfs_fs_info *fs_info,
 	u64 used;
 	u64 reclaim_bytes = 0;
 	int ret = 0;
+	bool pending_tickets;
 
 	ASSERT(orig_bytes);
 	ASSERT(!current->journal_info || flush != BTRFS_RESERVE_FLUSH_ALL);
@@ -945,14 +946,17 @@  static int __reserve_metadata_bytes(struct btrfs_fs_info *fs_info,
 	spin_lock(&space_info->lock);
 	ret = -ENOSPC;
 	used = btrfs_space_info_used(space_info, true);
+	pending_tickets = !list_empty(&space_info->tickets) ||
+		!list_empty(&space_info->priority_tickets);
 
 	/*
 	 * Carry on if we have enough space (short-circuit) OR call
 	 * can_overcommit() to ensure we can overcommit to continue.
 	 */
-	if ((used + orig_bytes <= space_info->total_bytes) ||
-	    can_overcommit(fs_info, space_info, orig_bytes, flush,
-			   system_chunk)) {
+	if (!pending_tickets &&
+	    ((used + orig_bytes <= space_info->total_bytes) ||
+	     can_overcommit(fs_info, space_info, orig_bytes, flush,
+			   system_chunk))) {
 		btrfs_space_info_update_bytes_may_use(fs_info, space_info,
 						      orig_bytes);
 		trace_btrfs_space_reservation(fs_info, "space_info",