on 6/13/2023 9:22 AM, Kemeng Shi wrote:
>
>
> on 6/12/2023 11:49 AM, Theodore Ts'o wrote:
>> On Mon, Jun 12, 2023 at 10:24:55AM +0800, Kemeng Shi wrote:
>>
>>> Hi ted, sorry for this issue. This patch added a WARN_ON for case that we free block
>>> to uninitialized block group which should be invalid.
>>> We can simply remove the WARN_ON to allow free on uninitialized block group as old
>>> way for emergency fix and I will find out why we free blocks to uninitialized block
>>> group in fast commit code path and is it a valid behavior.
>>
>> What I've done for now in the dev branch was to drop patches 12
>> through 19 of this patch series. That seemed to be a good break
>> point, and I wanted to make sure we had something working so we can
>> start doing a lot more intesive testing on the patches so far.
>>
>> Also, that way, when you resend the last 8 patches in the patch
>> series, we can make sure they get a proper review as opposed to making
>> changes on the fly.
> Sure, I will resend last 8 patches after I solve the issue. I can also take my time
> to look at problem in this way :)
Updates for how WARN_ON of free blocks to uninitialized block group is triggerred under
fast commit path in test generic/468.
# /sbin/mkfs.ext4 -F -q -O inline_data,fast_commit /dev/vdc
# /bin/mount -t ext4 -o acl,user_xattr -o block_validity /dev/vdc /vdc
# /root/xfstests/bin/xfs_io -i -f -c 'truncate 4202496' -c 'pwrite 0
4202496' -c fsync -c 'falloc 4202496 104857600' /vdc/testfile
The "falloc 4202496 104857600" will trigger block allocation in a
new uninitialized block group for file range "4202496 104857600" as
following:
ext4_map_blocks
/*
* Alloc blocks from uninitialized block group. Change to set
* group intialized will be full journaled.
*/
ext4_mb_new_blocks
[...]
/*
* New extents will be tracked in fast commit.
*/
ext4_fc_track_range
/*
* Add new extents of allocated range to inode which still has sapce
* in ext_inode_hdr
*/
ext4_ext_insert_extent
[...]
/*
* depth is 0 as inode has space in ext_inode_hdr, this will track
* inode in fast commit.
*/
ext4_ext_dirty(handle, inode, path + path->p_depth);
ext4_mark_inode_dirty
ext4_mark_iloc_dirty
ext4_fc_track_inode
# /root/xfstests/bin/xfs_io -i -c fsync /vdc/testfile
The fast commit is performed in fsync as following:
vfs_fsync
ext4_fsync_journal
ext4_fc_commit
ext4_fc_perform_commit
add EXT4_FC_TAG_ADD_RANGE of new extent range
add EXT4_FC_TAG_INODE of changed inode
# /root/xfstests/src/godown /vdc
Journaled change to set group intialized is discard as following:
ext4_shutdown
jbd2_journal_abort
# /bin/umount /dev/vdc
# /bin/mount -t ext4 -o acl,user_xattr -o block_validity /dev/vdc /vdc
Replay fast commit when mounting and added WARN_ON is triggered as
following:
ext4_fc_replay
/*
* replay EXT4_FC_TAG_ADD_RANGE, add extents contains blocks from
* uninitialized group back to inode
*/
ext4_fc_replay_add_range
/*
* replay EXT4_FC_TAG_INODE, this will mark trigger WARN_ON
*/
ext4_fc_replay_inode
/*
* mark all blocks in old inode free, then blocks from uninitialized
* block is freed and WARN_ON occurs
*/
ext4_ext_clear_bb
/* update inode with data journaled in fast commit */
[...]
/*
* mark all blocks in new inode in use, and gdp will be mark
* initialized normally
*/
ext4_fc_record_modified_inode
[...]
ext4_fc_set_bitmaps_and_counters
In this situation, free blocks to uninitialized block group do no harm.
And there may be more harmless situations, so I would like to simply
drop WARN_ON in next version.
--
Best wishes
Kemeng Shi
