Message-ID: <CADhLXY4Pk60+sSLtOOuR2QdTKbYXUAjwhgb7nH8qugf4DROT7w@mail.gmail.com>
Date: Wed, 3 Dec 2025 13:13:35 +0530
From: Deepanshu Kartikey <kartikey406@...il.com>
To: Zhang Yi <yi.zhang@...weicloud.com>
Cc: linux-ext4@...r.kernel.org, linux-kernel@...r.kernel.org, 
	syzbot+b0a0670332b6b3230a0a@...kaller.appspotmail.com, tytso@....edu, 
	adilger.kernel@...ger.ca, djwong@...nel.org
Subject: Re: [PATCH v2] ext4: check folio uptodate state in ext4_page_mkwrite()

On Wed, Dec 3, 2025 at 12:22 PM Zhang Yi <yi.zhang@...weicloud.com> wrote:
>
> On 12/3/2025 9:37 AM, Deepanshu Kartikey wrote:
> > On Tue, Dec 2, 2025 at 5:54 PM Zhang Yi <yi.zhang@...weicloud.com> wrote:
> >>
> >> Hi Deepanshu!
> >>
> >> On 11/30/2025 10:06 AM, Deepanshu Kartikey wrote:
> >>> On Sat, Nov 22, 2025 at 7:27 AM Deepanshu Kartikey
> >>> <kartikey406@...il.com> wrote:
> >>>>
> >>>> When delayed block allocation fails due to filesystem corruption,
> >>>> ext4's writeback error handling invalidates affected folios by calling
> >>>> mpage_release_unused_pages() with invalidate=true, which explicitly
> >>>> clears the uptodate flag:
> >>>>
> >>>>     static void mpage_release_unused_pages(..., bool invalidate)
> >>>>     {
> >>>>         ...
> >>>>         if (invalidate) {
> >>>>             block_invalidate_folio(folio, 0, folio_size(folio));
> >>>>             folio_clear_uptodate(folio);
> >>>>         }
> >>>>     }
> >>>>
> >>>> If ext4_page_mkwrite() is subsequently called on such a non-uptodate
> >>>> folio, it can proceed to mark the folio dirty without checking its
> >>>> state. This triggers a warning in __folio_mark_dirty():
> >>>>
> >>>>     WARNING: CPU: 0 PID: 5 at mm/page-writeback.c:2960
> >>>>     __folio_mark_dirty+0x578/0x880
> >>>>
> >>>>     Call Trace:
> >>>>      fault_dirty_shared_page+0x16e/0x2d0
> >>>>      do_wp_page+0x38b/0xd20
> >>>>      handle_pte_fault+0x1da/0x450
> >>>>      __handle_mm_fault+0x652/0x13b0
> >>>>      handle_mm_fault+0x22a/0x6f0
> >>>>      do_user_addr_fault+0x200/0x8a0
> >>>>      exc_page_fault+0x81/0x1b0
> >>>>
> >>>> This scenario occurs when:
> >>>> 1. A write with delayed allocation marks a folio dirty (uptodate=1)
> >>>> 2. Writeback attempts block allocation but detects filesystem corruption
> >>>> 3. Error handling calls mpage_release_unused_pages(invalidate=true),
> >>>>    which clears the uptodate flag via folio_clear_uptodate()
> >>>> 4. A subsequent ftruncate() triggers ext4_truncate()
> >>>> 5. ext4_block_truncate_page() attempts to zero the page tail
> >>>> 6. This triggers a write fault on the mmap'd page
> >>>> 7. ext4_page_mkwrite() is called with the non-uptodate folio
> >>>> 8. Without checking uptodate, it proceeds to mark the folio dirty
> >>>> 9. __folio_mark_dirty() triggers: WARN_ON_ONCE(!folio_test_uptodate())
> >>
> >> Thank you a lot for analyzing this issue and the fix patch. As I was
> >> going through the process of understanding this issue, I had one
> >> question. Is the code flow that triggers the warning as follows?
> >>
> >> wp_page_shared()
> >>   do_page_mkwrite()
> >>     ext4_page_mkwrite()
> >>       block_page_mkwrite()   //The default delalloc path
> >>         block_commit_write()
> >>           mark_buffer_dirty()
> >>             __folio_mark_dirty(0)  //'warn' is false, doesn't trigger warning
> >>         folio_mark_dirty()
> >>           ext4_dirty_folio()
> >>             block_dirty_folio  //newly_dirty is false, doesn't call __folio_mark_dirty()
> >>   fault_dirty_shared_page()
> >>     folio_mark_dirty()  //Trigger warning ?
> >>
> >> This folio has been marked as dirty. How was this warning triggered?
> >> Am I missing something?
> >>
> >> Thanks,
> >> Yi.
> >>
> >
> > Hi Yi,
> >
> > Thank you for your question about the exact flow that triggers the warning.
> >
>
> Thank you for the clarification, but there are still some details that
> are unclear.
>
> > You're correct that the code paths within ext4_page_mkwrite() and
> > block_page_mkwrite() call __folio_mark_dirty() with warn=0, so no
> > warning occurs there. The warning actually triggers later, in
> > fault_dirty_shared_page() after page_mkwrite returns.
> >
> > Here's the complete flow:
> >
> >   wp_page_shared()
> >     ↓
> >     do_page_mkwrite()
> >       ↓
> >       ext4_page_mkwrite()
> >         ↓
> >         block_page_mkwrite()
> >           ↓
> >           mark_buffer_dirty() → __folio_mark_dirty(warn=0)  // No warning
>
>              ↓
>              if (!folio_test_set_dirty(folio))
>                         //The folio will be mark as dirty --- 1
>
> >         ↓
> >         Returns success
> >     ↓
> >     fault_dirty_shared_page(vma, folio)  ← Warning triggers here
> >       ↓
> >       folio_mark_dirty(folio)
> >         ↓
> >         ext4_dirty_folio()
> >           ↓
> >           block_dirty_folio()
> >             ↓
> >             if (!folio_test_set_dirty(folio))  // Folio not already dirty
>
>               This makes me confused, this folio should be set to dirty
>               at position 1. If it is not dirty here, who cleared the dirty
>               flag for this folio?
>

Hi Yi,

Excellent question! You're absolutely right that mark_buffer_dirty()
marks the folio dirty at position 1. The key is that the folio dirty
flag is cleared later by the error handling code.

When delayed allocation fails and mpage_release_unused_pages() is
called with invalidate=true, it calls:

  block_invalidate_folio(folio, 0, folio_size(folio));

This function not only invalidates the folio but also clears the dirty
flag. Looking at the code path:

  block_invalidate_folio()
    → do_invalidate_folio()
      → Clears the dirty flag
      → Detaches buffer heads

So the sequence is:

1. First wp_page_shared(): folio becomes dirty=1, uptodate=1 (via
mark_buffer_dirty)
2. Writeback fails due to corruption
3. mpage_release_unused_pages(invalidate=true):
   - block_invalidate_folio() clears dirty flag
   - folio_clear_uptodate() clears uptodate flag
   - Result: folio is now dirty=0, uptodate=0
4. Second wp_page_shared(): called with folio dirty=0, uptodate=0

This is confirmed by my debug logs:

  [22.400513] WP_PAGE_SHARED: ENTER folio=... uptodate=0 dirty=0

The folio is BOTH non-uptodate AND non-dirty when the second
wp_page_shared() is called.

Without my fix:
- ext4_page_mkwrite() succeeds (doesn't check uptodate)
- block_page_mkwrite() tries to mark the folio dirty again
- fault_dirty_shared_page() is reached
- block_dirty_folio() finds folio not dirty (dirty=0)
- Calls __folio_mark_dirty(warn=1) with uptodate=0
- WARNING triggers

With my fix:
- ext4_page_mkwrite() checks uptodate and returns VM_FAULT_SIGBUS
- Never reaches fault_dirty_shared_page()
- No warning

Does this answer your question?

Best regards,
Deepanshu

Powered by blists - more mailing lists