| 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: <20130711124112.GA6427@gmail.com>
Date: Thu, 11 Jul 2013 20:41:13 +0800
From: Zheng Liu <gnehzuil.liu@...il.com>
To: Theodore Ts'o <tytso@....edu>
Cc: Ext4 Developers List <linux-ext4@...r.kernel.org>
Subject: Re: [RFC PATCH 1/3] ext4: refactor code to read the extent tree block
On Thu, Jul 11, 2013 at 12:39:00AM -0400, Theodore Ts'o wrote:
> Refactor out the code needed to read the extent tree block into a
> single read_extent_tree_block() function. In addition to simplifying
> the code, it also makes sure that we call the ext4_ext_load_extent
> tracepoint whenever we need to read an extent tree block from disk.
>
> Signed-off-by: "Theodore Ts'o" <tytso@....edu>
The patch looks good to me.
Reviewed-by: Zheng Liu <wenqing.lz@...bao.com>
- Zheng
> ---
> fs/ext4/extents.c | 100 ++++++++++++++++++++++++------------------------------
> 1 file changed, 45 insertions(+), 55 deletions(-)
>
> diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c
> index 7097b0f..e174814 100644
> --- a/fs/ext4/extents.c
> +++ b/fs/ext4/extents.c
> @@ -464,25 +464,41 @@ int ext4_ext_check_inode(struct inode *inode)
> return ext4_ext_check(inode, ext_inode_hdr(inode), ext_depth(inode));
> }
>
> -static int __ext4_ext_check_block(const char *function, unsigned int line,
> - struct inode *inode,
> - struct ext4_extent_header *eh,
> - int depth,
> - struct buffer_head *bh)
> +static struct buffer_head *
> +__read_extent_tree_block(const char *function, unsigned int line,
> + struct inode *inode, ext4_fsblk_t pblk, int depth)
> {
> - int ret;
> + struct buffer_head *bh;
> + int err;
> +
> + BUG_ON(!rwsem_is_locked(&EXT4_I(inode)->i_data_sem));
> +
> + bh = sb_getblk(inode->i_sb, pblk);
> + if (unlikely(!bh))
> + return ERR_PTR(-ENOMEM);
>
> + if (!bh_uptodate_or_lock(bh)) {
> + trace_ext4_ext_load_extent(inode, pblk, _RET_IP_);
> + err = bh_submit_read(bh);
> + if (err < 0)
> + goto errout;
> + }
> if (buffer_verified(bh))
> - return 0;
> - ret = ext4_ext_check(inode, eh, depth);
> - if (ret)
> - return ret;
> + return bh;
> + err = __ext4_ext_check(function, line, inode,
> + ext_block_hdr(bh), depth);
> + if (err)
> + goto errout;
> set_buffer_verified(bh);
> - return ret;
> + return bh;
> +errout:
> + put_bh(bh);
> + return ERR_PTR(err);
> +
> }
>
> -#define ext4_ext_check_block(inode, eh, depth, bh) \
> - __ext4_ext_check_block(__func__, __LINE__, inode, eh, depth, bh)
> +#define read_extent_tree_block(inode, pblk, depth) \
> + __read_extent_tree_block(__func__, __LINE__, (inode), (pblk), (depth))
>
> #ifdef EXT_DEBUG
> static void ext4_ext_show_path(struct inode *inode, struct ext4_ext_path *path)
> @@ -748,20 +764,12 @@ ext4_ext_find_extent(struct inode *inode, ext4_lblk_t block,
> path[ppos].p_depth = i;
> path[ppos].p_ext = NULL;
>
> - bh = sb_getblk(inode->i_sb, path[ppos].p_block);
> - if (unlikely(!bh)) {
> - ret = -ENOMEM;
> + bh = read_extent_tree_block(inode, path[ppos].p_block, --i);
> + if (IS_ERR(bh)) {
> + ret = PTR_ERR(bh);
> goto err;
> }
> - if (!bh_uptodate_or_lock(bh)) {
> - trace_ext4_ext_load_extent(inode, block,
> - path[ppos].p_block);
> - ret = bh_submit_read(bh);
> - if (ret < 0) {
> - put_bh(bh);
> - goto err;
> - }
> - }
> +
> eh = ext_block_hdr(bh);
> ppos++;
> if (unlikely(ppos > depth)) {
> @@ -773,11 +781,6 @@ ext4_ext_find_extent(struct inode *inode, ext4_lblk_t block,
> }
> path[ppos].p_bh = bh;
> path[ppos].p_hdr = eh;
> - i--;
> -
> - ret = ext4_ext_check_block(inode, eh, i, bh);
> - if (ret < 0)
> - goto err;
> }
>
> path[ppos].p_depth = i;
> @@ -1412,29 +1415,21 @@ got_index:
> ix++;
> block = ext4_idx_pblock(ix);
> while (++depth < path->p_depth) {
> - bh = sb_bread(inode->i_sb, block);
> - if (bh == NULL)
> - return -EIO;
> - eh = ext_block_hdr(bh);
> /* subtract from p_depth to get proper eh_depth */
> - if (ext4_ext_check_block(inode, eh,
> - path->p_depth - depth, bh)) {
> - put_bh(bh);
> - return -EIO;
> - }
> + bh = read_extent_tree_block(inode, *logical,
> + path->p_depth - depth);
> + if (IS_ERR(bh))
> + return PTR_ERR(bh);
> + eh = ext_block_hdr(bh);
> ix = EXT_FIRST_INDEX(eh);
> block = ext4_idx_pblock(ix);
> put_bh(bh);
> }
>
> - bh = sb_bread(inode->i_sb, block);
> - if (bh == NULL)
> - return -EIO;
> + bh = read_extent_tree_block(inode, *logical, path->p_depth - depth);
> + if (IS_ERR(bh))
> + return PTR_ERR(bh);
> eh = ext_block_hdr(bh);
> - if (ext4_ext_check_block(inode, eh, path->p_depth - depth, bh)) {
> - put_bh(bh);
> - return -EIO;
> - }
> ex = EXT_FIRST_EXTENT(eh);
> found_extent:
> *logical = le32_to_cpu(ex->ee_block);
> @@ -2829,21 +2824,16 @@ again:
> ext_debug("move to level %d (block %llu)\n",
> i + 1, ext4_idx_pblock(path[i].p_idx));
> memset(path + i + 1, 0, sizeof(*path));
> - bh = sb_bread(sb, ext4_idx_pblock(path[i].p_idx));
> - if (!bh) {
> - /* should we reset i_size? */
> - err = -EIO;
> + bh = read_extent_tree_block(inode,
> + ext4_idx_pblock(path[i].p_idx), depth - i - 1);
> + if (IS_ERR(bh)) {
> + err = PTR_ERR(bh);
> break;
> }
> if (WARN_ON(i + 1 > depth)) {
> err = -EIO;
> break;
> }
> - if (ext4_ext_check_block(inode, ext_block_hdr(bh),
> - depth - i - 1, bh)) {
> - err = -EIO;
> - break;
> - }
> path[i + 1].p_bh = bh;
>
> /* save actual number of indexes since this
> --
> 1.7.12.rc0.22.gcdd159b
>
--
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