| 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: <BANLkTing0XEdEgoHs3v-gO8e+R42+qZCPg@mail.gmail.com>
Date: Thu, 21 Apr 2011 09:18:57 +0800
From: Yongqiang Yang <xiaoqiangnk@...il.com>
To: Allison Henderson <achender@...ux.vnet.ibm.com>
Cc: linux-ext4@...r.kernel.org, cmm@...ibm.com
Subject: Re: [PATCH RFC 2/3] ext4:Add two functions splitting an extent.
On Thu, Apr 21, 2011 at 1:21 AM, Allison Henderson
<achender@...ux.vnet.ibm.com> wrote:
> On 4/13/2011 10:40 PM, Yongqiang Yang wrote:
>> 1] Add a function named ext4_split_extent_at() which splits an extent
>> into two extents at given logical block.
>>
>> 2] Add a function called ext4_split_extent() which splits an extent
>> into three extents.
>>
>> Signed-off-by: Yongqiang Yang<xiaoqiangnk@...il.com>
>> ---
>> fs/ext4/extents.c | 200 +++++++++++++++++++++++++++++++++++++++++++++++++++++
>> 1 files changed, 200 insertions(+), 0 deletions(-)
>>
>> diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c
>> index 11f30d2..1756e0f 100644
>> --- a/fs/ext4/extents.c
>> +++ b/fs/ext4/extents.c
>> @@ -2554,6 +2554,206 @@ static int ext4_ext_zeroout(struct inode *inode, struct ext4_extent *ex)
>> return ret;
>> }
>>
>> +/*
>> + * used by extent splitting.
>> + */
>> +#define EXT4_EXT_MAY_ZEROOUT 0x1 /* safe to zeroout if split fails \
>> + due to ENOSPC */
>> +#define EXT4_EXT_MARK_UNINIT1 0x2 /* mark first half uninitialized */
>> +#define EXT4_EXT_MARK_UNINIT2 0x4 /* mark second half uninitialized */
>> +
>> +/*
>> + * ext4_split_extent_at() splits an extent at given block.
>> + *
>> + * @handle: the journal handle
>> + * @inode: the file inode
>> + * @path: the path to the extent
>> + * @split: the logical block where the extent is splitted.
>> + * @split_flags: indicates if the extent could be zeroout if split fails, and
>> + * the states(init or uninit) of new extents.
>> + * @flags: flags used to insert new extent to extent tree.
>> + *
>> + *
>> + * Splits extent [a, b] into two extents [a, @split) and [@split, b], states
>> + * of which are deterimined by split_flag.
>> + *
>> + * There are two cases:
>> + * a> the extent are splitted into two extent.
>> + * b> split is not needed, and just mark the extent.
>> + *
>> + * return 0 on success.
>> + */
>> +static int ext4_split_extent_at(handle_t *handle,
>> + struct inode *inode,
>> + struct ext4_ext_path *path,
>> + ext4_lblk_t split,
>> + int split_flag,
>> + int flags)
>> +{
>> + ext4_fsblk_t newblock;
>> + ext4_lblk_t ee_block;
>> + struct ext4_extent_header *eh;
>> + struct ext4_extent *ex, newex, orig_ex;
>> + struct ext4_extent *ex2 = NULL;
>> + unsigned int ee_len, depth;
>> + int err = 0;
>> +
>> + ext_debug("ext4_split_extents_at: inode %lu, logical"
>> + "block %llu\n", inode->i_ino, (unsigned long long)split);
>> +
>> + ext4_ext_show_leaf(inode, path);
>> +
>> + depth = ext_depth(inode);
>> + eh = path[depth].p_hdr;
>> + ex = path[depth].p_ext;
>> + ee_block = le32_to_cpu(ex->ee_block);
>> + ee_len = ext4_ext_get_actual_len(ex);
>> + newblock = split - ee_block + ext4_ext_pblock(ex);
>> +
>> + BUG_ON(split< ee_block || split>= (ee_block + ee_len));
>> +
>> + err = ext4_ext_get_access(handle, inode, path + depth);
>> + if (err)
>> + goto out;
>> +
>> + if (split == ee_block) {
>> + /*
>> + * case b: block @split is the block that the extent begins with
>> + * then we just change the state of the extent, and splitting
>> + * is not needed.
>> + */
>> + if (split_flag& EXT4_EXT_MARK_UNINIT2)
>> + ext4_ext_mark_uninitialized(ex);
>> + else
>> + ext4_ext_mark_initialized(ex);
>> +
>> + if (!(flags& EXT4_GET_BLOCKS_PRE_IO))
>> + ext4_ext_try_to_merge(inode, path, ex);
>> +
>> + err = ext4_ext_dirty(handle, inode, path + depth);
>> + goto out;
>> + }
>> +
>> + /* case a */
>> + orig_ex.ee_block = ex->ee_block;
>> + orig_ex.ee_len = ex->ee_len;
>> + ext4_ext_store_pblock(&orig_ex, ext4_ext_pblock(ex));
>> +
>> + ex->ee_len = cpu_to_le16(split - ee_block);
>> + if (split_flag& EXT4_EXT_MARK_UNINIT1)
>> + ext4_ext_mark_uninitialized(ex);
>> +
>> + /*
>> + * path may lead to new leaf, not to original leaf any more
>> + * after ext4_ext_insert_extent() returns,
>> + */
>> + err = ext4_ext_dirty(handle, inode, path + depth);
>> + if (err)
>> + goto fix_extent_len;
>> +
>> + ex2 =&newex;
>> + ex2->ee_block = cpu_to_le32(split);
>> + ex2->ee_len = cpu_to_le16(ee_len - (split - ee_block));
>> + ext4_ext_store_pblock(ex2, newblock);
>> + if (split_flag& EXT4_EXT_MARK_UNINIT2)
>> + ext4_ext_mark_uninitialized(ex2);
>> +
>> + err = ext4_ext_insert_extent(handle, inode, path,&newex, flags);
>> + if (err == -ENOSPC&& (EXT4_EXT_MAY_ZEROOUT& split_flag)) {
>> +
>> + err = ext4_ext_zeroout(inode,&orig_ex);
>> + if (err)
>> + goto fix_extent_len;
>> + /* update the extent length and mark as initialized */
>> + ex->ee_block = orig_ex.ee_block;
>> + ex->ee_len = orig_ex.ee_len;
>> + ext4_ext_mark_initialized(ex);
>> + ext4_ext_store_pblock(ex, ext4_ext_pblock(&orig_ex));
>> + err = ext4_ext_dirty(handle, inode, path + depth);
>> + goto out;
>> + } else if (err)
>> + goto fix_extent_len;
>> +
>> +out:
>> + ext4_ext_show_leaf(inode, path);
>> + return err;
>> +
>> +fix_extent_len:
>> + ex->ee_block = orig_ex.ee_block;
>> + ex->ee_len = orig_ex.ee_len;
>> + ext4_ext_store_pblock(ex, ext4_ext_pblock(&orig_ex));
>> + ext4_ext_dirty(handle, inode, path + depth);
>> + return err;
>> +}
>> +
>> +/*
>> + * ext4_split_extents() splits an extent and mark extent which is covered
>> + * by @map as split_flags indicates
>> + *
>> + * It may result in splitting the extent into multiple extents (upto three)
>> + * There are three possibilities:
>> + * a> There is no split required
>> + * b> Splits in two extents: Split is happening at either end of the extent
>> + * c> Splits in three extents: Somone is splitting in middle of the extent
>> + *
>> + */
>> +static int ext4_split_extent(handle_t *handle,
>> + struct inode *inode,
>> + struct ext4_ext_path *path,
>> + struct ext4_map_blocks *map,
>> + int split_flag,
>> + int flags)
>> +{
>> + ext4_lblk_t ee_block;
>> + struct ext4_extent *ex;
>> + unsigned int ee_len, depth;
>> + int err = 0;
>> + int uninitialized;
>> + int split_flag1, flags1;
>> +
>> + depth = ext_depth(inode);
>> + ex = path[depth].p_ext;
>> + ee_block = le32_to_cpu(ex->ee_block);
>> + ee_len = ext4_ext_get_actual_len(ex);
>> + uninitialized = ext4_ext_is_uninitialized(ex);
>> +
>> + if (map->m_lblk + map->m_len< ee_block + ee_len) {
>> + split_flag1 = 0;
>> + split_flag1 |= split_flag& EXT4_EXT_MAY_ZEROOUT ?
>> + EXT4_EXT_MAY_ZEROOUT : 0;
>> + flags1 = flags;
>> + flags1 |= EXT4_GET_BLOCKS_PRE_IO;
>> + if (uninitialized)
>> + split_flag1 |= EXT4_EXT_MARK_UNINIT1 |
>> + EXT4_EXT_MARK_UNINIT2;
>> + err = ext4_split_extent_at(handle, inode, path,
>> + map->m_lblk + map->m_len, split_flag1, flags1);
>> + }
>> +
>> + ext4_ext_drop_refs(path);
>> + path = ext4_ext_find_extent(inode, map->m_lblk, path);
>> + if (IS_ERR(path))
>> + return PTR_ERR(path);
>> +
>> + if (map->m_lblk>= ee_block) {
>> + split_flag1 = 0;
>> + split_flag1 |= split_flag& EXT4_EXT_MAY_ZEROOUT ?
>> + EXT4_EXT_MAY_ZEROOUT : 0;
>> + if (uninitialized)
>> + split_flag1 |= EXT4_EXT_MARK_UNINIT1;
>> + if (split_flag& EXT4_EXT_MARK_UNINIT2)
>> + split_flag1 |= EXT4_EXT_MARK_UNINIT2;
>> + err = ext4_split_extent_at(handle, inode, path,
>> + map->m_lblk, split_flag1, flags);
>> + if (err)
>> + goto out;
>> + }
>> +
>> + ext4_ext_show_leaf(inode, path);
>> +out:
>> + return err ? err : map->m_len;
>> +}
>> +
>> #define EXT4_EXT_ZERO_LEN 7
>> /*
>> * This function is called by ext4_ext_map_blocks() if someone tries to write
>
>
> Hi there,
>
> I've been working on trying to get the punch hole patch to work with with these new changes, but it looks like some test cases are not passing at the moment, so I'm trying track down where the issues are. I had to make some adjustments to this patch to fix one of the test cases. Here is what I did:
>
> ---
> :100644 100644 ee2dda3... c7d763d... M fs/ext4/extents.c
> fs/ext4/extents.c | 6 +++---
> 1 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c
> index ee2dda3..c7d763d 100644
> --- a/fs/ext4/extents.c
> +++ b/fs/ext4/extents.c
> @@ -2717,12 +2717,12 @@ static int ext4_split_extent(handle_t *handle,
> ee_len = ext4_ext_get_actual_len(ex);
> uninitialized = ext4_ext_is_uninitialized(ex);
>
> + flags1 = flags;
> + flags1 |= EXT4_GET_BLOCKS_PRE_IO;
Hi, the flag should be passed via parameters,
ext4_convert_to_initialized() uses this function also. so merge
should be done in this case.
If the patches has other problems needing me to fix, please let me know.
Thank you,
Yongqiang.
> if (map->m_lblk + map->m_len < ee_block + ee_len) {
> split_flag1 = 0;
> split_flag1 |= split_flag & EXT4_EXT_MAY_ZEROOUT ?
> EXT4_EXT_MAY_ZEROOUT : 0;
> - flags1 = flags;
> - flags1 |= EXT4_GET_BLOCKS_PRE_IO;
> if (uninitialized)
> split_flag1 |= EXT4_EXT_MARK_UNINIT1 |
> EXT4_EXT_MARK_UNINIT2;
> @@ -2744,7 +2744,7 @@ static int ext4_split_extent(handle_t *handle,
> if (split_flag & EXT4_EXT_MARK_UNINIT2)
> split_flag1 |= EXT4_EXT_MARK_UNINIT2;
> err = ext4_split_extent_at(handle, inode, path,
> - map->m_lblk, split_flag1, flags);
> + map->m_lblk, split_flag1, flags1);
> if (err)
> goto out;
> }
> --
> 1.7.1
>
>
> Let me know if this change works for you. It looks like the problem was that the extents were getting merged back together because the second ext4_split_extent_at didnt have the EXT4_GET_BLOCKS_PRE_IO flag, so this should fix it. There are still some other test cases that are showing some odd behavior, so I will keep you posted if I have to make any other changes.
>
> Allison Henderson
>
--
Best Wishes
Yongqiang Yang
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Powered by blists - more mailing lists