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] [thread-next>] [day] [month] [year] [list]
Message-ID: <AANLkTik962xWpBCaFgfu9Xm0YkPJ8+RwTJH9Oj48Wh2r@mail.gmail.com>
Date:	Sun, 20 Mar 2011 12:26:57 +0200
From:	Amir Goldstein <amir73il@...il.com>
To:	"Theodore Ts'o" <tytso@....edu>
Cc:	linux-ext4@...r.kernel.org
Subject: Re: [PATCH, RFC 03/12] ext4: Convert instances of EXT4_BLOCKS_PER_GROUP
 to EXT4_CLUSTERS_PER_GROUP

On Sat, Mar 19, 2011 at 11:28 PM, Theodore Ts'o <tytso@....edu> wrote:
> Change the places in fs/ext4/mballoc.c where EXT4_BLOCKS_PER_GROUP are
> used to indicate the number of bits in a block bitmap (which is really
> a cluster allocation bitmap in bigalloc file systems).  There are
> still some places in the ext4 codebase where usage of
> EXT4_BLOCKS_PER_GROUP needs to be audited/fixed, in code paths that
> aren't used given the initial restricted assumptions for bigalloc.
> These will need to be fixed before we can relax those restrictions.
>
> Signed-off-by: "Theodore Ts'o" <tytso@....edu>
> ---
>  fs/ext4/mballoc.c |   34 +++++++++++++++++-----------------
>  1 files changed, 17 insertions(+), 17 deletions(-)
>
> diff --git a/fs/ext4/mballoc.c b/fs/ext4/mballoc.c
> index 2f6f0dd..02f099f 100644
> --- a/fs/ext4/mballoc.c
> +++ b/fs/ext4/mballoc.c
> @@ -651,7 +651,7 @@ static void ext4_mb_mark_free_simple(struct super_block *sb,
>        ext4_grpblk_t chunk;
>        unsigned short border;
>
> -       BUG_ON(len > EXT4_BLOCKS_PER_GROUP(sb));
> +       BUG_ON(len > EXT4_CLUSTERS_PER_GROUP(sb));
>
>        border = 2 << sb->s_blocksize_bits;
>
> @@ -703,7 +703,7 @@ void ext4_mb_generate_buddy(struct super_block *sb,
>                                void *buddy, void *bitmap, ext4_group_t group)
>  {
>        struct ext4_group_info *grp = ext4_get_group_info(sb, group);
> -       ext4_grpblk_t max = EXT4_BLOCKS_PER_GROUP(sb);
> +       ext4_grpblk_t max = EXT4_CLUSTERS_PER_GROUP(sb);
>        ext4_grpblk_t i = 0;
>        ext4_grpblk_t first;
>        ext4_grpblk_t len;
> @@ -1680,8 +1680,8 @@ static void ext4_mb_measure_extent(struct ext4_allocation_context *ac,
>        struct ext4_free_extent *gex = &ac->ac_g_ex;
>
>        BUG_ON(ex->fe_len <= 0);
> -       BUG_ON(ex->fe_len > EXT4_BLOCKS_PER_GROUP(ac->ac_sb));
> -       BUG_ON(ex->fe_start >= EXT4_BLOCKS_PER_GROUP(ac->ac_sb));
> +       BUG_ON(ex->fe_len > EXT4_CLUSTERS_PER_GROUP(ac->ac_sb));
> +       BUG_ON(ex->fe_start >= EXT4_CLUSTERS_PER_GROUP(ac->ac_sb));
>        BUG_ON(ac->ac_status != AC_STATUS_CONTINUE);
>
>        ac->ac_found++;
> @@ -1879,8 +1879,8 @@ void ext4_mb_complex_scan_group(struct ext4_allocation_context *ac,
>
>        while (free && ac->ac_status == AC_STATUS_CONTINUE) {
>                i = mb_find_next_zero_bit(bitmap,
> -                                               EXT4_BLOCKS_PER_GROUP(sb), i);
> -               if (i >= EXT4_BLOCKS_PER_GROUP(sb)) {
> +                                               EXT4_CLUSTERS_PER_GROUP(sb), i);
> +               if (i >= EXT4_CLUSTERS_PER_GROUP(sb)) {
>                        /*
>                         * IF we have corrupt bitmap, we won't find any
>                         * free blocks even though group info says we
> @@ -1943,7 +1943,7 @@ void ext4_mb_scan_aligned(struct ext4_allocation_context *ac,
>        do_div(a, sbi->s_stripe);
>        i = (a * sbi->s_stripe) - first_group_block;
>
> -       while (i < EXT4_BLOCKS_PER_GROUP(sb)) {
> +       while (i < EXT4_CLUSTERS_PER_GROUP(sb)) {
>                if (!mb_test_bit(i, bitmap)) {
>                        max = mb_find_extent(e4b, 0, i, sbi->s_stripe, &ex);
>                        if (max >= sbi->s_stripe) {
> @@ -3071,7 +3071,7 @@ ext4_mb_normalize_request(struct ext4_allocation_context *ac,
>        }
>        BUG_ON(start + size <= ac->ac_o_ex.fe_logical &&
>                        start > ac->ac_o_ex.fe_logical);
> -       BUG_ON(size <= 0 || size > EXT4_BLOCKS_PER_GROUP(ac->ac_sb));
> +       BUG_ON(size <= 0 || size > EXT4_CLUSTERS_PER_GROUP(ac->ac_sb));
>
>        /* now prepare goal request */
>
> @@ -3724,7 +3724,7 @@ ext4_mb_discard_group_preallocations(struct super_block *sb,
>        }
>
>        if (needed == 0)
> -               needed = EXT4_BLOCKS_PER_GROUP(sb) + 1;
> +               needed = EXT4_CLUSTERS_PER_GROUP(sb) + 1;
>
>        INIT_LIST_HEAD(&list);
>  repeat:
> @@ -4039,8 +4039,8 @@ ext4_mb_initialize_context(struct ext4_allocation_context *ac,
>        len = ar->len;
>
>        /* just a dirty hack to filter too big requests  */
> -       if (len >= EXT4_BLOCKS_PER_GROUP(sb) - 10)
> -               len = EXT4_BLOCKS_PER_GROUP(sb) - 10;
> +       if (len >= EXT4_CLUSTERS_PER_GROUP(sb) - 10)
> +               len = EXT4_CLUSTERS_PER_GROUP(sb) - 10;
>
>        /* start searching from the goal */
>        goal = ar->goal;
> @@ -4579,8 +4579,8 @@ do_more:
>         * Check to see if we are freeing blocks across a group
>         * boundary.
>         */
> -       if (bit + count > EXT4_BLOCKS_PER_GROUP(sb)) {
> -               overflow = bit + count - EXT4_BLOCKS_PER_GROUP(sb);
> +       if (bit + count > EXT4_CLUSTERS_PER_GROUP(sb)) {
> +               overflow = bit + count - EXT4_CLUSTERS_PER_GROUP(sb);



On Sat, Mar 19, 2011 at 11:28 PM, Theodore Ts'o <tytso@....edu> wrote:
> Change the places in fs/ext4/mballoc.c where EXT4_BLOCKS_PER_GROUP are
> used to indicate the number of bits in a block bitmap (which is really
> a cluster allocation bitmap in bigalloc file systems).  There are
> still some places in the ext4 codebase where usage of
> EXT4_BLOCKS_PER_GROUP needs to be audited/fixed, in code paths that
> aren't used given the initial restricted assumptions for bigalloc.
> These will need to be fixed before we can relax those restrictions.
>
> Signed-off-by: "Theodore Ts'o" <tytso@....edu>
> ---
>  fs/ext4/mballoc.c |   34 +++++++++++++++++-----------------
>  1 files changed, 17 insertions(+), 17 deletions(-)
>
> diff --git a/fs/ext4/mballoc.c b/fs/ext4/mballoc.c
> index 2f6f0dd..02f099f 100644
> --- a/fs/ext4/mballoc.c
> +++ b/fs/ext4/mballoc.c
> @@ -651,7 +651,7 @@ static void ext4_mb_mark_free_simple(struct super_block *sb,
>        ext4_grpblk_t chunk;
>        unsigned short border;
>
> -       BUG_ON(len > EXT4_BLOCKS_PER_GROUP(sb));
> +       BUG_ON(len > EXT4_CLUSTERS_PER_GROUP(sb));
>
>        border = 2 << sb->s_blocksize_bits;
>
> @@ -703,7 +703,7 @@ void ext4_mb_generate_buddy(struct super_block *sb,
>                                void *buddy, void *bitmap, ext4_group_t group)
>  {
>        struct ext4_group_info *grp = ext4_get_group_info(sb, group);
> -       ext4_grpblk_t max = EXT4_BLOCKS_PER_GROUP(sb);
> +       ext4_grpblk_t max = EXT4_CLUSTERS_PER_GROUP(sb);
>        ext4_grpblk_t i = 0;
>        ext4_grpblk_t first;
>        ext4_grpblk_t len;
> @@ -1680,8 +1680,8 @@ static void ext4_mb_measure_extent(struct ext4_allocation_context *ac,
>        struct ext4_free_extent *gex = &ac->ac_g_ex;
>
>        BUG_ON(ex->fe_len <= 0);
> -       BUG_ON(ex->fe_len > EXT4_BLOCKS_PER_GROUP(ac->ac_sb));
> -       BUG_ON(ex->fe_start >= EXT4_BLOCKS_PER_GROUP(ac->ac_sb));
> +       BUG_ON(ex->fe_len > EXT4_CLUSTERS_PER_GROUP(ac->ac_sb));
> +       BUG_ON(ex->fe_start >= EXT4_CLUSTERS_PER_GROUP(ac->ac_sb));
>        BUG_ON(ac->ac_status != AC_STATUS_CONTINUE);
>
>        ac->ac_found++;
> @@ -1879,8 +1879,8 @@ void ext4_mb_complex_scan_group(struct ext4_allocation_context *ac,
>
>        while (free && ac->ac_status == AC_STATUS_CONTINUE) {
>                i = mb_find_next_zero_bit(bitmap,
> -                                               EXT4_BLOCKS_PER_GROUP(sb), i);
> -               if (i >= EXT4_BLOCKS_PER_GROUP(sb)) {
> +                                               EXT4_CLUSTERS_PER_GROUP(sb), i);
> +               if (i >= EXT4_CLUSTERS_PER_GROUP(sb)) {
>                        /*
>                         * IF we have corrupt bitmap, we won't find any
>                         * free blocks even though group info says we
> @@ -1943,7 +1943,7 @@ void ext4_mb_scan_aligned(struct ext4_allocation_context *ac,
>        do_div(a, sbi->s_stripe);
>        i = (a * sbi->s_stripe) - first_group_block;
>
> -       while (i < EXT4_BLOCKS_PER_GROUP(sb)) {
> +       while (i < EXT4_CLUSTERS_PER_GROUP(sb)) {
>                if (!mb_test_bit(i, bitmap)) {
>                        max = mb_find_extent(e4b, 0, i, sbi->s_stripe, &ex);
>                        if (max >= sbi->s_stripe) {
> @@ -3071,7 +3071,7 @@ ext4_mb_normalize_request(struct ext4_allocation_context *ac,
>        }
>        BUG_ON(start + size <= ac->ac_o_ex.fe_logical &&
>                        start > ac->ac_o_ex.fe_logical);
> -       BUG_ON(size <= 0 || size > EXT4_BLOCKS_PER_GROUP(ac->ac_sb));
> +       BUG_ON(size <= 0 || size > EXT4_CLUSTERS_PER_GROUP(ac->ac_sb));
>
>        /* now prepare goal request */
>
> @@ -3724,7 +3724,7 @@ ext4_mb_discard_group_preallocations(struct super_block *sb,
>        }
>
>        if (needed == 0)
> -               needed = EXT4_BLOCKS_PER_GROUP(sb) + 1;
> +               needed = EXT4_CLUSTERS_PER_GROUP(sb) + 1;
>
>        INIT_LIST_HEAD(&list);
>  repeat:
> @@ -4039,8 +4039,8 @@ ext4_mb_initialize_context(struct ext4_allocation_context *ac,
>        len = ar->len;
>
>        /* just a dirty hack to filter too big requests  */
> -       if (len >= EXT4_BLOCKS_PER_GROUP(sb) - 10)
> -               len = EXT4_BLOCKS_PER_GROUP(sb) - 10;
> +       if (len >= EXT4_CLUSTERS_PER_GROUP(sb) - 10)
> +               len = EXT4_CLUSTERS_PER_GROUP(sb) - 10;
>
>        /* start searching from the goal */
>        goal = ar->goal;
> @@ -4579,8 +4579,8 @@ do_more:
>         * Check to see if we are freeing blocks across a group
>         * boundary.
>         */
> -       if (bit + count > EXT4_BLOCKS_PER_GROUP(sb)) {
> -               overflow = bit + count - EXT4_BLOCKS_PER_GROUP(sb);
> +       if (bit + count > EXT4_CLUSTERS_PER_GROUP(sb)) {
> +               overflow = bit + count - EXT4_CLUSTERS_PER_GROUP(sb);

If I am not mistaken, while 'bit' was already converted to cluster units,
'count' is still in block units.

I think ext4_free_blocks() need to do 2 things:
1. convert 'count' to clusters (after issuing journal_forget())
2. round 'bit' up (and round 'count' down) if start block is not
on cluster boundary, so truncate/punch hole, will not free a
cluster when it's 'base' block is still allocated.


>                count -= overflow;
>        }
>        bitmap_bh = ext4_read_block_bitmap(sb, block_group);
> @@ -4844,7 +4844,7 @@ int ext4_trim_fs(struct super_block *sb, struct fstrim_range *range)
>        minlen = range->minlen >> sb->s_blocksize_bits;
>        trimmed = 0;
>
> -       if (unlikely(minlen > EXT4_BLOCKS_PER_GROUP(sb)))
> +       if (unlikely(minlen > EXT4_CLUSTERS_PER_GROUP(sb)))
>                return -EINVAL;
>        if (start < first_data_blk) {
>                len -= first_data_blk - start;
> @@ -4857,7 +4857,7 @@ int ext4_trim_fs(struct super_block *sb, struct fstrim_range *range)
>        ext4_get_group_no_and_offset(sb, (ext4_fsblk_t) (start + len),
>                                     &last_group, &last_block);
>        last_group = (last_group > ngroups - 1) ? ngroups - 1 : last_group;
> -       last_block = EXT4_BLOCKS_PER_GROUP(sb);
> +       last_block = EXT4_CLUSTERS_PER_GROUP(sb);
>
>        if (first_group > last_group)
>                return -EINVAL;
> @@ -4870,8 +4870,8 @@ int ext4_trim_fs(struct super_block *sb, struct fstrim_range *range)
>                        break;
>                }
>
> -               if (len >= EXT4_BLOCKS_PER_GROUP(sb))
> -                       len -= (EXT4_BLOCKS_PER_GROUP(sb) - first_block);
> +               if (len >= EXT4_CLUSTERS_PER_GROUP(sb))
> +                       len -= (EXT4_CLUSTERS_PER_GROUP(sb) - first_block);
>                else
>                        last_block = first_block + len;
>
> --
> 1.7.3.1
>
> --
> 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
>
--
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

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ