On Tue, Jul 16, 2024 at 07:54:43PM +0800, Baokun Li wrote: > Hi Ojaswin, > > On 2024/7/16 17:54, Ojaswin Mujoo wrote: > > > > But the journal will ensure the consistency of the extents path after > > > > this patch. > > > > > > > > When ext4_ext_get_access() or ext4_ext_dirty() returns an error in > > > > ext4_ext_rm_idx() and ext4_ext_correct_indexes(), this may cause > > > > the extents tree to be inconsistent. But the inconsistency just > > > > exists in memory and doesn't land on disk. > > > > > > > > For ext4_ext_get_access(), the handle must have been aborted > > > > when it returned an error, as follows: > > > ext4_ext_get_access > > > ext4_journal_get_write_access > > > __ext4_journal_get_write_access > > > err = jbd2_journal_get_write_access > > > if (err) > > > ext4_journal_abort_handle > > > > For ext4_ext_dirty(), since path->p_bh must not be null and handle > > > > must be valid, handle is aborted anyway when an error is returned: > > > ext4_ext_dirty > > > __ext4_ext_dirty > > > if (path->p_bh) > > > __ext4_handle_dirty_metadata > > > if (ext4_handle_valid(handle)) > > > err = jbd2_journal_dirty_metadata > > > if (!is_handle_aborted(handle) && WARN_ON_ONCE(err)) > > > ext4_journal_abort_handle > > > > Thus the extents tree will only be inconsistent in memory, so only > > > > the verified bit of the modified buffer needs to be cleared to avoid > > > > these inconsistent data being used in memory. > > > > > > > Regards, > > > Baokun > > Thanks for the explanation Baokun, so basically we only have the > > inconsitency in the memory. > > > > I do have a followup questions: > > > > So in the above example, after we have the error, we'll have the buffer > > for depth=0 marked as valid but pointing to the wrong ei_block. > It looks wrong here. When there is an error, the ei_block of the > unmodified buffer with depth=0 is the correct one, it is indeed > 'valid' and it is consistent with the disk. Only buffers that were Hey Baokun, Ahh I see now, I was looking at it the wrong way. So basically since depth 1 to 4 is inconsistent to the disk we mark then non verified so then subsequent lookups can act accordingly. Thanks for the explanation! I am in the middle of testing this patchset with xfstests on a POWERPC system with 64k page size. I'll let you know how that goes! Regards, Ojaswin > modified during the error process need to be checked. > > Regards, > Baokun > > > > In this case, can we have something like below: > > > > ----------------- > > ext4_ext_remove_space > > err = ext4_ext_rm_idx (error, path[0].p_bh inconsistent but verified) > > /* > > * we release buffers of the path but path[0].p_bh is not cleaned up > > * due to other references to it (possible?) > > */ > > > > ... at a later point...: > > > > ext4_find_extent > > bh = read_extent_tree_block() > > /* > > * we get the bh that was left inconsistent previously > > * since its verified, we dont check it again corrupting > > * the lookup > > */ > > > > ----------------- > > > > Is the above scenario possible? Or would the path[0].p_bh that was > > corrupted previously always be reread during the subsequent > > ext4_find_extent() lookup? > > > > Thanks again, > > Ojaswin >
