[RFC PATCH v2 0/2] Fix an error caused by improperly dirtied buffer

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



From: Shida Zhang <zhangshida@xxxxxxxxxx>

Hi all,

On an old kernel version(4.19, ext3, journal=data, pagesize=64k),
an assertion failure will occasionally be triggered by the line below:
---------
jbd2_journal_commit_transaction
{
...
J_ASSERT_BH(bh, !buffer_dirty(bh));
/*
* The buffer on BJ_Forget list and not jbddirty means
...
}
---------

The same condition may also be applied to the lattest kernel version.

This patch set fixes it by:
1.Trace the user data dirting in ext4_block_write_begin().(patch 1)
2.Replace the __block_write_begin with ext4_block_write_begin().(patch 2)
3.Remove some superfluous things.(patch 3)

But there is no patch 3. :p

Because the first two patch will have a restrained effect for ext4,
in that it works only when data = journal.
But for the patch 3, it is intended for removing the clear_buffer_new() 
and mark_buffer_dirty(), as suggested by Jan in [1]:

> From the part:
>                                 if (folio_test_uptodate(folio)) {
>                                         clear_buffer_new(bh);
>                                         set_buffer_uptodate(bh);
>                                         mark_buffer_dirty(bh);
>                                         continue;
>                                 }
>
> we can actually remove the clear_buffer_new() and mark_buffer_dirty() bits
> because they will be done by block_commit_write() or
> folio_zero_new_buffers() and they are superfluous and somewhat odd here
> anyway.
>
> And the call to folio_zero_new_buffers() from ext4_block_write_begin()
> needs to call ext4_journalled_zero_new_buffers() for inodes where data is
> journalled.
>

Specifically, assume we remove the clear_buffer_new() and mark_buffer_dirty(),
who will be reponsible for tracing/dirting it?
In data=journal:
ext4_journalled_write_end
   ext4_journalled_zero_new_buffers
       if (buffer_new(bh))
          if(!folio_test_uptodate(folio))
              write_end_fn
                 ext4_dirty_journalled_data(handle, bh);//mark dirty
          }
          clear_buffer_new(bh);//clear new
 
that means it will be dirtied only if the folio is not uptodate.

Maybe we should clear folio uptodate, too?
Things start to become a little scary now. 
So whether we should remove the mark_buffer_dirty() remains to be discussed.


-Shida.

[1] Version 1:
https://lore.kernel.org/linux-ext4/CANubcdVHbbq=WsTXU4EWAUPUby5--CLe5rf1GPzNPv+Y0a9VzQ@xxxxxxxxxxxxxx/T/#m19d3b9357f5dff050f75edc863e47f3cb018d778

Shida Zhang (2):
  ext4: fix a potential assertion failure due to improperly dirtied
    buffer
  ext4: Replace the __block_write_begin with ext4_block_write_begin

 fs/ext4/inode.c | 49 ++++++++++++++++++++++++++++---------------------
 1 file changed, 28 insertions(+), 21 deletions(-)

-- 
2.33.0





[Index of Archives]     [Reiser Filesystem Development]     [Ceph FS]     [Kernel Newbies]     [Security]     [Netfilter]     [Bugtraq]     [Linux FS]     [Yosemite National Park]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Device Mapper]     [Linux Media]

  Powered by Linux