Re: [PATCH/RFC] btrfs: fix folio lock leak in writepage_delalloc() for folios dirtied behind btrfs' back
Qu Wenruo <[email protected]> Mon, 27 Jul 2026 18:11:34 +0930
| Newsgroups | org.kernel.vger.linux-btrfs,org.kernel.vger.kvm,org.kernel.vger.linux-fsdevel,org.kernel.vger.linux-kernel,org.kernel.vger.linux-s390,org.kvack.linux-mm |
|---|---|
| Message-ID | <[email protected]> |
在 2026/7/27 17:41, Christian Borntraeger 写道:
> Am 25.07.26 um 08:26 schrieb Boris Burkov:
>> On Fri, Jul 24, 2026 at 08:10:36AM +0930, Qu Wenruo wrote:
>>>
>>>
>>> 在 2026/7/23 21:30, Matthew Wilcox 写道:
>>>> On Thu, Jul 23, 2026 at 10:12:27AM +0930, Qu Wenruo wrote:
>>>>>
>>>>>
>>>>> 在 2026/7/22 22:27, Matthew Wilcox 写道:
>>>>>> On Wed, Jul 22, 2026 at 11:29:36AM +0200, Christian Borntraeger
>>>>>> wrote:
>>>>>>> * 4. Thread B loops sync_file_range(WRITE|WAIT) on the
>>>>>>> target file.
>>>>>>> * Whenever a full clean cycle (clear_page_dirty_for_io(),
>>>>>>> * writeback, bits cleared) completes inside thread A's
>>>>>>> * submission->completion window, the completion-time
>>>>>>> * set_page_dirty_lock() hits a *clean* folio:
>>>>>>> filemap_dirty_folio()
>>>>>>> * sets only the folio flag and the xarray tag - no btrfs
>>>>>>> subpage
>>>>>>> * dirty bit, no delalloc reservation. See the 20-year-
>>>>>>> old comment
>>>>>>> * above bio_set_pages_dirty() in block/bio.c describing
>>>>>>> exactly
>>>>>>> * this ("other code (eg, flusher threads) could clean
>>>>>>> the pages").
>>>>>>
>>>>>> There's your problem. filemap_dirty_folio() documents that btrfs is
>>>>>> doing it wrongly:
>>>>>>
>>>>>> * Filesystems which do not use buffer heads should call this
>>>>>> function
>>>>>> * from their dirty_folio address space operation. It ignores the
>>>>>> * contents of folio_get_private(), so if the filesystem marks
>>>>>> individual
>>>>>> * blocks as dirty, the filesystem should handle that itself.
>>>>>>
>>>>>> fs/btrfs/inode.c: .dirty_folio = filemap_dirty_folio,
>>>>>>
>>>>>> so btrfs should have its own btrfs_dirty_folio() which does whatever
>>>>>> metadata updates it needs to and then call filemap_dirty_folio() to
>>>>>> take care of the page cache business. See iomap_dirty_folio() as
>>>>>> an example, but many other filesystems also do this.
>>>>>
>>>>> Thanks a lot for the advice.
>>>>>
>>>>> However it looks like the sub-folio dirty block tracking is a little
>>>>> different between iomap and btrfs.
>>>>
>>>> My point is not that "you should do it the exact same way as iomap".
>>>> Rather "the dirty_folio op is the entry point to tell the filesystem
>>>> that a folio is being dirtied".
>>> And since dirty_folio() is not allowed to sleep, we should introduce
>>> some
>>> extra mechanism, e.g. page private 2/checked, to notify the fs that the
>>> folio is marked dirty without proper preparation.
>>>
>>> Then during writeback, detect such folio and do needed preparation
>>> for it
>>> since at writeback we're allowed to sleep.
>>>
>>> That sounds feasible, but I haven't seen anyone doing that (including
>>> the
>>> older btrfs cow fixup).
>>>
>>> Will explore that path. Thanks a lot again for the dirty_folio() help.
>>>
>>> Thanks,
>>> Qu
>>
>> Here is my proposal for a candidate fix. It passes the reproducer in
>> this thread as well as several more intense reproducers (alluded to but
>> not yet included)
>>
>> https://lore.kernel.org/linux-
>> btrfs/6758d4f27be0bbdb865cee7dd5adc435c969f4a3.1784960646.git.boris@bur.io/T/#u
> I am willing to give it a try, but that does not seem to apply on 7.2-rc4
>
The latest version is out:
https://lore.kernel.org/linux-btrfs/9f7a81cdd022d9e3f69d347a17c0a25438ccec14.1785131713.git.boris@bur.io/
And you can apply it on the latest btrfs for-next branch without any
conflict:
https://github.com/btrfs/linux.git for-next