Skip to content

Commit 9dc1f5d

Browse files
osandovloemraw
authored andcommitted
btrfs: fix subvolume deletion lockup caused by inodes xarray race
There is a race condition between inode eviction and inode caching that can cause a live struct btrfs_inode to be missing from the root->inodes xarray. Specifically, there is a window during evict() between the inode being unhashed and deleted from the xarray. If btrfs_iget() is called for the same inode in that window, it will be recreated and inserted into the xarray, but then eviction will delete the new entry, leaving nothing in the xarray: Thread 1 Thread 2 --------------------------------------------------------------- evict() remove_inode_hash() btrfs_iget_path() btrfs_iget_locked() btrfs_read_locked_inode() btrfs_add_inode_to_root() destroy_inode() btrfs_destroy_inode() btrfs_del_inode_from_root() __xa_erase In turn, this can cause issues for subvolume deletion. Specifically, if an inode is in this lost state, and all other inodes are evicted, then btrfs_del_inode_from_root() will call btrfs_add_dead_root() prematurely. If the lost inode has a delayed_node attached to it, then when btrfs_clean_one_deleted_snapshot() calls btrfs_kill_all_delayed_nodes(), it will loop forever because the delayed_nodes xarray will never become empty (unless memory pressure forces the inode out). We saw this manifest as soft lockups in production. Fix it by only deleting the xarray entry if it matches the given inode (using __xa_cmpxchg()). Fixes: 310b2f5 ("btrfs: use an xarray to track open inodes in a root") Cc: [email protected] # 6.11+ Reviewed-by: Josef Bacik <[email protected]> Reviewed-by: Filipe Manana <[email protected]> Co-authored-by: Leo Martins <[email protected]> Signed-off-by: Leo Martins <[email protected]> Signed-off-by: Omar Sandoval <[email protected]>
1 parent a514983 commit 9dc1f5d

File tree

1 file changed

+11
-1
lines changed

1 file changed

+11
-1
lines changed

fs/btrfs/inode.c

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5697,7 +5697,17 @@ static void btrfs_del_inode_from_root(struct btrfs_inode *inode)
56975697
bool empty = false;
56985698

56995699
xa_lock(&root->inodes);
5700-
entry = __xa_erase(&root->inodes, btrfs_ino(inode));
5700+
/*
5701+
* This btrfs_inode is being freed and has already been unhashed at this
5702+
* point. It's possible that another btrfs_inode has already been
5703+
* allocated for the same inode and inserted itself into the root, so
5704+
* don't delete it in that case.
5705+
*
5706+
* Note that this shouldn't need to allocate memory, so the gfp flags
5707+
* don't really matter.
5708+
*/
5709+
entry = __xa_cmpxchg(&root->inodes, btrfs_ino(inode), inode, NULL,
5710+
GFP_ATOMIC);
57015711
if (entry == inode)
57025712
empty = xa_empty(&root->inodes);
57035713
xa_unlock(&root->inodes);

0 commit comments

Comments
 (0)