From: Andreas Dilger <adilger@sun.com>
Subject: ext4_ext_calc_credits_for_single_extent() bug?
Date: Tue, 30 Jun 2009 00:41:33 +0200
Message-ID: <20090629224133.GF19362@webber.adilger.int>
Mime-Version: 1.0
Content-Type: text/plain; CHARSET=US-ASCII
Content-Transfer-Encoding: 7BIT
Cc: linux-ext4@vger.kernel.org
To: Aneesh Kumar <aneesh.kumar@linux.vnet.ibm.com>,
	Mingming Cao <cmm@us.ibm.com>
Content-disposition: inline
Sender: linux-ext4-owner@vger.kernel.org

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?

Cheers, Andreas
--
Andreas Dilger
Sr. Staff Engineer, Lustre Group
Sun Microsystems of Canada, Inc.