Messages in this thread | | | Date | Sun, 28 Nov 2021 19:34:30 -0500 | From | Josef Bacik <> | Subject | Re: [BUG] fs: btrfs: several possible ABBA deadlocks |
| |
On Sat, Nov 27, 2021 at 04:23:37PM +0800, Jia-Ju Bai wrote: > Hello, > > My static analysis tool reports several possible ABBA deadlocks in the btrfs > module in Linux 5.10: > > # DEADLOCK 1: > __clear_extent_bit() > spin_lock(&tree->lock); --> Line 733 (Lock A) > split_state() > btrfs_split_delalloc_extent() > spin_lock(&BTRFS_I(inode)->lock); --> Line 1870 (Lock B) > > btrfs_inode_safe_disk_i_size_write() > spin_lock(&BTRFS_I(inode)->lock); --> Line 53 (Lock B) > find_contiguous_extent_bit() > spin_lock(&tree->lock); --> Line 1620 (Lock A) > > When __clear_extent_bit() and btrfs_inode_safe_disk_i_size_write() are > concurrently executed, the deadlock can occur. > > # DEADLOCK 2: > __set_extent_bit() > spin_lock(&tree->lock); --> Line 995 (Lock A) > set_state_bits() > btrfs_set_delalloc_extent() > spin_lock(&BTRFS_I(inode)->lock); --> Line 2007 or 2017 or 2029 (Lock > B) > > btrfs_inode_safe_disk_i_size_write() > spin_lock(&BTRFS_I(inode)->lock); --> Line 53 (Lock B) > find_contiguous_extent_bit() > spin_lock(&tree->lock); --> Line 1620 (Lock A) > > When __set_extent_bit() and btrfs_inode_safe_disk_i_size_write() are > concurrently executed, the deadlock can occur. > > # DEADLOCK 3: > convert_extent_bit() > spin_lock(&tree->lock); --> Line 1241 (Lock A) > set_state_bits() > btrfs_set_delalloc_extent() > spin_lock(&BTRFS_I(inode)->lock); --> Line 2007 or 2017 or 2029 (Lock > B) > > btrfs_inode_safe_disk_i_size_write() > spin_lock(&BTRFS_I(inode)->lock); --> Line 53 (Lock B) > find_contiguous_extent_bit() > spin_lock(&tree->lock); --> Line 1620 (Lock A) > > When convert_extent_bit() and btrfs_inode_safe_disk_i_size_write() are > concurrently executed, the deadlock can occur. > > I am not quite sure whether these possible deadlocks are real and how to fix > them if they are real. > Any feedback would be appreciated, thanks :) >
Hey Jia-Ju,
This is pretty good work, unfortunately it's wrong but it's in a subtle way that a tool wouldn't be able to catch. The btrfs_inode_safe_disk_i_size_write() helper only messes with BTRFS_I(inode)->file_extent_tree, which is separate from the BTRFS_I(inode)->io_tree. io_tree gets the btrfs_set_delalloc_extent() stuff called on it, but the file_extent_tree does not. The file_extent_tree has inode->lock -> tree->lock as the locking order, whereas the file_extent_tree has inode->lock -> tree->lock as the locking order. Thanks,
Josef
| |