On 24.09.19 г. 23:50 ч., Josef Bacik wrote: > When free'ing extents in a block group we check to see if the block > group is not cached, and then cache it if we need to. However we'll > just carry on as long as we're loading the cache. This is problematic > because we are dirtying the block group here. If we are fast enough we > could do a transaction commit and clear the free space cache while we're This would imply a race condition between loading the space cache and running delayed refs - because there where __btrfs_free_extent->btrfs_update_blockgroup(alloc=0) is called from, no ? > still loading the space cache in another thread. This truncates the > free space inode, which will keep it from loading the space cache. > > Fix this by using the btrfs_block_group_cache_done helper so that we try > to load the space cache unconditionally here, which will result in the > caller waiting for the fast caching to complete and keep us from > truncating the free space inode. So the problem was that cache->cached == BTRFS_CACHE_NO misses other interim states e.g. CACHE_STARTED/CACHE_FAST. Which leads to the aforementioned race, correct? > > Signed-off-by: Josef Bacik <josef@xxxxxxxxxxxxxx> Based on our offlist discussion: Reviewed-by: Nikolay Borisov <nborisov@xxxxxxxx> > --- > fs/btrfs/block-group.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/fs/btrfs/block-group.c b/fs/btrfs/block-group.c > index bf7e3f23bba7..d7b3a516f27a 100644 > --- a/fs/btrfs/block-group.c > +++ b/fs/btrfs/block-group.c > @@ -2661,7 +2661,7 @@ int btrfs_update_block_group(struct btrfs_trans_handle *trans, > * is because we need the unpinning stage to actually add the > * space back to the block group, otherwise we will leak space. > */ > - if (!alloc && cache->cached == BTRFS_CACHE_NO) > + if (!alloc && !btrfs_block_group_cache_done(cache)) > btrfs_cache_block_group(cache, 1); > > byte_in_group = bytenr - cache->key.objectid; >
