[Ocfs2-tools-devel] [PATCH 2/5] Add "list-sparse" option in
tunefs.ocfs2, take 1
Mark Fasheh
mark.fasheh at oracle.com
Wed Oct 17 17:25:58 PDT 2007
On Wed, Oct 17, 2007 at 05:25:56PM +0800, tao.ma wrote:
> ocfs2 now support sparse files, so add a new option "list-sparse"
> to show all the sparse files and their holes in the volume.
That's a good idea Tao. Also, I'm glad you put that in a seperate
tunefs.ocfs2 file.
Can you provide a small description of the design approach you took in the
implementation of this patch? I've got some comments on the code below.
Overall, I feel that this could be a lot simpler.
> +/*
> + * iterate a extent list and output the total clusters occupied by the
> + * extent list. The holes will exist only in the leaf extent list.
> + *
> + * start and end indicate:
> + * the range of a file if the extent list belongs to the dinode;
> + * the range of the extent rec if the extent list belongs to a extent block.
> + */
> +static errcode_t iterate_extent_list(ocfs2_filesys *fs,
> + struct ocfs2_extent_list *el,
> + uint32_t start,
> + uint32_t end,
> + void (*func)(void *priv_data,
> + uint32_t hole_start,
> + uint32_t hole_end),
> + void *priv_data)
> +{
> + int i;
> + errcode_t ret = 0;
> + uint64_t blkno;
> + uint32_t first_rec_begin = 0, prev_end = 0;
> + struct ocfs2_extent_rec *rec = NULL;
> + char *eb_buf = NULL;
> + struct ocfs2_extent_block *eb = NULL;
> + struct ocfs2_extent_list *child_el = NULL;
> +
> + assert(func);
> +
> + if (el->l_tree_depth > 0) {
> +
> + ret = ocfs2_malloc_block(fs->fs_io, &eb_buf);
> + if (ret)
> + goto bail;
> +
> + for (i = 0; i <= el->l_next_free_rec; i++) {
> + rec = &el->l_recs[i];
> +
> + if (!ocfs2_rec_clusters(el->l_tree_depth, rec))
> + continue;
> +
> + blkno = rec->e_blkno;
> + ret = ocfs2_read_extent_block(fs, blkno, eb_buf);
> + if (ret)
> + goto bail;
> +
> + eb = (struct ocfs2_extent_block *)eb_buf;
> + child_el = &eb->h_list;
> +
> + ret = iterate_extent_list(fs, child_el, rec->e_cpos,
> + rec->e_cpos + rec->e_int_clusters,
> + func, priv_data);
> + if (ret)
> + goto bail;
> + }
> + } else {
> + for (i = 0; i < el->l_next_free_rec; i++) {
> + rec = &el->l_recs[i];
> +
> + if (!ocfs2_rec_clusters(el->l_tree_depth, rec))
> + continue;
> +
> + /*
> + * We have to consider the hole between the start of
> + * the first extent rec and the beginning of the extent
> + * list.
> + *
> + * We have to check prev_end here because if the
> + * first extent rec begins with 0, we don't want to
> + * calculate it every time.
> + */
> + if (!first_rec_begin && !prev_end) {
> + first_rec_begin = rec->e_cpos;
> + if (first_rec_begin > start)
> + func(priv_data, start, first_rec_begin);
> + }
> +
> + if (prev_end && prev_end < rec->e_cpos)
> + func(priv_data, prev_end, rec->e_cpos);
> +
> + prev_end = rec->e_cpos + rec->e_leaf_clusters;
> + }
> +
> + if (prev_end && prev_end < end)
> + func(priv_data, prev_end, end);
> + }
> +
> +bail:
> + if (eb_buf)
> + ocfs2_free(&eb_buf);
> +
> + return ret;
> +}
> +
> +static errcode_t tunefs_iterate_regular(ocfs2_filesys *fs,
> + struct ocfs2_dinode *di,
> + struct list_ctxt *ctxt)
> +{
> + errcode_t ret = 0;
> + uint32_t end_clusters;
> + struct multi_link_file *file = NULL;
> + struct ocfs2_extent_list *el = &di->id2.i_list;
> +
> + assert(S_ISREG(di->i_mode));
> +
> + if (di->i_links_count > 1) {
> + file = multi_link_file_lookup(ctxt, di->i_blkno);
> +
> + if (file) {
> + ctxt->file_hole_len = file->clusters;
> + ctxt->duplicated = 1;
> + goto print;
> + }
> + }
> +
> + end_clusters = (di->i_size + fs->fs_clustersize -1 ) /
> + fs->fs_clustersize;
> + ret = iterate_extent_list(fs, el, 0, end_clusters, ctxt->func, ctxt);
> + if (ret)
> + goto bail;
Why are we using such a complicated iterate function? What's wrong with just
looping through all clusters, using ocfs2_get_clusters() (which you can
export) to find where holes are? I'm thinking something closer to what
ocfs2_allocate_unwritten_extents() in kernel does, except you're printing
holes instead of filling them.
> +static int iterate_dir_entry(ocfs2_filesys *fs,
> + uint64_t blkno,
> + uint64_t bcount,
> + uint16_t flags,
> + void *priv_data)
> +{
> + errcode_t ret;
> + uint32_t offset = 0;
> + struct list_ctxt *ctxt;
> + char *dirblock_buf = NULL, *di_buf = NULL;
> + struct ocfs2_dir_entry *dirent = NULL;
> + struct ocfs2_dinode *di = NULL;
> + char file_name[OCFS2_MAX_FILENAME_LEN];
> + int file_name_len;
> +
> + ctxt = (struct list_ctxt *)priv_data;
> + strcpy(file_name, ctxt->file_name);
> + file_name_len = ctxt->file_name_len;
> +
> + ret = ocfs2_malloc_block(fs->fs_io, &dirblock_buf);
> + if (ret)
> + goto bail;
> +
> + ret = ocfs2_malloc_block(fs->fs_io, &di_buf);
> + if (ret)
> + goto bail;
> +
> + ret = ocfs2_read_dir_block(fs, blkno, dirblock_buf);
> + if (ret)
> + goto bail;
> +
> + while (offset < fs->fs_blocksize) {
> + dirent = (struct ocfs2_dir_entry *)(dirblock_buf + offset);
> +
> + /* skip "." and "..". */
> + if (dirent->name[0] == '.')
> + switch (dirent->name_len) {
> + default:
> + break;
> + case 2:
> + if (dirent->name[1] != '.')
> + break;
> + case 1:
> + goto next_ent;
> + }
> +
> + ret = ocfs2_read_inode(fs, (uint64_t)dirent->inode, di_buf);
> + if (ret)
> + goto bail;
> +
> + di = (struct ocfs2_dinode *)di_buf;
> +
> + /* currently, we only handle directories and regular files. */
> + if (S_ISDIR(di->i_mode) || S_ISREG(di->i_mode)) {
> + ctxt->file_hole_len = 0;
> + ctxt->duplicated = 0;
> + strcpy(ctxt->file_name, file_name);
> + ctxt->file_name_len = file_name_len;
> +
> + if (dirent->name_len + ctxt->file_name_len + 1 >=
> + OCFS2_MAX_FILENAME_LEN) {
> + goto bail;
> + }
> + strncat(ctxt->file_name,
> + dirent->name,dirent->name_len);
> + ctxt->file_name_len += dirent->name_len;
> +
> + if (S_ISDIR(di->i_mode)) {
> + strcat(ctxt->file_name,"/");
> + ctxt->file_name_len ++;
> + ret = tunefs_dir_iterate(fs, di, ctxt);
> + if (ret)
> + goto bail;
> + } else {
> + ret = tunefs_iterate_regular(fs, di, ctxt);
> + if (ret)
> + goto bail;
> + if (!ctxt->duplicated)
> + ctxt->total_clusters +=
> + ctxt->file_hole_len;
> + }
> +
> + }
> +
> +next_ent:
> + offset += dirent->rec_len;
> + }
> +
> +bail:
> + if (di_buf)
> + ocfs2_free(&di_buf);
> + if (dirblock_buf)
> + ocfs2_free(&dirblock_buf);
> + return ret;
> +}
> +
> +static errcode_t tunefs_dir_iterate(ocfs2_filesys *fs,
> + struct ocfs2_dinode *di,
> + struct list_ctxt *ctxt)
> +{
> + assert(S_ISDIR(di->i_mode));
> +
> + return ocfs2_block_iterate_inode(fs, di,
> + OCFS2_EXTENT_FLAG_DATA_ONLY,
> + iterate_dir_entry, ctxt);
We're not supposed to be using block_iterate or extent_iterate any more,
remember? It was your patch which marked those as deprecated ;)
Also, the open coded dirent walking has to go. None of this code is safe for
inline-data, or indexed directories (whenever that gets added).
Please use ocfs2_dir_iterate() instead - it should be easier to do that
anyway... If there's a problem with ocfs2_dir_iterate(), please let us know.
--Mark
--
Mark Fasheh
Senior Software Developer, Oracle
mark.fasheh at oracle.com
More information about the Ocfs2-tools-devel
mailing list