| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20240806143836.a5cwdbvyehsjqwp3@quack3>
Date: Tue, 6 Aug 2024 16:38:36 +0200
From: Jan Kara <jack@...e.cz>
To: Zhang Yi <yi.zhang@...weicloud.com>
Cc: linux-ext4@...r.kernel.org, linux-fsdevel@...r.kernel.org,
linux-kernel@...r.kernel.org, tytso@....edu,
adilger.kernel@...ger.ca, jack@...e.cz, ritesh.list@...il.com,
yi.zhang@...wei.com, chengzhihao1@...wei.com, yukuai3@...wei.com
Subject: Re: [PATCH v2 01/10] ext4: factor out ext4_map_create_blocks() to
allocate new blocks
On Fri 02-08-24 19:51:11, Zhang Yi wrote:
> From: Zhang Yi <yi.zhang@...wei.com>
>
> Factor out a common helper ext4_map_create_blocks() from
> ext4_map_blocks() to do a real blocks allocation, no logic changes.
>
> Signed-off-by: Zhang Yi <yi.zhang@...wei.com>
Looks OK. Feel free to add:
Reviewed-by: Jan Kara <jack@...e.cz>
Honza
> ---
> fs/ext4/inode.c | 157 +++++++++++++++++++++++++-----------------------
> 1 file changed, 81 insertions(+), 76 deletions(-)
>
> diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
> index 941c1c0d5c6e..112aec171ee9 100644
> --- a/fs/ext4/inode.c
> +++ b/fs/ext4/inode.c
> @@ -482,6 +482,86 @@ static int ext4_map_query_blocks(handle_t *handle, struct inode *inode,
> return retval;
> }
>
> +static int ext4_map_create_blocks(handle_t *handle, struct inode *inode,
> + struct ext4_map_blocks *map, int flags)
> +{
> + struct extent_status es;
> + unsigned int status;
> + int err, retval = 0;
> +
> + /*
> + * Here we clear m_flags because after allocating an new extent,
> + * it will be set again.
> + */
> + map->m_flags &= ~EXT4_MAP_FLAGS;
> +
> + /*
> + * We need to check for EXT4 here because migrate could have
> + * changed the inode type in between.
> + */
> + if (ext4_test_inode_flag(inode, EXT4_INODE_EXTENTS)) {
> + retval = ext4_ext_map_blocks(handle, inode, map, flags);
> + } else {
> + retval = ext4_ind_map_blocks(handle, inode, map, flags);
> +
> + /*
> + * We allocated new blocks which will result in i_data's
> + * format changing. Force the migrate to fail by clearing
> + * migrate flags.
> + */
> + if (retval > 0 && map->m_flags & EXT4_MAP_NEW)
> + ext4_clear_inode_state(inode, EXT4_STATE_EXT_MIGRATE);
> + }
> + if (retval <= 0)
> + return retval;
> +
> + if (unlikely(retval != map->m_len)) {
> + ext4_warning(inode->i_sb,
> + "ES len assertion failed for inode %lu: "
> + "retval %d != map->m_len %d",
> + inode->i_ino, retval, map->m_len);
> + WARN_ON(1);
> + }
> +
> + /*
> + * We have to zeroout blocks before inserting them into extent
> + * status tree. Otherwise someone could look them up there and
> + * use them before they are really zeroed. We also have to
> + * unmap metadata before zeroing as otherwise writeback can
> + * overwrite zeros with stale data from block device.
> + */
> + if (flags & EXT4_GET_BLOCKS_ZERO &&
> + map->m_flags & EXT4_MAP_MAPPED && map->m_flags & EXT4_MAP_NEW) {
> + err = ext4_issue_zeroout(inode, map->m_lblk, map->m_pblk,
> + map->m_len);
> + if (err)
> + return err;
> + }
> +
> + /*
> + * If the extent has been zeroed out, we don't need to update
> + * extent status tree.
> + */
> + if (flags & EXT4_GET_BLOCKS_PRE_IO &&
> + ext4_es_lookup_extent(inode, map->m_lblk, NULL, &es)) {
> + if (ext4_es_is_written(&es))
> + return retval;
> + }
> +
> + status = map->m_flags & EXT4_MAP_UNWRITTEN ?
> + EXTENT_STATUS_UNWRITTEN : EXTENT_STATUS_WRITTEN;
> + if (!(flags & EXT4_GET_BLOCKS_DELALLOC_RESERVE) &&
> + !(status & EXTENT_STATUS_WRITTEN) &&
> + ext4_es_scan_range(inode, &ext4_es_is_delayed, map->m_lblk,
> + map->m_lblk + map->m_len - 1))
> + status |= EXTENT_STATUS_DELAYED;
> +
> + ext4_es_insert_extent(inode, map->m_lblk, map->m_len,
> + map->m_pblk, status);
> +
> + return retval;
> +}
> +
> /*
> * The ext4_map_blocks() function tries to look up the requested blocks,
> * and returns if the blocks are already mapped.
> @@ -630,12 +710,6 @@ int ext4_map_blocks(handle_t *handle, struct inode *inode,
> if (!(flags & EXT4_GET_BLOCKS_CONVERT_UNWRITTEN))
> return retval;
>
> - /*
> - * Here we clear m_flags because after allocating an new extent,
> - * it will be set again.
> - */
> - map->m_flags &= ~EXT4_MAP_FLAGS;
> -
> /*
> * New blocks allocate and/or writing to unwritten extent
> * will possibly result in updating i_data, so we take
> @@ -643,76 +717,7 @@ int ext4_map_blocks(handle_t *handle, struct inode *inode,
> * with create == 1 flag.
> */
> down_write(&EXT4_I(inode)->i_data_sem);
> -
> - /*
> - * We need to check for EXT4 here because migrate
> - * could have changed the inode type in between
> - */
> - if (ext4_test_inode_flag(inode, EXT4_INODE_EXTENTS)) {
> - retval = ext4_ext_map_blocks(handle, inode, map, flags);
> - } else {
> - retval = ext4_ind_map_blocks(handle, inode, map, flags);
> -
> - if (retval > 0 && map->m_flags & EXT4_MAP_NEW) {
> - /*
> - * We allocated new blocks which will result in
> - * i_data's format changing. Force the migrate
> - * to fail by clearing migrate flags
> - */
> - ext4_clear_inode_state(inode, EXT4_STATE_EXT_MIGRATE);
> - }
> - }
> -
> - if (retval > 0) {
> - unsigned int status;
> -
> - if (unlikely(retval != map->m_len)) {
> - ext4_warning(inode->i_sb,
> - "ES len assertion failed for inode "
> - "%lu: retval %d != map->m_len %d",
> - inode->i_ino, retval, map->m_len);
> - WARN_ON(1);
> - }
> -
> - /*
> - * We have to zeroout blocks before inserting them into extent
> - * status tree. Otherwise someone could look them up there and
> - * use them before they are really zeroed. We also have to
> - * unmap metadata before zeroing as otherwise writeback can
> - * overwrite zeros with stale data from block device.
> - */
> - if (flags & EXT4_GET_BLOCKS_ZERO &&
> - map->m_flags & EXT4_MAP_MAPPED &&
> - map->m_flags & EXT4_MAP_NEW) {
> - ret = ext4_issue_zeroout(inode, map->m_lblk,
> - map->m_pblk, map->m_len);
> - if (ret) {
> - retval = ret;
> - goto out_sem;
> - }
> - }
> -
> - /*
> - * If the extent has been zeroed out, we don't need to update
> - * extent status tree.
> - */
> - if ((flags & EXT4_GET_BLOCKS_PRE_IO) &&
> - ext4_es_lookup_extent(inode, map->m_lblk, NULL, &es)) {
> - if (ext4_es_is_written(&es))
> - goto out_sem;
> - }
> - status = map->m_flags & EXT4_MAP_UNWRITTEN ?
> - EXTENT_STATUS_UNWRITTEN : EXTENT_STATUS_WRITTEN;
> - if (!(flags & EXT4_GET_BLOCKS_DELALLOC_RESERVE) &&
> - !(status & EXTENT_STATUS_WRITTEN) &&
> - ext4_es_scan_range(inode, &ext4_es_is_delayed, map->m_lblk,
> - map->m_lblk + map->m_len - 1))
> - status |= EXTENT_STATUS_DELAYED;
> - ext4_es_insert_extent(inode, map->m_lblk, map->m_len,
> - map->m_pblk, status);
> - }
> -
> -out_sem:
> + retval = ext4_map_create_blocks(handle, inode, map, flags);
> up_write((&EXT4_I(inode)->i_data_sem));
> if (retval > 0 && map->m_flags & EXT4_MAP_MAPPED) {
> ret = check_block_validity(inode, map);
> --
> 2.39.2
>
--
Jan Kara <jack@...e.com>
SUSE Labs, CR
Powered by blists - more mailing lists