[RFC][8/21]ext3 modify variables to exceed 2G

[RFC][8/21]ext3 modify variables to exceed 2G

[sho]

Summary of this patch:
  [8/21]  change the type of variables manipulating a block or an
          inode(ext3)
          - Change the type of 4byte variables manipulating a block or
            an inode from signed to unsigned.

          - Where an overflow occurs in process of operation, casting
            it to long long.

Signed-off-by: Takashi Sato sho@tnes.nec.co.jp
---
diff -upNr -X linux-2.6.17-rc1/Documentation/dontdiff linux-2.6.17-rc1/fs/ext3/balloc.c linux-2.6.17-rc1.tmp/fs/ext3/balloc.c
--- linux-2.6.17-rc1/fs/ext3/balloc.c	2006-04-13 12:12:08.000000000 +0900
+++ linux-2.6.17-rc1.tmp/fs/ext3/balloc.c	2006-04-13 12:15:19.000000000 +0900
@@ -1135,7 +1135,7 @@ ext3_try_to_allocate_with_rsv(struct sup
 			try_to_extend_reservation(my_rsv, sb,
 					*count-my_rsv->rsv_end + goal - 1);
 
-		if ((my_rsv->rsv_start >= group_first_block + EXT3_BLOCKS_PER_GROUP(sb))
+		if ((my_rsv->rsv_start >= (unsigned long long)group_first_block + EXT3_BLOCKS_PER_GROUP(sb))
 		    || (my_rsv->rsv_end < group_first_block))
 			BUG();
 		ret = ext3_try_to_allocate(sb, handle, group, bitmap_bh, goal,
diff -upNr -X linux-2.6.17-rc1/Documentation/dontdiff linux-2.6.17-rc1/fs/ext3/ialloc.c linux-2.6.17-rc1.tmp/fs/ext3/ialloc.c
--- linux-2.6.17-rc1/fs/ext3/ialloc.c	2006-04-13 12:12:03.000000000 +0900
+++ linux-2.6.17-rc1.tmp/fs/ext3/ialloc.c	2006-04-13 12:15:19.000000000 +0900
@@ -202,7 +202,8 @@ error_return:
 static int find_group_dir(struct super_block *sb, struct inode *parent)
 {
 	int ngroups = EXT3_SB(sb)->s_groups_count;
-	int freei, avefreei;
+	unsigned long freei;
+	int avefreei;
 	struct ext3_group_desc *desc, *best_desc = NULL;
 	struct buffer_head *bh;
 	int group, best_group = -1;
@@ -261,9 +262,9 @@ static int find_group_orlov(struct super
 	struct ext3_super_block *es = sbi->s_es;
 	int ngroups = sbi->s_groups_count;
 	int inodes_per_group = EXT3_INODES_PER_GROUP(sb);
-	int freei, avefreei;
-	unsigned long freeb, avefreeb;
-	int blocks_per_dir, ndirs;
+	int avefreei;
+	unsigned long freeb, avefreeb, freei, ndirs;
+	int blocks_per_dir;
 	int max_debt, max_dirs, min_blocks, min_inodes;
 	int group = -1, i;
 	struct ext3_group_desc *desc;
diff -upNr -X linux-2.6.17-rc1/Documentation/dontdiff linux-2.6.17-rc1/fs/ext3/inode.c linux-2.6.17-rc1.tmp/fs/ext3/inode.c
--- linux-2.6.17-rc1/fs/ext3/inode.c	2006-04-13 12:12:08.000000000 +0900
+++ linux-2.6.17-rc1.tmp/fs/ext3/inode.c	2006-04-13 12:15:19.000000000 +0900
@@ -284,7 +284,7 @@ static int verify_chain(Indirect *from, 
  */
 
 static int ext3_block_to_path(struct inode *inode,
-			long i_block, int offsets[4], int *boundary)
+			unsigned long i_block, int offsets[4], int *boundary)
 {
 	int ptrs = EXT3_ADDR_PER_BLOCK(inode->i_sb);
 	int ptrs_bits = EXT3_ADDR_PER_BLOCK_BITS(inode->i_sb);
@@ -448,7 +448,7 @@ static unsigned long ext3_find_near(stru
  *	stores it in *@goal and returns zero.
  */
 
-static unsigned long ext3_find_goal(struct inode *inode, long block,
+static unsigned long ext3_find_goal(struct inode *inode, unsigned long block,
 		Indirect chain[4], Indirect *partial)
 {
 	struct ext3_block_alloc_info *block_i;
@@ -515,7 +515,7 @@ static int ext3_blks_to_allocate(Indirec
  *	@blks:	on return it will store the total number of allocated
  *		direct blocks
  */
-static int ext3_alloc_blocks(handle_t *handle, struct inode *inode,
+static unsigned int ext3_alloc_blocks(handle_t *handle, struct inode *inode,
 			unsigned long goal, int indirect_blks, int blks,
 			unsigned long long new_blocks[4], int *err)
 {
@@ -599,7 +599,7 @@ static int ext3_alloc_branch(handle_t *h
 	int i, n = 0;
 	int err = 0;
 	struct buffer_head *bh;
-	int num;
+	unsigned int num;
 	unsigned long long new_blocks[4];
 	unsigned long long current_block;
 
@@ -683,7 +683,7 @@ failed:
  * chain to new block and return 0.
  */
 static int ext3_splice_branch(handle_t *handle, struct inode *inode,
-			long block, Indirect *where, int num, int blks)
+			unsigned long block, Indirect *where, int num, int blks)
 {
 	int i;
 	int err = 0;
@@ -995,7 +995,7 @@ get_block:
  * `handle' can be NULL if create is zero
  */
 struct buffer_head *ext3_getblk(handle_t *handle, struct inode *inode,
-				long block, int create, int *errp)
+				unsigned long block, int create, int *errp)
 {
 	struct buffer_head dummy;
 	int fatal = 0, err;
@@ -1059,7 +1059,7 @@ err:
 }
 
 struct buffer_head *ext3_bread(handle_t *handle, struct inode *inode,
-			       int block, int create, int *err)
+			       unsigned int block, int create, int *err)
 {
 	struct buffer_head * bh;
 
@@ -2231,7 +2231,7 @@ void ext3_truncate(struct inode *inode)
 	Indirect *partial;
 	__le32 nr = 0;
 	int n;
-	long last_block;
+	unsigned long last_block;
 	unsigned blocksize = inode->i_sb->s_blocksize;
 	struct page *page;
 
diff -upNr -X linux-2.6.17-rc1/Documentation/dontdiff linux-2.6.17-rc1/fs/ext3/namei.c linux-2.6.17-rc1.tmp/fs/ext3/namei.c
--- linux-2.6.17-rc1/fs/ext3/namei.c	2006-04-13 12:12:08.000000000 +0900
+++ linux-2.6.17-rc1.tmp/fs/ext3/namei.c	2006-04-13 12:15:19.000000000 +0900
@@ -816,7 +816,8 @@ static struct buffer_head * ext3_find_en
 	int ra_ptr = 0;		/* Current index into readahead
 				   buffer */
 	int num = 0;
-	int nblocks, i, err;
+	unsigned int nblocks;
+	int i, err;
 	struct inode *dir = dentry->d_parent->d_inode;
 	int namelen;
 	const u8 *name;
diff -upNr -X linux-2.6.17-rc1/Documentation/dontdiff linux-2.6.17-rc1/fs/ext3/resize.c linux-2.6.17-rc1.tmp/fs/ext3/resize.c
--- linux-2.6.17-rc1/fs/ext3/resize.c	2006-04-13 12:12:08.000000000 +0900
+++ linux-2.6.17-rc1.tmp/fs/ext3/resize.c	2006-04-13 12:15:19.000000000 +0900
@@ -37,7 +37,7 @@ static int verify_group_input(struct sup
 		 le16_to_cpu(es->s_reserved_gdt_blocks)) : 0;
 	unsigned metaend = start + overhead;
 	struct buffer_head *bh = NULL;
-	int free_blocks_count;
+	long long free_blocks_count;
 	int err = -EINVAL;
 
 	input->free_blocks_count = free_blocks_count =
@@ -138,9 +138,9 @@ static struct buffer_head *bclean(handle
  * need to use it within a single byte (to ensure we get endianness right).
  * We can use memset for the rest of the bitmap as there are no other users.
  */
-static void mark_bitmap_end(int start_bit, int end_bit, char *bitmap)
+static void mark_bitmap_end(unsigned int start_bit, unsigned int end_bit, char *bitmap)
 {
-	int i;
+	unsigned int i;
 
 	if (start_bit >= end_bit)
 		return;
@@ -619,7 +619,7 @@ exit_free:
  * at this time.  The resize which changed s_groups_count will backup again.
  */
 static void update_backups(struct super_block *sb,
-			   int blk_off, char *data, int size)
+			   unsigned int blk_off, char *data, int size)
 {
 	struct ext3_sb_info *sbi = EXT3_SB(sb);
 	const unsigned long last = sbi->s_groups_count;
@@ -726,6 +726,18 @@ int ext3_group_add(struct super_block *s
 		return -EPERM;
 	}
 
+	if (((unsigned long long)es->s_blocks_count + input->blocks_count) > ~0U) {
+		ext3_warning(sb, __FUNCTION__,
+			     "blocks_count must be less than 4G!");  
+		return -EPERM;
+	}
+
+	if (((unsigned long long)es->s_inodes_count + EXT3_INODES_PER_GROUP(sb)) > ~0U) {
+		ext3_warning(sb, __FUNCTION__,
+			     "inodes_count must be less than 4G!");
+		return -EPERM;
+	}
+
 	if (reserved_gdb || gdb_off == 0) {
 		if (!EXT3_HAS_COMPAT_FEATURE(sb,
 					     EXT3_FEATURE_COMPAT_RESIZE_INODE)){
@@ -943,6 +955,12 @@ int ext3_group_extend(struct super_block
 
 	add = EXT3_BLOCKS_PER_GROUP(sb) - last;
 
+	if (((unsigned long long)o_blocks_count + add) > ~0U) {
+		ext3_warning(sb, __FUNCTION__,
+			     "blocks_count must be less than 4G!");
+		return -EPERM;
+	}
+
 	if (o_blocks_count + add > n_blocks_count)
 		add = n_blocks_count - o_blocks_count;
 
diff -upNr -X linux-2.6.17-rc1/Documentation/dontdiff linux-2.6.17-rc1/fs/ext3/super.c linux-2.6.17-rc1.tmp/fs/ext3/super.c
--- linux-2.6.17-rc1/fs/ext3/super.c	2006-04-13 12:12:08.000000000 +0900
+++ linux-2.6.17-rc1.tmp/fs/ext3/super.c	2006-04-13 12:15:19.000000000 +0900
@@ -38,6 +38,7 @@
 #include <linux/seq_file.h>
 
 #include <asm/uaccess.h>
+#include <asm/div64.h>
 
 #include "xattr.h"
 #include "acl.h"
@@ -1141,7 +1142,7 @@ static int ext3_check_descriptors (struc
 					sbi->s_group_desc[desc_block++]->b_data;
 		if (le32_to_cpu(gdp->bg_block_bitmap) < block ||
 		    le32_to_cpu(gdp->bg_block_bitmap) >=
-				block + EXT3_BLOCKS_PER_GROUP(sb))
+				(unsigned long long)block + EXT3_BLOCKS_PER_GROUP(sb))
 		{
 			ext3_error (sb, "ext3_check_descriptors",
 				    "Block bitmap for group %d"
@@ -1152,7 +1153,7 @@ static int ext3_check_descriptors (struc
 		}
 		if (le32_to_cpu(gdp->bg_inode_bitmap) < block ||
 		    le32_to_cpu(gdp->bg_inode_bitmap) >=
-				block + EXT3_BLOCKS_PER_GROUP(sb))
+				(unsigned long long)block + EXT3_BLOCKS_PER_GROUP(sb))
 		{
 			ext3_error (sb, "ext3_check_descriptors",
 				    "Inode bitmap for group %d"
@@ -1163,7 +1164,7 @@ static int ext3_check_descriptors (struc
 		}
 		if (le32_to_cpu(gdp->bg_inode_table) < block ||
 		    le32_to_cpu(gdp->bg_inode_table) + sbi->s_itb_per_group >=
-		    block + EXT3_BLOCKS_PER_GROUP(sb))
+		    (unsigned long long)block + EXT3_BLOCKS_PER_GROUP(sb))
 		{
 			ext3_error (sb, "ext3_check_descriptors",
 				    "Inode table for group %d"
@@ -1354,6 +1355,7 @@ static int ext3_fill_super (struct super
 	int i;
 	int needs_recovery;
 	__le32 features;
+	unsigned long long tmp_blocks;
 
 	sbi = kmalloc(sizeof(*sbi), GFP_KERNEL);
 	if (!sbi)
@@ -1566,10 +1568,11 @@ static int ext3_fill_super (struct super
 
 	if (EXT3_BLOCKS_PER_GROUP(sb) == 0)
 		goto cantfind_ext3;
-	sbi->s_groups_count = (le32_to_cpu(es->s_blocks_count) -
-			       le32_to_cpu(es->s_first_data_block) +
-			       EXT3_BLOCKS_PER_GROUP(sb) - 1) /
-			      EXT3_BLOCKS_PER_GROUP(sb);
+	tmp_blocks = (le32_to_cpu(es->s_blocks_count) -
+		      le32_to_cpu(es->s_first_data_block) +
+		      (unsigned long long)EXT3_BLOCKS_PER_GROUP(sb) - 1);
+	do_div(tmp_blocks, EXT3_BLOCKS_PER_GROUP(sb));
+	sbi->s_groups_count = tmp_blocks;
 	db_count = (sbi->s_groups_count + EXT3_DESC_PER_BLOCK(sb) - 1) /
 		   EXT3_DESC_PER_BLOCK(sb);
 	sbi->s_group_desc = kmalloc(db_count * sizeof (struct buffer_head *),
diff -upNr -X linux-2.6.17-rc1/Documentation/dontdiff linux-2.6.17-rc1/include/linux/ext3_fs.h linux-2.6.17-rc1.tmp/include/linux/ext3_fs.h
--- linux-2.6.17-rc1/include/linux/ext3_fs.h	2006-04-13 12:11:55.000000000 +0900
+++ linux-2.6.17-rc1.tmp/include/linux/ext3_fs.h	2006-04-13 12:16:56.000000000 +0900
@@ -776,8 +776,8 @@ extern unsigned long ext3_count_free (st
 
 /* inode.c */
 int ext3_forget(handle_t *, int, struct inode *, struct buffer_head *, unsigned long);
-struct buffer_head * ext3_getblk (handle_t *, struct inode *, long, int, int *);
-struct buffer_head * ext3_bread (handle_t *, struct inode *, int, int, int *);
+struct buffer_head * ext3_getblk (handle_t *, struct inode *, unsigned long, int, int *);
+struct buffer_head * ext3_bread (handle_t *, struct inode *, unsigned int, int, int *);
 int ext3_get_blocks_handle(handle_t *handle, struct inode *inode,
 	sector_t iblock, unsigned long maxblocks, struct buffer_head *bh_result,
 	int create, int extend_disksize);

Re: [Ext2-devel] [RFC][8/21]ext3 modify variables to exceed 2G

[Andreas Dilger]

On Apr 13, 2006  16:06 +0900, sho@tnes.nec.co.jp wrote:
> Summary of this patch:
>   [8/21]  change the type of variables manipulating a block or an
>           inode(ext3)
>           - Change the type of 4byte variables manipulating a block or
>             an inode from signed to unsigned.

Takashi-san, please, it would make the code much more maintainable if the
changes made here would use new types for filesystem-wide block offsets
and for file-relative block offsets, as was previously discussed, instead
of just changing some variables to be unsigned long.  Like:

typedef unsigned long ext3_fsblk_t;	# block offset in the filesystem
typedef unsigned long ext3_fscnt_t;	# block count in the filesystem
typedef unsigned long ext3_fileblk_t;	# block offset in a file

I believe we should already be using le32 for all on-disk blocks (e.g.
indirect blocks, inodes, etc), but this would be good to verify.

This way, when we get to the stage where we want to increase the blocks
to be 64-bit (as Laurent has wanted to do) we don't need to go through
and re-patch all of the same files to change "unsigned long" to
"unsigned long long".  It also makes the code easier to read, since it
will be clear what kind of variable is being used.

Cheers, Andreas
--
Andreas Dilger
Principal Software Engineer
Cluster File Systems, Inc.

Re: [Ext2-devel] [RFC][8/21]ext3 modify variables to exceed 2G

[Takashi Sato]

Thank you for your comment, Andreas.

> Takashi-san, please, it would make the code much more maintainable if the
> changes made here would use new types for filesystem-wide block offsets
> and for file-relative block offsets, as was previously discussed, instead
> of just changing some variables to be unsigned long.  Like:
> 
> typedef unsigned long ext3_fsblk_t; # block offset in the filesystem
> typedef unsigned long ext3_fscnt_t; # block count in the filesystem
> typedef unsigned long ext3_fileblk_t; # block offset in a file

I agree that, but it will need a lots of work...
Mingming, you got same comment from Andreas in "Extend ext3
filesystem limit from 8TB to 16TB", did you do something about
this?

Cheers, sho

Re: [Ext2-devel] [RFC][8/21]ext3 modify variables to exceed 2G

[Mingming Cao]

On Fri, 2006-04-14 at 18:23 +0900, Takashi Sato wrote:
> Thank you for your comment, Andreas.
> 
> > Takashi-san, please, it would make the code much more maintainable if the
> > changes made here would use new types for filesystem-wide block offsets
> > and for file-relative block offsets, as was previously discussed, instead
> > of just changing some variables to be unsigned long.  Like:
> > 
> > typedef unsigned long ext3_fsblk_t; # block offset in the filesystem
> > typedef unsigned long ext3_fscnt_t; # block count in the filesystem
> > typedef unsigned long ext3_fileblk_t; # block offset in a file
> 
> I agree that, but it will need a lots of work...
> Mingming, you got same comment from Andreas in "Extend ext3
> filesystem limit from 8TB to 16TB", did you do something about
> this?

No, I haven't get chance to do it yet. Please feel free to take it.:)

Laurent did some ground work for the filesystem wide physical block
number in his 64 bit ext3 patches. He changed physical block number from
"unsigned long" to "sector_t", that's probably a good start.

BTW, I think we should have one more:

typedef long ext3_grpblk_t; # block offset in the block group


Mingming