linux-ext4 - Re: ext4_ext_calc_credits_for_single

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20090701183854.GC31235@skywalker>
Date:	Thu, 2 Jul 2009 00:08:54 +0530
From:	"Aneesh Kumar K.V" <aneesh.kumar@...ux.vnet.ibm.com>
To:	Andreas Dilger <adilger@....com>
Cc:	Mingming Cao <cmm@...ibm.com>, linux-ext4@...r.kernel.org
Subject: Re: ext4_ext_calc_credits_for_single_extent() bug?

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@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html