Re: [PATCH] btrfs: fix subvolume deletion lockup caused by inodes xarray race

[Omar Sandoval <osandov@osandov.com>]

On Tue, Aug 26, 2025 at 04:25:07PM -0400, Josef Bacik wrote:
> On Tue, Aug 26, 2025 at 01:01:38PM -0700, Omar Sandoval wrote:
> > From: Omar Sandoval <osandov@fb.com>
> > 
> > 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: 310b2f5d5a94 ("btrfs: use an xarray to track open inodes in a root")
> > Cc: stable@vger.kernel.org # 6.11+
> > Co-authored-by: Leo Martins <loemra.dev@gmail.com>
> > Signed-off-by: Leo Martins <loemra.dev@gmail.com>
> > Signed-off-by: Omar Sandoval <osandov@fb.com>
> 
> Heh, I was just noticing weirdness here as I started converting inodes over to
> using an xarray instead of a hash.

Yeah, and there's still some weirdness in btrfs_del_inode_from_root()
with dropping the xa_lock just to check btrfs_root_refs(). I think
that's just left over from f72ff01df9cf ("btrfs: do not call
synchronize_srcu() in inode_tree_del"). Either I or Leo can send a
followup cleanup for that.

> Nice catch,
> 
> Reviewed-by: Josef Bacik <josef@toxicpanda.com>

Thanks!