On Tue, Jun 30, 2009 at 12:41:33AM +0200, Andreas Dilger wrote: > I was looking at the function ext4_ext_calc_credits_for_insert(), because > it appeared similar to a function we were using in Lustre and I wanted to > remove redundant code from our patches. On closer inspection, however, > that function is doing something strange that wasn't in our original code. > > int ext4_ext_calc_credits_for_single_extent(struct inode *inode, int nrblocks, > struct ext4_ext_path *path) > { > if (path) { > int depth = ext_depth(inode); > int ret = 0; > > /* probably there is space in leaf? */ > if (le16_to_cpu(path[depth].p_hdr->eh_entries) > < le16_to_cpu(path[depth].p_hdr->eh_max)) { > > /* > * There are some space in the leaf tree, no > * need to account for leaf block credit > * > * bitmaps and block group descriptor blocks > * and other metadat blocks still need to be > * accounted. > */ > /* 1 bitmap, 1 block group descriptor */ > ret = 2 + EXT4_META_TRANS_BLOCKS(inode->i_sb); > } > } > > return ext4_chunk_trans_blocks(inode, nrblocks); > } > > It computes "ret" but doesn't actually use it, and then > ext4_chunk_trans_blocks() does nothing but call ext4_meta_trans_blocks(), > which is doing the bulk of the calculation. > > It looks like this was committed by Mingming in "ext4: journal credits > reservation fixes for extent file writepage" > ee12b630687d510f6f4b6d4acdc4e267fd4adeda > > Ideally we would use the "path" information to generate an accurate block > count for this extent. Can we just return "ret" in this case? > I guess so a return ret is missing. If we have space in the leaf tree then i guess we can safely add the extent information to the leaf and update only bitmap and group descriptors. -aneesh -- To unsubscribe from this list: send the line "unsubscribe linux-ext4" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html