Re: [PATCH] ext4: fix extent status cache clearing when truncating inline data

[Jan Kara]

On Wed 14-10-26 10:17:14, Zhang Yi wrote:
> From: Zhang Yi <yi.zhang@xxxxxxxxxx>
> 
> ext4_inline_data_truncate() clears the whole extent status tree of the
> inode when it truncates the inline data, with
> 
> 	ext4_es_remove_extent(inode, 0, EXT_MAX_BLOCKS);
> 
> It does that when the inode still carries inline data while the
> EXT4_STATE_MAY_INLINE_DATA state bit is already cleared, which is the
> case once an inline to extent conversion has been initiated, for
> example a delalloc buffered write larger than the inline space.
> 
> The clear is meant to drop stale extent status entries of such an inode
> to prevent stale extents and stale reserved space after the conversion,
> but the range is too wide.  It also drops the entries of the blocks
> below the new end of file, that is the blocks that stay in the file.
> 
> When writeback of these blocks starts later, the lookup of such a block
> misses the extent status tree, falls back to the extent tree and caches
> a hole entry for it.  Those blocks then have dirty, mapped and delayed
> buffers together with a hole extent status entry.  This is an
> unexpected inconsistency.
> 
> This problem has been hidden so far because we allocate blocks and new
> extents for them during writeback.  But once [1] is merged during the
> iomap conversion, the writeback behavior will be normalized, and it will
> no longer allocate blocks for holes, thus exposing this problem.

Hum, I don't understand. So we have inode with inline data. We truncate it
(i.e. further reduce i_size). At this point we hold i_rwsem,
invalidate_lock, i_data_sem. So how can race with inline data
conversion?

								Honza

> 
> Therefore, restrict the range to the blocks that are actually truncated
> away, and pass EXT_MAX_BLOCKS minus lblk as the length.
> 
> [1] https://lore.kernel.org/linux-ext4/20261009103033.2920530-9-yi.zhang@xxxxxxxxxxxxxxx/
> 
> Reported-by: syzbot ci <syzbot+ci46f64f7b6a2a188e@xxxxxxxxxxxxxxxxxxxxxxxxx>
> Closes: https://lore.kernel.org/linux-ext4/6ac92a4c.70ed5d86.2ac85.0072.GAE@xxxxxxxxxx/
> Fixes: 0add491df4e5 ("ext4: remove extent cache entries when truncating inline data")
> Signed-off-by: Zhang Yi <yi.zhang@xxxxxxxxxx>
> ---
>  fs/ext4/inline.c | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/ext4/inline.c b/fs/ext4/inline.c
> index ceee69a66482b..b630e56815bb5 100644
> --- a/fs/ext4/inline.c
> +++ b/fs/ext4/inline.c
> @@ -1905,8 +1905,12 @@ int ext4_inline_data_truncate(struct inode *inode, int *has_inline)
>  		 * the extent status cache must be cleared to avoid leaving
>  		 * behind stale delayed allocated extent entries
>  		 */
> -		if (!ext4_test_inode_state(inode, EXT4_STATE_MAY_INLINE_DATA))
> -			ext4_es_remove_extent(inode, 0, EXT_MAX_BLOCKS);
> +		if (!ext4_test_inode_state(inode, EXT4_STATE_MAY_INLINE_DATA)) {
> +			ext4_lblk_t lblk = EXT4_B_TO_LBLK(inode, i_size);
> +
> +			ext4_es_remove_extent(inode, lblk,
> +					      EXT_MAX_BLOCKS - lblk);
> +		}
>  
>  		/* Clear the content in the xattr space. */
>  		if (inline_size > EXT4_MIN_INLINE_DATA_SIZE) {
> -- 
> 2.52.0
> 
-- 
Jan Kara <jack@xxxxxxxx>
SUSE Labs, CR