[Ocfs2-tools-devel] [PATCH 2/4] defrag.ocfs2: Pass 1: Defrag individual files and directories

[Joel Becker]

On Tue, May 11, 2010 at 11:02:34PM -0500, Goldwyn Rodrigues wrote:
> Defragging file -
> Allocate the maximum possible extent and copy the file into the
> newly allocated extent. If the allocated extent is smaller than
> the extent read from the file, write the collected data, and
> re-use the file extent. At any point, if the number of extents
> in the new inode increases than in the existing inode, abort.

	Finally, files.

> +static errcode_t calc_num_extents(ocfs2_filesys *fs,
> +		struct ocfs2_extent_list *el,
> +		uint32_t *ne)
> +{
> +	struct ocfs2_extent_block *eb;
> +	struct ocfs2_extent_rec *rec;
> +	errcode_t ret = 0;
> +	char *buf = NULL;
> +	int i;
> +	uint32_t clusters;
> +	uint32_t extents = 0;
> +
> +	*ne = 0;
> +        for (i = 0; i < el->l_next_free_rec; ++i) {
> +                rec = &(el->l_recs[i]);
> +                clusters = ocfs2_rec_clusters(el->l_tree_depth, rec);
> +
> +                /*
> +                 * In a unsuccessful insertion, we may shift a tree
> +                 * add a new branch for it and do no insertion. So we
> +                 * may meet a extent block which have
> +                 * clusters == 0, this should only be happen
> +                 * in the last extent rec. */
> +                if (!clusters && i == el->l_next_free_rec - 1)
> +                        break;
> +
> +                extents = 1;
> +
> +                if (el->l_tree_depth) {
> +                        ret = ocfs2_malloc_block(fs->fs_io, &buf);
> +                        if (ret)
> +                                goto bail;

	You've got a memory leak here.  You allocate a buffer for every
i, but only free the last buf at function exit.

> +static errcode_t copy_clusters(ocfs2_filesys *fs,
> +		uint64_t from, uint64_t to, int n)
> +{
> +	char *buf = NULL;
> +	int i, c = ocfs2_clusters_in_blocks(fs, 1);
> +	errcode_t ret = ocfs2_malloc_blocks(fs->fs_io, c, &buf);
> +	verbosef("Writing %d clusters to %"PRIu64" from %"PRIu64"\n", n,
> +			to, from);
> +	if (ret)
> +		return ret;
> +	for (i=0; i < n; i++) {
> +		ret = ocfs2_read_blocks(fs, from + i*c, c, buf);
> +		if (ret)
> +			goto out;
> +		ret = io_write_block(fs->fs_io, to + i*c, c, buf);
> +		if (ret)
> +			goto out;
> +	}
> +out:
> +	ocfs2_free(&buf);
> +	return ret;
> +}

	This will definitely be helped by running in BUFFERED mode ;-)

> +	/* Handle holes */
> +	if (c->expected_cpos < rec->e_cpos) {
> +		verbosef("Hole detected expected/rec cpos %d/%d\n",
> +				c->expected_cpos, rec->e_cpos);
> +
> +		if (c->c_offset) {
> +			verbosef("Adding collected extent rec p %d blk %"
> +				PRIu64"l %d\n", c->cpos, c->blkno, c->c_offset);
> +			ret = ocfs2_cached_inode_insert_extent(c->new_inode,
> +					c->cpos, c->blkno, c->c_offset, 0);
> +			if (ret) {
> +				com_err(whoami, ret,
> +						"while adding extent rec\n");
> +				c->status = DEFRAG_ERROR;
> +				return OCFS2_EXTENT_ABORT;
> +			}
> +			c->num_extents++;
> +			c->c_offset = 0;
> +		}
> +		c->expected_cpos = rec->e_cpos;
> +		c->clusters_left -= rec->e_cpos - c->expected_cpos;
> +		if (!c->clusters_left)
> +			return 0;
> +		ret = free_clusters(c);
> +		if (ret) {
> +			com_err(whoami, ret, "while freeing clusters\n");
> +			c->status = DEFRAG_ERROR;
> +			return OCFS2_EXTENT_ABORT;
> +		}
> +	}
> +
> +	c->expected_cpos += rec->e_leaf_clusters;
> +
> +	if (!c->c_end) {
> +		ret = alloc_clusters(c);
> +		if (ret) {
> +			com_err(whoami, ret, "while allocating clusters\n");
> +			c->status = DEFRAG_ERROR;
> +			return OCFS2_EXTENT_ABORT;
> +		}
> +
> +	}
> +
> +	/* Reuse clusters if they are bigger than allocated cluster */
> +	if (c->c_end <= rec->e_leaf_clusters) {
> +		if (c->c_offset) {
> +			verbosef("Adding collected extent rec p %d blk %"
> +				PRIu64"l %d\n", c->cpos, c->blkno, c->c_offset);
> +			ret = ocfs2_cached_inode_insert_extent(c->new_inode,
> +					c->cpos, c->blkno, c->c_offset, 0);
> +			if (ret) {
> +				com_err(whoami, ret,
> +						"while adding extent rec\n");
> +				c->status = DEFRAG_ERROR;
> +				return OCFS2_EXTENT_ABORT;
> +			}
> +			c->num_extents++;
> +		}
> +		verbosef("Adding old extent rec %d b %"PRIu64" l %d\n",
> +			rec->e_cpos, (uint64_t)rec->e_blkno, rec->e_leaf_clusters);
> +		ret = ocfs2_cached_inode_insert_extent(c->new_inode,
> +			rec->e_cpos, rec->e_blkno, rec->e_leaf_clusters, 0);
> +		if (ret) {
> +			com_err(whoami, ret, "while adding extent\n");
> +			c->status = DEFRAG_ERROR;
> +			return OCFS2_EXTENT_ABORT;
> +		}
> +		c->num_extents++;
> +		c->sc.rec[c->sc.num_recs].start = rec->e_blkno;
> +		c->sc.rec[c->sc.num_recs++].len = rec->e_leaf_clusters;
> +		if (c->sc.num_recs > MAX_RECS) {
> +			verbosef("Exceeded MAX_RECS limit %d\n",
> +					c->sc.num_recs);
> +			c->status = DEFRAG_ERROR;
> +			return OCFS2_EXTENT_ABORT;
> +		}
> +		c->cpos = rec->e_cpos + rec->e_leaf_clusters;
> +		c->clusters_left -= rec->e_leaf_clusters;
> +		ret = free_clusters(c);
> +		if (ret) {
> +			com_err(whoami, ret, "while freeing clusters\n");
> +			c->status = DEFRAG_ERROR;
> +			return OCFS2_EXTENT_ABORT;
> +		}
> +		return 0;
> +	}
> +
> +	/* Remaining clusters short. Fill allocated clusters and write */
> +	nc = 0;
> +	left = rec->e_leaf_clusters;
> +	while (c->c_end < c->c_offset + left) {
> +		/* Copy enough to fill the rest of the block */
> +		nc = c->c_end - c->c_offset;
> +		if (nc) {
> +			ret = copy_clusters(c->dst->dst_fs, rec->e_blkno,
> +					c->blkno + b*c->c_offset, nc);

	Can we get better names than 'b'?  Also, please use parenthesis
in expressions like "while (c->c_end < (c->c_offset + left))" and such.
It makes it easier to read.

Joel

-- 

Life's Little Instruction Book #347

	"Never waste the oppourtunity to tell someone you love them."

Joel Becker
Consulting Software Developer
Oracle
E-mail: joel.becker at oracle.com
Phone: (650) 506-8127