[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <9ef38162-2eeb-4cf6-aee4-02d6a5952757@huaweicloud.com>
Date: Tue, 16 Jul 2024 19:54:43 +0800
From: Baokun Li <libaokun@...weicloud.com>
To: Ojaswin Mujoo <ojaswin@...ux.ibm.com>
Cc: linux-ext4@...r.kernel.org, tytso@....edu, adilger.kernel@...ger.ca,
jack@...e.cz, ritesh.list@...il.com, linux-kernel@...r.kernel.org,
yi.zhang@...wei.com, yangerkun@...wei.com, Baokun Li <libaokun1@...wei.com>,
zhanchengbin <zhanchengbin1@...wei.com>, Baokun Li <libaokun@...weicloud.com>
Subject: Re: [PATCH 02/20] ext4: prevent partial update of the extents path
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
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
Powered by blists - more mailing lists