Re: [PATCH 02/20] ext4: prevent partial update of the extents path
From: Ojaswin Mujoo
Date: Tue Jul 16 2024 - 05:55:13 EST
> > 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.
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