Re: [PATCH 07/20] btrfs: call btrfs_try_granting_tickets when unpinning anything

[Nikolay Borisov <nborisov@xxxxxxxx>]

On 30.01.20 г. 1:50 ч., Josef Bacik wrote:
> When unpinning we were only calling btrfs_try_granting_tickets() if
> global_rsv->space_info == space_info, which is problematic because we
> use ticketing for SYSTEM chunks, and want to use it for DATA as well.
> Fix this by moving this call outside of that if statement.
> 
> Signed-off-by: Josef Bacik <josef@xxxxxxxxxxxxxx>


Reviewed-by: Nikolay Borisov <nborisov@xxxxxxxx> . 


nit: I only wonderef if making if (!readonly && return_free_space) 
the top level 'if' and then have 2 ifs inside of it : 
1 being 'if (global_rsv->space_info == space_info)' and the other 
being 'if (len)' would make the code look better but it's not - 
it's adding another level of nesting:

diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
index ab354ea09177..4ff9a3140864 100644
--- a/fs/btrfs/extent-tree.c
+++ b/fs/btrfs/extent-tree.c
@@ -2872,26 +2872,25 @@ static int unpin_extent_range(struct btrfs_fs_info *fs_info,
                        readonly = true;
                }
                spin_unlock(&cache->lock);
-               if (!readonly && return_free_space &&
-                   global_rsv->space_info == space_info) {
-                       u64 to_add = len;
-
-                       spin_lock(&global_rsv->lock);
-                       if (!global_rsv->full) {
-                               to_add = min(len, global_rsv->size -
-                                            global_rsv->reserved);
-                               global_rsv->reserved += to_add;
-                               btrfs_space_info_update_bytes_may_use(fs_info,
-                                               space_info, to_add);
-                               if (global_rsv->reserved >= global_rsv->size)
-                                       global_rsv->full = 1;
-                               len -= to_add;
+               if (!readonly && return_free_space) {
+                       if (global_rsv->space_info = space_info ){
+                               spin_lock(&global_rsv->lock);
+                               if (!global_rsv->full) {
+                                       u64 to_add = len;
+                                       to_add = min(len, global_rsv->size -
+                                                    global_rsv->reserved);
+                                       global_rsv->reserved += to_add;
+                                       btrfs_space_info_update_bytes_may_use(fs_info,
+                                                       space_info, to_add);
+                                       if (global_rsv->reserved >= global_rsv->size)
+                                               global_rsv->full = 1;
+                                       len -= to_add;
+                               }
+                               spin_unlock(&global_rsv->lock);
                        }
-                       spin_unlock(&global_rsv->lock);
+                       if (len)
+                               btrfs_try_granting_tickets(fs_info, space_info);
                }
-               /* Add to any tickets we may have */
-               if (!readonly && return_free_space && len)
-                       btrfs_try_granting_tickets(fs_info, space_info);
                spin_unlock(&space_info->lock);
        }