Messages in this thread | | | Subject | Re: [BUG] fs: ext4: possible ABBA deadlock in ext4_inline_data_truncate() and ext4_punch_hole() | From | Jia-Ju Bai <> | Date | Fri, 10 Dec 2021 10:03:37 +0800 |
| |
On 2021/12/10 1:00, Theodore Y. Ts'o wrote: > On Thu, Dec 09, 2021 at 07:10:44PM +0800, Jia-Ju Bai wrote: >> Hello, >> >> My static analysis tool reports a possible ABBA deadlock in the ext4 module >> in Linux 5.10: >> >> ext4_inline_data_truncate() >> down_write(&EXT4_I(inode)->i_data_sem); --> Line 1895 (Lock A) >> ext4_xattr_ibody_get() >> ext4_xattr_inode_get() >> ext4_xattr_inode_iget() >> inode_lock(inode); --> Line 427 (Lock B) >> >> ext4_punch_hole() >> inode_lock(inode); --> Line 4018 (Lock B) >> ext4_update_disksize_before_punch() >> ext4_update_i_disksize() >> down_write(&EXT4_I(inode)->i_data_sem); --> Line 3248 (Lock A) >> >> When ext4_inline_data_truncate() and ext4_punch_hole() are concurrently >> executed, the deadlock can occur. >> >> I am not quite sure whether this possible deadlock is real and how to fix it >> if it is real. > Hi, > > Thanks for the report. I don't believe this is deadlock is possible, > because the first thing ext4_punch_hole() does is to check to see if > the inode has inline data --- and if so, it calls > ext4_convert_inline_data() to convert it to a normal file. In > ext4_convert_inline_data(), we take the xattr lock, and then do the > conversion, and then drop the xattr lock. So by the time > ext4_punch_hole() starts doing its work, the inode is not an inline > data file. > > In ext4_inline_data_truncate(), we take the xattr lock, and once we > have the xattr lock, we check to see if inode is still an inline data > file. If it has been converted, we then bail out. > > Hence, the ABBA deadlock that your static analysis tool has pointed > shouldn't happen in practice.
Hi Ted,
Thank you very much for the detailed explanation! I will improve my static analysis tool for this point.
Best wishes, Jia-Ju Bai
| |