Re: [Ext2-devel] [PATCH 1/2] ext2/3: Support 2^32-1 blocks(Kernel)

From: Takashi Sato
Date: Thu Mar 16 2006 - 07:08:38 EST


Hi,

You changed most of the affected variables from "int" to "unsigned int",
that seems allow block number to address 2^32. It probably a good thing
to consider change the variables to sector_t type, so when the time we
want to support for 64 bit block number, we don't have to re-do the
similar work again. Laurent did very similar work on this before.

sector_t is 8bytes on normal configuration and there are many
variables for blocks on ext2/3. I thought extending variables may
influence on performance, so I didn't change.

Besides these limitations, I think there is one more to limit ext3
filesystem size to 8TB

- The superblock format currently stores the number of block groups as a
16-bit integer, and because (on a 4 KB blocksize filesystem) the maximum
number of blocks in a block group is 32,768 , a combination of these
constraints limits the maximum size of the filesystem to 8 TB

Is it s_block_group_nr in ext3_super_block?
mke2fs sets 65535 to the field if the number of block groups is greater
than 65535. Current kernel ignores the field and re-calculate from
other fields. findsuper command is the only user of it and it simply prints
the value. So, it does not limit the maximum size of the filesystem to 8 TB.
I confirmed that mke2fs with my change could make the filesysytem
which has more than 65536 groups and it could be mounted.

I noticed that the first patch set combines changes to ext2 filesystem
and changes to ext3 filesystem. It would be nice to split the changes to
two different filesystems.

Ok, I'll split it later.

But that doesn't fix all th problem. We still have places in ext3 block
reservation code that use int for system-wide block numbers. For e.g.,
alloc_new_reservation(), group_first_block, group_end_block, start_block
are all filesystem wide block numbers, they need to be changed. I will
check the code more closely tomorrow to see if the changes will break
any assumptions.

Thank you, I missed it. I'm looking forward to seeing your report.

Also, I noticed that in your first patch, you changed a few variables
for logical block number from "long" to "unsigned int". Just want to
point out that's a seperate issue- that's for enlarge the file size, not
for expand the max filesystem size.

Ok, I'll remove them when I update the patch next time.
They are left because I'm considering enlarging the file size max too...

-static int ext3_alloc_block (handle_t *handle,
- struct inode * inode, unsigned long goal, int *err)
+static unsigned int ext3_alloc_block (handle_t *handle,
+ struct inode * inode, unsigned int goal, int *err)
{

I did some changes in the same code to support ext3 multiple block
allocation. Those patches removed this function ext3_alloc_block(). The
patches are sitting in mm tree now.

BTW, why we change from unsigned long back to unsigned int here?

Because ext3_alloc_branch calls ext3_alloc_block with int type for the
block number and ext3_alloc_blocks returns int type.

struct ext3_block_alloc_info *block_i = EXT3_I(inode)->i_block_alloc_info;
@@ -505,21 +505,21 @@ static unsigned long ext3_find_goal(stru
static int ext3_alloc_branch(handle_t *handle, struct inode *inode,
int num,
unsigned long goal,
- int *offsets,
+ unsigned int *offsets,
Indirect *branch)

offsets[] array here store the index position within a indirect block,
where the physical block is stored. The indirect block takes a 4k block,
holds up to 1K entry of physical block numbers, so int type for the
index is good enough.

Ok, I'll update them too.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/