[Ocfs2-tools-devel] [RFC 1/8] dx_dirs v4: add indexed dirs support in fsck.ocfs2

Tao Ma tao.ma at oracle.com
Thu Jan 21 19:09:18 PST 2010



Coly Li wrote:
> This patch does,
> - Make fsck.ocfs2 understand indexed dir related blocks/clusters in pass1.
> - If a directory is changed in pass2, record the directory inode into a rb-tree.
> - Iterate all directory inodes from the rb-tree, if the directory data is not inlined, rebuild its indexed tree.
> 
> Signed-off-by: Coly Li <coly.li at suse.de>
> ---
>  dirblocks.c         |  130 ++++++++++++++++++++++++++++++++++++++++++++++++++--
>  include/dirblocks.h |    4 +
>  pass1.c             |   54 +++++++++++++++++++++
>  pass2.c             |   33 +++++++++++++
>  4 files changed, 216 insertions(+), 5 deletions(-)
> 
> diff --git a/fsck.ocfs2/dirblocks.c b/fsck.ocfs2/dirblocks.c
> index 1fd5560..17bd9bb 100644
> --- a/fsck.ocfs2/dirblocks.c
> +++ b/fsck.ocfs2/dirblocks.c
> @@ -34,6 +34,7 @@
>  #include "fsck.h"
>  #include "dirblocks.h"
>  #include "util.h"
> +#include "extent.h"
> 
>  errcode_t o2fsck_add_dir_block(o2fsck_dirblocks *db, uint64_t ino,
>  			       uint64_t blkno, uint64_t blkcount)
> @@ -43,11 +44,9 @@ errcode_t o2fsck_add_dir_block(o2fsck_dirblocks *db, uint64_t ino,
>  	o2fsck_dirblock_entry *dbe, *tmp_dbe;
>  	errcode_t ret = 0;
> 
> -	dbe = calloc(1, sizeof(*dbe));
> -	if (dbe == NULL) {
> -		ret = OCFS2_ET_NO_MEMORY;
> -		goto out;
> -	}
> +	ret = ocfs2_malloc0(sizeof(o2fsck_dirblock_entry), &dbe);
> +	if (ret)
> +       		goto out;
> 
>  	dbe->e_ino = ino;
>  	dbe->e_blkno = blkno;
> @@ -134,6 +133,75 @@ static int try_to_cache(ocfs2_filesys *fs, struct rb_node *node,
>  	return cached_blocks;
>  }
> 
> +uint64_t o2fsck_search_reidx_dir(struct rb_root *root, uint64_t dino)
> +{
> +	struct rb_node *node = root->rb_node;
> +	o2fsck_dirblock_entry *dbe;
> +
> +	while (node) {
> +		dbe = rb_entry(node, o2fsck_dirblock_entry, e_node);
> +
> +		if (dino < dbe->e_ino)
> +			node = node->rb_left;
> +		else if (dino > dbe->e_ino)
> +			node = node->rb_right;
> +		else
> +			return dbe->e_ino;
> +	}
> +	return 0;
> +}
> +
> +static errcode_t o2fsck_add_reidx_dir_ino(struct rb_root *root, uint64_t dino)
> +{
> +	struct rb_node **p = &root->rb_node;
> +	struct rb_node *parent = NULL;
> +	o2fsck_dirblock_entry *dp, *tmp_dp;
> +	errcode_t ret = 0;
> +
> +	ret = ocfs2_malloc0(sizeof (o2fsck_dirblock_entry), &dp);
> +	if (ret)
> +		goto out;
> +
> +	dp->e_ino = dino;
> +
> +	while(*p)
> +	{
> +		parent = *p;
> +		tmp_dp = rb_entry(parent, o2fsck_dirblock_entry, e_node);
> +
> +		if (dp->e_ino < tmp_dp->e_ino)
> +			p = &(*p)->rb_left;
> +		else if (dp->e_ino > tmp_dp->e_ino)
> +			p = &(*p)->rb_right;
> +		else {
> +			ret = OCFS2_ET_INTERNAL_FAILURE;
> +			ocfs2_free(&dp);
> +			goto out;
> +		}
> +	}
> +
> +	rb_link_node(&dp->e_node, parent, p);
> +	rb_insert_color(&dp->e_node, root);
> +
> +out:
> +	return ret;
> +}
> +
> +errcode_t o2fsck_try_add_reidx_dir(struct rb_root *root, uint64_t dino)
> +{
> +	errcode_t ret = 1;
> +	uint64_t ino;
> +	ino = o2fsck_search_reidx_dir(root, dino);
> +	if (ino) {
> +		ret = 0;
> +		goto out;
> +	}
> +	ret = o2fsck_add_reidx_dir_ino(root, dino);
Joel has pointed that the initializtion of "ret" should be a really 
error. not 1.
I am just curious why you can't do like this:
	errcode_t ret = 0;
	uint64_t ino;
	ino = o2fsck_search_reidx_dir(root, dino);
	if (ino)
		goto out;
In this case, you don't need to set "ret = 0" if ino is found.

> +
> +out:
> +	return ret;
> +}
> +
>  void o2fsck_dir_block_iterate(o2fsck_state *ost, dirblock_iterator func,
>  			      void *priv_data)
>  {
> @@ -174,3 +242,55 @@ void o2fsck_dir_block_iterate(o2fsck_state *ost, dirblock_iterator func,
>  	if (pre_cache_buf)
>  		ocfs2_free(&pre_cache_buf);
>  }
> +
> +static errcode_t ocfs2_rebuild_indexed_dir(ocfs2_filesys *fs, uint64_t ino)
> +{
> +	errcode_t ret = 0;
> +	char *di_buf = NULL;
> +	struct ocfs2_dinode *di;
> +
> +
> +	ret = ocfs2_malloc_block(fs->fs_io, &di_buf);
> +	if (ret)
> +		goto out;
> +	
> +	ret = ocfs2_read_inode(fs, ino, di_buf);
> +	if (ret)
> +		goto out;
> +	di = (struct ocfs2_dinode *)di_buf;
> +
> +	/* do not rebuild indexed tree for inline directory */
> +	if (di->i_dyn_features & OCFS2_INLINE_DATA_FL)
> +		goto out;
> +	ocfs2_free(&di_buf);
this is a bug. you don't reset di_buf here. so in "out" you will double 
free the same memory. Actually you don't need to free di_buf since you 
always exit from "out".
> +
> +	ret = ocfs2_dx_dir_truncate(fs, ino);
> +	if (ret)
> +		goto out;
> +
> +	ret = ocfs2_dx_dir_build(fs, ino);
> +out:
> +	if (di_buf)
> +		ocfs2_free(&di_buf);
> +	return ret;
> +}
> +
> +
> +errcode_t o2fsck_rebuild_indexed_dirs(ocfs2_filesys *fs, struct rb_root *root)
> +{
> +	struct rb_node *node;
> +	o2fsck_dirblock_entry *dbe;
> +	uint64_t ino;
> +	errcode_t ret;
> +
> +	for (node = rb_first(root); node; node = rb_next(node)) {
> +		dbe = rb_entry(node, o2fsck_dirblock_entry, e_node);
> +		ino = dbe->e_ino;
> +		ret = ocfs2_rebuild_indexed_dir(fs, ino);
> +		if (ret)
> +			goto out;
> +	}
> +out:
> +	return ret;
> +}
> +
> diff --git a/fsck.ocfs2/include/dirblocks.h b/fsck.ocfs2/include/dirblocks.h
> index 7b3a2e9..f85974f 100644
> --- a/fsck.ocfs2/include/dirblocks.h
> +++ b/fsck.ocfs2/include/dirblocks.h
> @@ -48,6 +48,10 @@ struct _o2fsck_state;
>  void o2fsck_dir_block_iterate(struct _o2fsck_state *ost, dirblock_iterator func,
>                                void *priv_data);
>  		
> +uint64_t o2fsck_search_reidx_dir(struct rb_root *root, uint64_t dino);
> +errcode_t o2fsck_try_add_reidx_dir(struct rb_root *root, uint64_t dino);
> +errcode_t o2fsck_rebuild_indexed_dirs(ocfs2_filesys *fs, struct rb_root *root);
> +errcode_t o2fsck_check_dir_index(struct _o2fsck_state *ost, struct ocfs2_dinode *di);
> 
>  #endif /* __O2FSCK_DIRBLOCKS_H__ */
> 
> diff --git a/fsck.ocfs2/pass1.c b/fsck.ocfs2/pass1.c
> index db4225b..c8804d6 100644
> --- a/fsck.ocfs2/pass1.c
> +++ b/fsck.ocfs2/pass1.c
> @@ -760,6 +760,53 @@ static int clear_block(ocfs2_filesys *fs,
>  	return 0;
>  }
> 
> +
> +static errcode_t o2fsck_check_dx_dir(o2fsck_state *ost, struct ocfs2_dinode *di)
> +{
> +	errcode_t ret = 0;
> +	char *buf = NULL;
> +	struct ocfs2_dx_root_block *dx_root;
> +	ocfs2_filesys *fs = ost->ost_fs;
> +	struct extent_info ei = {0,};
> +	int changed = 0;
> +
> +	if (!ocfs2_supports_indexed_dirs(OCFS2_RAW_SB(fs->fs_super)))
> +		goto out;
> +
> +	if (!ocfs2_dir_indexed(di))
> +		goto out;
> +
> +	ret = ocfs2_malloc_block(fs->fs_io, &buf);
> +	if (ret)
> +		goto out;
> +
> +	ret = ocfs2_read_dx_root(fs, (uint64_t)di->i_dx_root, buf);
> +	if (ret)
> +		goto out;
> +
> +	dx_root = (struct ocfs2_dx_root_block *)buf;
> +	if (dx_root->dr_flags & OCFS2_DX_FLAG_INLINE)
> +		goto out;
> +
> +	ret = check_el(ost, &ei, di, &dx_root->dr_list,
> +			ocfs2_extent_recs_per_dx_root(fs->fs_blocksize),
> +			&changed);
Just FYI. check_el changed somehow if my refcount tree get committed.
> +	if (ret)
> +		goto out;
> +
> +	if (changed) {
> +		ret = ocfs2_write_dx_root(fs, (uint64_t)di->i_dx_root, (char *)dx_root);
> +		if (ret)
> +			com_err(whoami, ret, "while writing an updated "
> +				"dx_root block at %"PRIu64" for inode %"PRIu64,
> +				(uint64_t)di->i_dx_root, (uint64_t)di->i_blkno);
> +	}
> +out:
> +	if (buf)
> +		ocfs2_free(&buf);
> +	return ret;
> +}
> +
>  /*
>   * this verifies i_size and i_clusters for inodes that use i_list to
>   * reference extents of data.
> @@ -815,6 +862,13 @@ static errcode_t o2fsck_check_blocks(ocfs2_filesys *fs, o2fsck_state *ost,
>  		goto out;
>  	}
> 
> +	ret = o2fsck_check_dx_dir(ost, di);
> +	if (ret) {
> +		com_err(whoami, ret, "while iterating over the dir indexed "
> +			"tree for directory inode %"PRIu64, (uint64_t)di->i_blkno);
> +		goto out;
> +	}
> +
>  	if (S_ISLNK(di->i_mode))
>  		check_link_data(&vb);
> 
> diff --git a/fsck.ocfs2/pass2.c b/fsck.ocfs2/pass2.c
> index d61c501..a018e13 100644
> --- a/fsck.ocfs2/pass2.c
> +++ b/fsck.ocfs2/pass2.c
> @@ -36,6 +36,7 @@
>  #include <time.h>
> 
>  #include "ocfs2/ocfs2.h"
> +#include "ocfs2/kernel-rbtree.h"
> 
>  #include "dirparents.h"
>  #include "icount.h"
> @@ -70,6 +71,7 @@ struct dirblock_data {
>  	errcode_t	ret;
>  	o2fsck_strings	strings;
>  	uint64_t	last_ino;
> +	struct rb_root	re_idx_dirs;
>  };
> 
>  static int dirent_has_dots(struct ocfs2_dir_entry *dirent, int num_dots)
> @@ -851,6 +853,16 @@ next:
>  			com_err(whoami, ret, "while writing dir block %"PRIu64,
>  				dbe->e_blkno);
>  			dd->ost->ost_write_error = 1;
> +			goto out;
> +		}
> +
> +		if (ocfs2_supports_indexed_dirs(OCFS2_RAW_SB(dd->fs->fs_super)) &&
> +		    !(di->i_dyn_features & OCFS2_INLINE_DATA_FL)) {
> +			ret = o2fsck_try_add_reidx_dir(&dd->re_idx_dirs, dbe->e_ino);
> +			if (ret)
> +				com_err(whoami, ret, "while adding block for "
> +					"directory inode %"PRIu64" to rebuild "
> +					"dir index", dbe->e_ino);
>  		}
>  	}
> 
> @@ -860,6 +872,18 @@ out:
>  	return ret_flags;
>  }
> 
> +static void release_re_idx_dirs_rbtree(struct rb_root * root)
> +{
> +	struct rb_node *node;
> +	o2fsck_dirblock_entry *dp;
> +
> +	while ((node = rb_first(root)) != NULL) {
> +		dp = rb_entry(node, o2fsck_dirblock_entry, e_node);
> +		rb_erase(&dp->e_node, root);
> +		free(dp);
please use ocfs2_free since you use ocfs2_malloc0.

Regards,
Tao



More information about the Ocfs2-tools-devel mailing list