Re: [PATCH] btrfs: wait for ordered extents before buffered write fallback in direct IO
Qu Wenruo <[email protected]>
| Newsgroups | org.kernel.vger.linux-btrfs,org.kernel.vger.linux-kernel |
|---|---|
| Message-ID | <[email protected]> |
在 2026/6/25 16:16, Zhou, Yun 写道: > > > On 6/25/26 13:17, Qu Wenruo wrote: >> >> 在 2026/6/25 14:43, Qu Wenruo 写道: >>> >>> >>> 在 2026/6/25 11:44, Yun Zhou 写道: >>>> When btrfs_direct_write() falls back to buffered IO after a failed DIO >>>> attempt, it may race with the asynchronous completion of DIO ordered >>>> extents. This leads to a BUG_ON in insert_ordered_extent() due to >>>> overlapping ordered extents in the per-inode rb-tree. >>>> >>>> The race sequence is: >>>> 1. DIO creates an ordered extent via btrfs_dio_iomap_begin() >>>> 2. Page fault occurs (nofault=true), no bio is submitted >>>> (submitted=0) >>>> 3. btrfs_dio_iomap_end() truncates and finishes the OE asynchronously >>>> via btrfs_finish_ordered_extent() which queues work >>>> 4. iomap returns 0, retry logic faults in pages and retries DIO >>>> 5. Second DIO attempt also fails, code reaches buffered: label >>>> 6. btrfs_buffered_write() dirties pages for the same range >>> >>> btrfs_buffered_write() >>> |- copy_one_range() >>> |- lock_and_cleanup_extent_if_needed() >>> |- btrfs_start_ordered_extent() >>> >>> So your explanation doesn't makes sense. As if there is the direct IO oe >>> remaining, we will wait for that OE to complete. >>> >>> There is still something missing. >>> >>>> 7. btrfs_fdatawrite_range() triggers writeback >>>> 8. run_delalloc_nocow() -> fallback_to_cow() -> cow_file_range() >>>> tries to insert a new ordered extent for the same file offset >>>> 9. The DIO ordered extent hasn't been removed from the rb-tree yet >>>> (btrfs_finish_ordered_io running async in workqueue) -> BUG_ON >>>> >>>> Fix this by waiting for any pending ordered extents in the target range >>>> before starting the buffered write. >>>> >>>> Reported-by: [email protected] >>>> Closes: https://syzkaller.appspot.com/bug?extid=ba2afde329fc27e3f22e >>>> Fixes: acf9ed3a6c00 ("btrfs: retry faulting in the pages after a zero >>>> sized short direct write") >> >> And the fixes tag is also incorrect. >> >> Without that commit, we will directly fallback to buffered write without >> retry faulting in the pages. >> >> So by your explanation it will trigger the same problem, with or without >> that commit. > > Yes, my previous analysis does seem inaccurate. Commit acf9ed3a6c00 (which > added retries) merely amplified the window for the issue to occur, but > the problem has actually existed since ff66fe666233 ('btrfs: fix > incorrect buffered IO fallback for append direct writes'), which > introduced i_size revert on DIO short writes, causing > lock_and_cleanup_extent_if_need() to skip the OE check (since start_pos > >= reverted i_size). I will correct the commit message and the Fixes > tag in v2. That's right, and I have already sent out a fix based on that isize revert: https://lore.kernel.org/linux-btrfs/aaabe0f2bee07800fb0490d24fab5cd162e5241d.1782366056.git.wqu@suse.com/ > > Thanks, > Yun