[PATCH 1/7] ext4: Introduce le32_t and le16_t

[PATCH 1/7] ext4: Introduce le32_t and le16_t

[Aneesh Kumar K.V]

ext4 file system layout contain different split members
like bg_block_bitmap  and bg_block_bitmap_hi. Introduce
data type le32_t and le16_t to be used as the type of
these split members. This prevents these members frome
being accessed directly. Accesing them directly gives
a compiler warning. This helps in catching some BUGS
due to direct partial access of these split fields.

Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
---
 fs/ext4/super.c           |    8 ++++----
 include/linux/ext4_fs.h   |    4 ++--
 include/linux/ext4_fs_i.h |    4 ++++
 3 files changed, 10 insertions(+), 6 deletions(-)

diff --git a/fs/ext4/super.c b/fs/ext4/super.c
index d035653..c8f8e4d 100644
--- a/fs/ext4/super.c
+++ b/fs/ext4/super.c
@@ -70,9 +70,9 @@ static void ext4_write_super_lockfs(struct super_block *sb);
 ext4_fsblk_t ext4_block_bitmap(struct super_block *sb,
 			       struct ext4_group_desc *bg)
 {
-	return le32_to_cpu(bg->bg_block_bitmap) |
+	return le32_to_cpu(bg->bg_block_bitmap.value) |
 		(EXT4_DESC_SIZE(sb) >= EXT4_MIN_DESC_SIZE_64BIT ?
-		 (ext4_fsblk_t)le32_to_cpu(bg->bg_block_bitmap_hi) << 32 : 0);
+		 (ext4_fsblk_t)le32_to_cpu(bg->bg_block_bitmap_hi.value) << 32 : 0);
 }
 
 ext4_fsblk_t ext4_inode_bitmap(struct super_block *sb,
@@ -94,9 +94,9 @@ ext4_fsblk_t ext4_inode_table(struct super_block *sb,
 void ext4_block_bitmap_set(struct super_block *sb,
 			   struct ext4_group_desc *bg, ext4_fsblk_t blk)
 {
-	bg->bg_block_bitmap = cpu_to_le32((u32)blk);
+	bg->bg_block_bitmap.value = cpu_to_le32((u32)blk);
 	if (EXT4_DESC_SIZE(sb) >= EXT4_MIN_DESC_SIZE_64BIT)
-		bg->bg_block_bitmap_hi = cpu_to_le32(blk >> 32);
+		bg->bg_block_bitmap_hi.value = cpu_to_le32(blk >> 32);
 }
 
 void ext4_inode_bitmap_set(struct super_block *sb,
diff --git a/include/linux/ext4_fs.h b/include/linux/ext4_fs.h
index 046a6a7..4494f8e 100644
--- a/include/linux/ext4_fs.h
+++ b/include/linux/ext4_fs.h
@@ -150,7 +150,7 @@ struct ext4_allocation_request {
  */
 struct ext4_group_desc
 {
-	__le32	bg_block_bitmap;	/* Blocks bitmap block */
+	le32_t	bg_block_bitmap;	/* Blocks bitmap block */
 	__le32	bg_inode_bitmap;	/* Inodes bitmap block */
 	__le32	bg_inode_table;		/* Inodes table block */
 	__le16	bg_free_blocks_count;	/* Free blocks count */
@@ -160,7 +160,7 @@ struct ext4_group_desc
 	__u32	bg_reserved[2];		/* Likely block/inode bitmap checksum */
 	__le16  bg_itable_unused;	/* Unused inodes count */
 	__le16  bg_checksum;		/* crc16(sb_uuid+group+desc) */
-	__le32	bg_block_bitmap_hi;	/* Blocks bitmap block MSB */
+	le32_t	bg_block_bitmap_hi;	/* Blocks bitmap block MSB */
 	__le32	bg_inode_bitmap_hi;	/* Inodes bitmap block MSB */
 	__le32	bg_inode_table_hi;	/* Inodes table block MSB */
 };
diff --git a/include/linux/ext4_fs_i.h b/include/linux/ext4_fs_i.h
index 22ba80e..d7eb271 100644
--- a/include/linux/ext4_fs_i.h
+++ b/include/linux/ext4_fs_i.h
@@ -27,6 +27,10 @@ typedef int ext4_grpblk_t;
 /* data type for filesystem-wide blocks number */
 typedef unsigned long long ext4_fsblk_t;
 
+/* data type to carry split 64 and 48 bits */
+typedef struct { __le16 value; } le16_t;
+typedef struct { __le32 value; } le32_t;
+
 struct ext4_reserve_window {
 	ext4_fsblk_t	_rsv_start;	/* First byte reserved */
 	ext4_fsblk_t	_rsv_end;	/* Last byte reserved or 0 */
-- 
1.5.3.1.91.gd3392-dirty

[PATCH 2/7] ext4: Convert bg_inode_bitmap and bg_inode_table to new type

[Aneesh Kumar K.V]

Convert bg_inode_bitmap and bg_inode_table to le32_t
This helps in finding BUGs due to direct partial access of
these split 64 bit values

Also fix one direct partial access

Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
---
 fs/ext4/balloc.c        |    2 +-
 fs/ext4/super.c         |   18 +++++++++---------
 include/linux/ext4_fs.h |    8 ++++----
 3 files changed, 14 insertions(+), 14 deletions(-)

diff --git a/fs/ext4/balloc.c b/fs/ext4/balloc.c
index 2a37635..b717a37 100644
--- a/fs/ext4/balloc.c
+++ b/fs/ext4/balloc.c
@@ -103,7 +103,7 @@ unsigned ext4_init_block_bitmap(struct super_block *sb, struct buffer_head *bh,
 		/* Set bits for block and inode bitmaps, and inode table */
 		ext4_set_bit(ext4_block_bitmap(sb, gdp) - start, bh->b_data);
 		ext4_set_bit(ext4_inode_bitmap(sb, gdp) - start, bh->b_data);
-		for (bit = le32_to_cpu(gdp->bg_inode_table) - start,
+		for (bit = (ext4_inode_table(sb, gdp) - start),
 		     bit_max = bit + sbi->s_itb_per_group; bit < bit_max; bit++)
 			ext4_set_bit(bit, bh->b_data);
 	}
diff --git a/fs/ext4/super.c b/fs/ext4/super.c
index c8f8e4d..b9dda19 100644
--- a/fs/ext4/super.c
+++ b/fs/ext4/super.c
@@ -72,23 +72,23 @@ ext4_fsblk_t ext4_block_bitmap(struct super_block *sb,
 {
 	return le32_to_cpu(bg->bg_block_bitmap.value) |
 		(EXT4_DESC_SIZE(sb) >= EXT4_MIN_DESC_SIZE_64BIT ?
-		 (ext4_fsblk_t)le32_to_cpu(bg->bg_block_bitmap_hi.value) << 32 : 0);
+	 (ext4_fsblk_t)le32_to_cpu(bg->bg_block_bitmap_hi.value) << 32 : 0);
 }
 
 ext4_fsblk_t ext4_inode_bitmap(struct super_block *sb,
 			       struct ext4_group_desc *bg)
 {
-	return le32_to_cpu(bg->bg_inode_bitmap) |
+	return le32_to_cpu(bg->bg_inode_bitmap.value) |
 		(EXT4_DESC_SIZE(sb) >= EXT4_MIN_DESC_SIZE_64BIT ?
-		 (ext4_fsblk_t)le32_to_cpu(bg->bg_inode_bitmap_hi) << 32 : 0);
+	 (ext4_fsblk_t)le32_to_cpu(bg->bg_inode_bitmap_hi.value) << 32 : 0);
 }
 
 ext4_fsblk_t ext4_inode_table(struct super_block *sb,
 			      struct ext4_group_desc *bg)
 {
-	return le32_to_cpu(bg->bg_inode_table) |
+	return le32_to_cpu(bg->bg_inode_table.value) |
 		(EXT4_DESC_SIZE(sb) >= EXT4_MIN_DESC_SIZE_64BIT ?
-		 (ext4_fsblk_t)le32_to_cpu(bg->bg_inode_table_hi) << 32 : 0);
+	 (ext4_fsblk_t)le32_to_cpu(bg->bg_inode_table_hi.value) << 32 : 0);
 }
 
 void ext4_block_bitmap_set(struct super_block *sb,
@@ -102,17 +102,17 @@ void ext4_block_bitmap_set(struct super_block *sb,
 void ext4_inode_bitmap_set(struct super_block *sb,
 			   struct ext4_group_desc *bg, ext4_fsblk_t blk)
 {
-	bg->bg_inode_bitmap  = cpu_to_le32((u32)blk);
+	bg->bg_inode_bitmap.value  = cpu_to_le32((u32)blk);
 	if (EXT4_DESC_SIZE(sb) >= EXT4_MIN_DESC_SIZE_64BIT)
-		bg->bg_inode_bitmap_hi = cpu_to_le32(blk >> 32);
+		bg->bg_inode_bitmap_hi.value = cpu_to_le32(blk >> 32);
 }
 
 void ext4_inode_table_set(struct super_block *sb,
 			  struct ext4_group_desc *bg, ext4_fsblk_t blk)
 {
-	bg->bg_inode_table = cpu_to_le32((u32)blk);
+	bg->bg_inode_table.value = cpu_to_le32((u32)blk);
 	if (EXT4_DESC_SIZE(sb) >= EXT4_MIN_DESC_SIZE_64BIT)
-		bg->bg_inode_table_hi = cpu_to_le32(blk >> 32);
+		bg->bg_inode_table_hi.value = cpu_to_le32(blk >> 32);
 }
 
 /*
diff --git a/include/linux/ext4_fs.h b/include/linux/ext4_fs.h
index 4494f8e..84ef557 100644
--- a/include/linux/ext4_fs.h
+++ b/include/linux/ext4_fs.h
@@ -151,8 +151,8 @@ struct ext4_allocation_request {
 struct ext4_group_desc
 {
 	le32_t	bg_block_bitmap;	/* Blocks bitmap block */
-	__le32	bg_inode_bitmap;	/* Inodes bitmap block */
-	__le32	bg_inode_table;		/* Inodes table block */
+	le32_t	bg_inode_bitmap;	/* Inodes bitmap block */
+	le32_t	bg_inode_table;		/* Inodes table block */
 	__le16	bg_free_blocks_count;	/* Free blocks count */
 	__le16	bg_free_inodes_count;	/* Free inodes count */
 	__le16	bg_used_dirs_count;	/* Directories count */
@@ -161,8 +161,8 @@ struct ext4_group_desc
 	__le16  bg_itable_unused;	/* Unused inodes count */
 	__le16  bg_checksum;		/* crc16(sb_uuid+group+desc) */
 	le32_t	bg_block_bitmap_hi;	/* Blocks bitmap block MSB */
-	__le32	bg_inode_bitmap_hi;	/* Inodes bitmap block MSB */
-	__le32	bg_inode_table_hi;	/* Inodes table block MSB */
+	le32_t	bg_inode_bitmap_hi;	/* Inodes bitmap block MSB */
+	le32_t	bg_inode_table_hi;	/* Inodes table block MSB */
 };
 
 #define EXT4_BG_INODE_UNINIT	0x0001 /* Inode table/bitmap not in use */
-- 
1.5.3.1.91.gd3392-dirty

[PATCH 3/7] ext4: Convert s_blocks_count_hi and s_blocks_count to le32_t

[Aneesh Kumar K.V]

Convert s_blocks_count_hi and s_blocks_count to le32_t
This helps in finding BUGs due to direct partial access of
these split 64 bit values

Also fix direct partial access in ext4 code

Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
---
 fs/ext4/mballoc.c       |    4 ++--
 fs/ext4/super.c         |    4 ++--
 include/linux/ext4_fs.h |   12 ++++++------
 3 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/fs/ext4/mballoc.c b/fs/ext4/mballoc.c
index c4e6c92..5b804b2 100644
--- a/fs/ext4/mballoc.c
+++ b/fs/ext4/mballoc.c
@@ -4036,7 +4036,7 @@ int ext4_mb_initialize_context(struct ext4_allocation_context *ac,
 	/* start searching from the goal */
 	goal = ar->goal;
 	if (goal < le32_to_cpu(es->s_first_data_block) ||
-			goal >= le32_to_cpu(es->s_blocks_count))
+			goal >= ext4_blocks_count(es))
 		goal = le32_to_cpu(es->s_first_data_block);
 	ext4_get_group_no_and_offset(sb, goal, &group, &block);
 
@@ -4337,7 +4337,7 @@ void ext4_mb_free_blocks(handle_t *handle, struct inode *inode,
 	es = EXT4_SB(sb)->s_es;
 	if (block < le32_to_cpu(es->s_first_data_block) ||
 	    block + count < block ||
-	    block + count > le32_to_cpu(es->s_blocks_count)) {
+	    block + count > ext4_blocks_count(es)) {
 		ext4_error(sb, __FUNCTION__,
 			    "Freeing blocks not in datazone - "
 			    "block = %lu, count = %lu", block, count);
diff --git a/fs/ext4/super.c b/fs/ext4/super.c
index b9dda19..069d5f3 100644
--- a/fs/ext4/super.c
+++ b/fs/ext4/super.c
@@ -2653,7 +2653,7 @@ static int ext4_statfs (struct dentry * dentry, struct kstatfs * buf)
 
 	if (test_opt(sb, MINIX_DF)) {
 		sbi->s_overhead_last = 0;
-	} else if (sbi->s_blocks_last != le32_to_cpu(es->s_blocks_count)) {
+	} else if (sbi->s_blocks_last != ext4_blocks_count(es)) {
 		unsigned long ngroups = sbi->s_groups_count, i;
 		ext4_fsblk_t overhead = 0;
 		smp_rmb();
@@ -2688,7 +2688,7 @@ static int ext4_statfs (struct dentry * dentry, struct kstatfs * buf)
 		overhead += ngroups * (2 + sbi->s_itb_per_group);
 		sbi->s_overhead_last = overhead;
 		smp_wmb();
-		sbi->s_blocks_last = le32_to_cpu(es->s_blocks_count);
+		sbi->s_blocks_last = ext4_blocks_count(es);
 	}
 
 	buf->f_type = EXT4_SUPER_MAGIC;
diff --git a/include/linux/ext4_fs.h b/include/linux/ext4_fs.h
index 84ef557..53a8665 100644
--- a/include/linux/ext4_fs.h
+++ b/include/linux/ext4_fs.h
@@ -563,7 +563,7 @@ do {									       \
  */
 struct ext4_super_block {
 /*00*/	__le32	s_inodes_count;		/* Inodes count */
-	__le32	s_blocks_count;		/* Blocks count */
+	le32_t	s_blocks_count;		/* Blocks count */
 	__le32	s_r_blocks_count;	/* Reserved blocks count */
 	__le32	s_free_blocks_count;	/* Free blocks count */
 /*10*/	__le32	s_free_inodes_count;	/* Free inodes count */
@@ -633,7 +633,7 @@ struct ext4_super_block {
 	__le32	s_mkfs_time;		/* When the filesystem was created */
 	__le32	s_jnl_blocks[17];	/* Backup of the journal inode */
 	/* 64bit support valid if EXT4_FEATURE_COMPAT_64BIT */
-/*150*/	__le32	s_blocks_count_hi;	/* Blocks count */
+/*150*/	le32_t	s_blocks_count_hi;	/* Blocks count */
 	__le32	s_r_blocks_count_hi;	/* Reserved blocks count */
 	__le32	s_free_blocks_count_hi;	/* Free blocks count */
 	__le16	s_min_extra_isize;	/* All inodes have at least # bytes */
@@ -1066,8 +1066,8 @@ extern void ext4_inode_table_set(struct super_block *sb,
 
 static inline ext4_fsblk_t ext4_blocks_count(struct ext4_super_block *es)
 {
-	return ((ext4_fsblk_t)le32_to_cpu(es->s_blocks_count_hi) << 32) |
-		le32_to_cpu(es->s_blocks_count);
+	return ((ext4_fsblk_t)le32_to_cpu(es->s_blocks_count_hi.value) << 32) |
+		le32_to_cpu(es->s_blocks_count.value);
 }
 
 static inline ext4_fsblk_t ext4_r_blocks_count(struct ext4_super_block *es)
@@ -1085,8 +1085,8 @@ static inline ext4_fsblk_t ext4_free_blocks_count(struct ext4_super_block *es)
 static inline void ext4_blocks_count_set(struct ext4_super_block *es,
 					 ext4_fsblk_t blk)
 {
-	es->s_blocks_count = cpu_to_le32((u32)blk);
-	es->s_blocks_count_hi = cpu_to_le32(blk >> 32);
+	es->s_blocks_count.value = cpu_to_le32((u32)blk);
+	es->s_blocks_count_hi.value = cpu_to_le32(blk >> 32);
 }
 
 static inline void ext4_free_blocks_count_set(struct ext4_super_block *es,
-- 
1.5.3.1.91.gd3392-dirty

[PATCH 4/7] ext4: Convert s_r_blocks_count[_hi] s_free_blocks_count[_hi] to le32_t

[Aneesh Kumar K.V]

Convert s_r_blocks_count[_hi] s_free_blocks_count[_hi] to le32_t
This helps in finding BUGs due to direct partial access of
these split 64 bit values

Also fix direct partial access in ext4 code

Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
---
 fs/ext4/super.c         |    2 +-
 include/linux/ext4_fs.h |   24 ++++++++++++------------
 2 files changed, 13 insertions(+), 13 deletions(-)

diff --git a/fs/ext4/super.c b/fs/ext4/super.c
index 069d5f3..8bc5afb 100644
--- a/fs/ext4/super.c
+++ b/fs/ext4/super.c
@@ -2695,7 +2695,7 @@ static int ext4_statfs (struct dentry * dentry, struct kstatfs * buf)
 	buf->f_bsize = sb->s_blocksize;
 	buf->f_blocks = ext4_blocks_count(es) - sbi->s_overhead_last;
 	buf->f_bfree = percpu_counter_sum(&sbi->s_freeblocks_counter);
-	es->s_free_blocks_count = cpu_to_le32(buf->f_bfree);
+	ext4_free_blocks_count_set(es, buf->f_bfree);
 	buf->f_bavail = buf->f_bfree - ext4_r_blocks_count(es);
 	if (buf->f_bfree < ext4_r_blocks_count(es))
 		buf->f_bavail = 0;
diff --git a/include/linux/ext4_fs.h b/include/linux/ext4_fs.h
index 53a8665..b96e792 100644
--- a/include/linux/ext4_fs.h
+++ b/include/linux/ext4_fs.h
@@ -564,8 +564,8 @@ do {									       \
 struct ext4_super_block {
 /*00*/	__le32	s_inodes_count;		/* Inodes count */
 	le32_t	s_blocks_count;		/* Blocks count */
-	__le32	s_r_blocks_count;	/* Reserved blocks count */
-	__le32	s_free_blocks_count;	/* Free blocks count */
+	le32_t	s_r_blocks_count;	/* Reserved blocks count */
+	le32_t	s_free_blocks_count;	/* Free blocks count */
 /*10*/	__le32	s_free_inodes_count;	/* Free inodes count */
 	__le32	s_first_data_block;	/* First Data Block */
 	__le32	s_log_block_size;	/* Block size */
@@ -634,8 +634,8 @@ struct ext4_super_block {
 	__le32	s_jnl_blocks[17];	/* Backup of the journal inode */
 	/* 64bit support valid if EXT4_FEATURE_COMPAT_64BIT */
 /*150*/	le32_t	s_blocks_count_hi;	/* Blocks count */
-	__le32	s_r_blocks_count_hi;	/* Reserved blocks count */
-	__le32	s_free_blocks_count_hi;	/* Free blocks count */
+	le32_t	s_r_blocks_count_hi;	/* Reserved blocks count */
+	le32_t	s_free_blocks_count_hi;	/* Free blocks count */
 	__le16	s_min_extra_isize;	/* All inodes have at least # bytes */
 	__le16	s_want_extra_isize; 	/* New inodes should reserve # bytes */
 	__le32	s_flags;		/* Miscellaneous flags */
@@ -1072,14 +1072,14 @@ static inline ext4_fsblk_t ext4_blocks_count(struct ext4_super_block *es)
 
 static inline ext4_fsblk_t ext4_r_blocks_count(struct ext4_super_block *es)
 {
-	return ((ext4_fsblk_t)le32_to_cpu(es->s_r_blocks_count_hi) << 32) |
-		le32_to_cpu(es->s_r_blocks_count);
+	return ((ext4_fsblk_t)le32_to_cpu(es->s_r_blocks_count_hi.value) << 32) |
+		le32_to_cpu(es->s_r_blocks_count.value);
 }
 
 static inline ext4_fsblk_t ext4_free_blocks_count(struct ext4_super_block *es)
 {
-	return ((ext4_fsblk_t)le32_to_cpu(es->s_free_blocks_count_hi) << 32) |
-		le32_to_cpu(es->s_free_blocks_count);
+	return ((ext4_fsblk_t)le32_to_cpu(es->s_free_blocks_count_hi.value) << 32) |
+		le32_to_cpu(es->s_free_blocks_count.value);
 }
 
 static inline void ext4_blocks_count_set(struct ext4_super_block *es,
@@ -1092,15 +1092,15 @@ static inline void ext4_blocks_count_set(struct ext4_super_block *es,
 static inline void ext4_free_blocks_count_set(struct ext4_super_block *es,
 					      ext4_fsblk_t blk)
 {
-	es->s_free_blocks_count = cpu_to_le32((u32)blk);
-	es->s_free_blocks_count_hi = cpu_to_le32(blk >> 32);
+	es->s_free_blocks_count.value = cpu_to_le32((u32)blk);
+	es->s_free_blocks_count_hi.value = cpu_to_le32(blk >> 32);
 }
 
 static inline void ext4_r_blocks_count_set(struct ext4_super_block *es,
 					   ext4_fsblk_t blk)
 {
-	es->s_r_blocks_count = cpu_to_le32((u32)blk);
-	es->s_r_blocks_count_hi = cpu_to_le32(blk >> 32);
+	es->s_r_blocks_count.value = cpu_to_le32((u32)blk);
+	es->s_r_blocks_count_hi.value = cpu_to_le32(blk >> 32);
 }
 
 
-- 
1.5.3.1.91.gd3392-dirty

[PATCH 5/7] ext4: Convert ext4_extent.ee_start and ee_start_hi to le32_t and le16_t

[Aneesh Kumar K.V]

Convert ext4_extent.ee_start and ee_start_hi to le32_t and le16_t
This helps in finding BUGs due to direct partial access of
these split 48 bit values

Also fix direct partial access in ext4 code

Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
---
 fs/ext4/extents.c               |   12 +++++-------
 fs/ext4/migrate.c               |    4 ++--
 include/linux/ext4_fs_extents.h |    4 ++--
 3 files changed, 9 insertions(+), 11 deletions(-)

diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c
index 9ccdb85..a7e70d8 100644
--- a/fs/ext4/extents.c
+++ b/fs/ext4/extents.c
@@ -52,8 +52,8 @@ static ext4_fsblk_t ext_pblock(struct ext4_extent *ex)
 {
 	ext4_fsblk_t block;
 
-	block = le32_to_cpu(ex->ee_start);
-	block |= ((ext4_fsblk_t) le16_to_cpu(ex->ee_start_hi) << 31) << 1;
+	block = le32_to_cpu(ex->ee_start.value);
+	block |= ((ext4_fsblk_t) le16_to_cpu(ex->ee_start_hi.value) << 31) << 1;
 	return block;
 }
 
@@ -77,8 +77,8 @@ static ext4_fsblk_t idx_pblock(struct ext4_extent_idx *ix)
  */
 static void ext4_ext_store_pblock(struct ext4_extent *ex, ext4_fsblk_t pb)
 {
-	ex->ee_start = cpu_to_le32((unsigned long) (pb & 0xffffffff));
-	ex->ee_start_hi = cpu_to_le16((unsigned long) ((pb >> 31) >> 1) & 0xffff);
+	ex->ee_start.value = cpu_to_le32((unsigned long) (pb & 0xffffffff));
+	ex->ee_start_hi.value = cpu_to_le16((unsigned long) ((pb >> 31) >> 1) & 0xffff);
 }
 
 /*
@@ -1551,8 +1551,7 @@ has_space:
 	eh->eh_entries = cpu_to_le16(le16_to_cpu(eh->eh_entries)+1);
 	nearex = path[depth].p_ext;
 	nearex->ee_block = newext->ee_block;
-	nearex->ee_start = newext->ee_start;
-	nearex->ee_start_hi = newext->ee_start_hi;
+	ext4_ext_store_pblock(nearex, ext_pblock(newext));
 	nearex->ee_len = newext->ee_len;
 
 merge:
@@ -2321,7 +2320,6 @@ int ext4_ext_convert_to_initialized(handle_t *handle, struct inode *inode,
 	}
 	/* ex2: iblock to iblock + maxblocks-1 : initialised */
 	ex2->ee_block = cpu_to_le32(iblock);
-	ex2->ee_start = cpu_to_le32(newblock);
 	ext4_ext_store_pblock(ex2, newblock);
 	ex2->ee_len = cpu_to_le16(allocated);
 	if (ex2 != ex)
diff --git a/fs/ext4/migrate.c b/fs/ext4/migrate.c
index 5efcdd0..2e9a807 100644
--- a/fs/ext4/migrate.c
+++ b/fs/ext4/migrate.c
@@ -24,8 +24,8 @@ struct list_blocks_struct {
 /* will go away */
 static void ext4_ext_store_pblock(struct ext4_extent *ex, ext4_fsblk_t pb)
 {
-	ex->ee_start = cpu_to_le32((unsigned long) (pb & 0xffffffff));
-	ex->ee_start_hi = cpu_to_le16((unsigned long) ((pb >> 31) >> 1) & 0xffff);
+	ex->ee_start.value = cpu_to_le32((unsigned long) (pb & 0xffffffff));
+	ex->ee_start_hi.value = cpu_to_le16((unsigned long) ((pb >> 31) >> 1) & 0xffff);
 }
 
 static int finish_range(handle_t *handle, struct inode *inode,
diff --git a/include/linux/ext4_fs_extents.h b/include/linux/ext4_fs_extents.h
index 5ed0891..112926e 100644
--- a/include/linux/ext4_fs_extents.h
+++ b/include/linux/ext4_fs_extents.h
@@ -73,8 +73,8 @@
 struct ext4_extent {
 	__le32	ee_block;	/* first logical block extent covers */
 	__le16	ee_len;		/* number of blocks covered by extent */
-	__le16	ee_start_hi;	/* high 16 bits of physical block */
-	__le32	ee_start;	/* low 32 bits of physical block */
+	le16_t	ee_start_hi;	/* high 16 bits of physical block */
+	le32_t	ee_start;	/* low 32 bits of physical block */
 };
 
 /*
-- 
1.5.3.1.91.gd3392-dirty

[PATCH 6/7] ext4: Convert ext4_extent_idx.ei_leaf and ei_leaf_hi to le32_t and le16_t

[Aneesh Kumar K.V]

Convert ext4_extent_idx.ei_leaf and ei_leaf_hi to le32_t and le16_t
This helps in finding BUGs due to direct partial access of
these split 48 bit values

Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
---
 fs/ext4/extents.c               |    8 ++++----
 fs/ext4/migrate.c               |    4 ++--
 include/linux/ext4_fs_extents.h |    4 ++--
 3 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c
index a7e70d8..858997b 100644
--- a/fs/ext4/extents.c
+++ b/fs/ext4/extents.c
@@ -65,8 +65,8 @@ static ext4_fsblk_t idx_pblock(struct ext4_extent_idx *ix)
 {
 	ext4_fsblk_t block;
 
-	block = le32_to_cpu(ix->ei_leaf);
-	block |= ((ext4_fsblk_t) le16_to_cpu(ix->ei_leaf_hi) << 31) << 1;
+	block = le32_to_cpu(ix->ei_leaf.value);
+	block |= ((ext4_fsblk_t) le16_to_cpu(ix->ei_leaf_hi.value) << 31) << 1;
 	return block;
 }
 
@@ -88,8 +88,8 @@ static void ext4_ext_store_pblock(struct ext4_extent *ex, ext4_fsblk_t pb)
  */
 static void ext4_idx_store_pblock(struct ext4_extent_idx *ix, ext4_fsblk_t pb)
 {
-	ix->ei_leaf = cpu_to_le32((unsigned long) (pb & 0xffffffff));
-	ix->ei_leaf_hi = cpu_to_le16((unsigned long) ((pb >> 31) >> 1) & 0xffff);
+	ix->ei_leaf.value = cpu_to_le32((unsigned long) (pb & 0xffffffff));
+	ix->ei_leaf_hi.value = cpu_to_le16((unsigned long) ((pb >> 31) >> 1) & 0xffff);
 }
 
 static handle_t *ext4_ext_journal_restart(handle_t *handle, int needed)
diff --git a/fs/ext4/migrate.c b/fs/ext4/migrate.c
index 2e9a807..6a4de2c 100644
--- a/fs/ext4/migrate.c
+++ b/fs/ext4/migrate.c
@@ -375,8 +375,8 @@ static ext4_fsblk_t idx_pblock(struct ext4_extent_idx *ix)
 {
 	ext4_fsblk_t block;
 
-	block = le32_to_cpu(ix->ei_leaf);
-	block |= ((ext4_fsblk_t) le16_to_cpu(ix->ei_leaf_hi) << 31) << 1;
+	block = le32_to_cpu(ix->ei_leaf.value);
+	block |= ((ext4_fsblk_t) le16_to_cpu(ix->ei_leaf_hi.value) << 31) << 1;
 	return block;
 }
 
diff --git a/include/linux/ext4_fs_extents.h b/include/linux/ext4_fs_extents.h
index 112926e..2930120 100644
--- a/include/linux/ext4_fs_extents.h
+++ b/include/linux/ext4_fs_extents.h
@@ -83,9 +83,9 @@ struct ext4_extent {
  */
 struct ext4_extent_idx {
 	__le32	ei_block;	/* index covers logical blocks from 'block' */
-	__le32	ei_leaf;	/* pointer to the physical block of the next *
+	le32_t	ei_leaf;	/* pointer to the physical block of the next *
 				 * level. leaf or next index could be there */
-	__le16	ei_leaf_hi;	/* high 16 bits of physical block */
+	le16_t	ei_leaf_hi;	/* high 16 bits of physical block */
 	__u16	ei_unused;
 };
 
-- 
1.5.3.1.91.gd3392-dirty

[PATCH 7/7] ext4: sparse fixes

[Aneesh Kumar K.V]

Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
---
 fs/ext4/fsync.c |    2 +-
 fs/ext4/xattr.c |    4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/fs/ext4/fsync.c b/fs/ext4/fsync.c
index 2a167d7..8d50879 100644
--- a/fs/ext4/fsync.c
+++ b/fs/ext4/fsync.c
@@ -47,7 +47,7 @@ int ext4_sync_file(struct file * file, struct dentry *dentry, int datasync)
 	struct inode *inode = dentry->d_inode;
 	int ret = 0;
 
-	J_ASSERT(ext4_journal_current_handle() == 0);
+	J_ASSERT(ext4_journal_current_handle() == NULL);
 
 	/*
 	 * data=writeback:
diff --git a/fs/ext4/xattr.c b/fs/ext4/xattr.c
index 0fbaa0e..d796213 100644
--- a/fs/ext4/xattr.c
+++ b/fs/ext4/xattr.c
@@ -1120,7 +1120,7 @@ int ext4_expand_extra_isize_ea(struct inode *inode, int new_extra_isize,
 	int total_ino, total_blk;
 	void *base, *start, *end;
 	int extra_isize = 0, error = 0, tried_min_extra_isize = 0;
-	int s_min_extra_isize = EXT4_SB(inode->i_sb)->s_es->s_min_extra_isize;
+	int s_min_extra_isize = le16_to_cpu(EXT4_SB(inode->i_sb)->s_es->s_min_extra_isize);
 
 	down_write(&EXT4_I(inode)->xattr_sem);
 retry:
@@ -1292,7 +1292,7 @@ retry:
 
 		i.name = b_entry_name;
 		i.value = buffer;
-		i.value_len = cpu_to_le32(size);
+		i.value_len = size;
 		error = ext4_xattr_block_find(inode, &i, bs);
 		if (error)
 			goto cleanup;
-- 
1.5.3.1.91.gd3392-dirty

Re: [PATCH 1/7] ext4: Introduce le32_t and le16_t

[Aneesh Kumar K.V]

Andreas,

The patches are on top of patch queue. I haven't touched
the uid and gid of ext4_inode. Do you think i should
change that too ?

7/7 is not the part of the series. But it is important.
Should i send it as a separate patch for the 2.6.24 ?

-aneesh

Re: [PATCH 1/7] ext4: Introduce le32_t and le16_t

[Andreas Dilger]

On Sep 25, 2007  14:41 +0530, Aneesh Kumar K.V wrote:
> The patches are on top of patch queue. I haven't touched
> the uid and gid of ext4_inode. Do you think i should
> change that too ?

You can add a Signed-off-by: Andreas Dilger <adilger@clusterfs.com>
to those patches.  As I suspected there were a number of bugs hiding
in there.  I would also change the uid and gid fields, even if we
don't suspect any problems now.

> 7/7 is not the part of the series. But it is important.
> Should i send it as a separate patch for the 2.6.24 ?

Probably yes.

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

Re: [PATCH 1/7] ext4: Introduce le32_t and le16_t

[Aneesh Kumar K.V]

Andreas Dilger wrote:
> On Sep 25, 2007  14:41 +0530, Aneesh Kumar K.V wrote:
>> The patches are on top of patch queue. I haven't touched
>> the uid and gid of ext4_inode. Do you think i should
>> change that too ?
> 
> You can add a Signed-off-by: Andreas Dilger <adilger@clusterfs.com>
> to those patches.  As I suspected there were a number of bugs hiding
> in there.  I would also change the uid and gid fields, even if we
> don't suspect any problems now.
> 

the primary reason for me not looking at uid gid and file_acl fields are

a) we can mount with option nouid32 and that will look at only the low 16 
   bits
b) same is true with the gid fields

c) Also i_file_acl. If the creater os is HURD we look at only the low 32 bits.


That means all the pace where we access these fields we have conditional access
That makes it less error prone. 

with the changes the code will some what as below 

         if(!(test_opt (inode->i_sb, NO_UID32))) {
-               inode->i_uid |= le16_to_cpu(raw_inode->i_uid_high) << 16;
-               inode->i_gid |= le16_to_cpu(raw_inode->i_gid_high) << 16;
+               inode->i_uid = ext4_get_i_uid(raw_inode);
+               inode->i_gid = ext4_get_i_gid(raw_inode);
+       } else {
+               inode->i_uid = ext4_get_i_uid_low(raw_inode);
+               inode->i_gid = ext4_get_i_gid_low(raw_inode);
        }


-aneesh

Re: [PATCH 1/7] ext4: Introduce le32_t and le16_t

[Dave Kleikamp]

On Tue, 2007-09-25 at 14:33 +0530, Aneesh Kumar K.V wrote:
> ext4 file system layout contain different split members
> like bg_block_bitmap  and bg_block_bitmap_hi. Introduce
> data type le32_t and le16_t to be used as the type of
> these split members. This prevents these members frome
> being accessed directly. Accesing them directly gives
> a compiler warning. This helps in catching some BUGS
> due to direct partial access of these split fields.

Ugh.  I don't like this typedef at all. It just makes the data type more
confusing.

Why not just change the name of bg_block_bitmap to something like
bg_block_bitmap_lo or _bg_block_bitmap?  It should be clear from the
name that it shouldn't be used without careful consideration.

> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
> ---
>  fs/ext4/super.c           |    8 ++++----
>  include/linux/ext4_fs.h   |    4 ++--
>  include/linux/ext4_fs_i.h |    4 ++++
>  3 files changed, 10 insertions(+), 6 deletions(-)
> 
> diff --git a/fs/ext4/super.c b/fs/ext4/super.c
> index d035653..c8f8e4d 100644
> --- a/fs/ext4/super.c
> +++ b/fs/ext4/super.c
> @@ -70,9 +70,9 @@ static void ext4_write_super_lockfs(struct super_block *sb);
>  ext4_fsblk_t ext4_block_bitmap(struct super_block *sb,
>  			       struct ext4_group_desc *bg)
>  {
> -	return le32_to_cpu(bg->bg_block_bitmap) |
> +	return le32_to_cpu(bg->bg_block_bitmap.value) |
>  		(EXT4_DESC_SIZE(sb) >= EXT4_MIN_DESC_SIZE_64BIT ?
> -		 (ext4_fsblk_t)le32_to_cpu(bg->bg_block_bitmap_hi) << 32 : 0);
> +		 (ext4_fsblk_t)le32_to_cpu(bg->bg_block_bitmap_hi.value) << 32 : 0);
>  }
> 
>  ext4_fsblk_t ext4_inode_bitmap(struct super_block *sb,
> @@ -94,9 +94,9 @@ ext4_fsblk_t ext4_inode_table(struct super_block *sb,
>  void ext4_block_bitmap_set(struct super_block *sb,
>  			   struct ext4_group_desc *bg, ext4_fsblk_t blk)
>  {
> -	bg->bg_block_bitmap = cpu_to_le32((u32)blk);
> +	bg->bg_block_bitmap.value = cpu_to_le32((u32)blk);
>  	if (EXT4_DESC_SIZE(sb) >= EXT4_MIN_DESC_SIZE_64BIT)
> -		bg->bg_block_bitmap_hi = cpu_to_le32(blk >> 32);
> +		bg->bg_block_bitmap_hi.value = cpu_to_le32(blk >> 32);
>  }
> 
>  void ext4_inode_bitmap_set(struct super_block *sb,
> diff --git a/include/linux/ext4_fs.h b/include/linux/ext4_fs.h
> index 046a6a7..4494f8e 100644
> --- a/include/linux/ext4_fs.h
> +++ b/include/linux/ext4_fs.h
> @@ -150,7 +150,7 @@ struct ext4_allocation_request {
>   */
>  struct ext4_group_desc
>  {
> -	__le32	bg_block_bitmap;	/* Blocks bitmap block */
> +	le32_t	bg_block_bitmap;	/* Blocks bitmap block */
>  	__le32	bg_inode_bitmap;	/* Inodes bitmap block */
>  	__le32	bg_inode_table;		/* Inodes table block */
>  	__le16	bg_free_blocks_count;	/* Free blocks count */
> @@ -160,7 +160,7 @@ struct ext4_group_desc
>  	__u32	bg_reserved[2];		/* Likely block/inode bitmap checksum */
>  	__le16  bg_itable_unused;	/* Unused inodes count */
>  	__le16  bg_checksum;		/* crc16(sb_uuid+group+desc) */
> -	__le32	bg_block_bitmap_hi;	/* Blocks bitmap block MSB */
> +	le32_t	bg_block_bitmap_hi;	/* Blocks bitmap block MSB */
>  	__le32	bg_inode_bitmap_hi;	/* Inodes bitmap block MSB */
>  	__le32	bg_inode_table_hi;	/* Inodes table block MSB */
>  };
> diff --git a/include/linux/ext4_fs_i.h b/include/linux/ext4_fs_i.h
> index 22ba80e..d7eb271 100644
> --- a/include/linux/ext4_fs_i.h
> +++ b/include/linux/ext4_fs_i.h
> @@ -27,6 +27,10 @@ typedef int ext4_grpblk_t;
>  /* data type for filesystem-wide blocks number */
>  typedef unsigned long long ext4_fsblk_t;
> 
> +/* data type to carry split 64 and 48 bits */
> +typedef struct { __le16 value; } le16_t;
> +typedef struct { __le32 value; } le32_t;
> +
>  struct ext4_reserve_window {
>  	ext4_fsblk_t	_rsv_start;	/* First byte reserved */
>  	ext4_fsblk_t	_rsv_end;	/* Last byte reserved or 0 */
-- 
David Kleikamp
IBM Linux Technology Center

Re: [PATCH 1/7] ext4: Introduce le32_t and le16_t

[Aneesh Kumar K.V]

Dave Kleikamp wrote:
> On Tue, 2007-09-25 at 14:33 +0530, Aneesh Kumar K.V wrote:
>> ext4 file system layout contain different split members
>> like bg_block_bitmap  and bg_block_bitmap_hi. Introduce
>> data type le32_t and le16_t to be used as the type of
>> these split members. This prevents these members frome
>> being accessed directly. Accesing them directly gives
>> a compiler warning. This helps in catching some BUGS
>> due to direct partial access of these split fields.
> 
> Ugh.  I don't like this typedef at all. It just makes the data type more
> confusing.


Confusing enough to make people look at them more carefully.



> 
> Why not just change the name of bg_block_bitmap to something like
> bg_block_bitmap_lo or _bg_block_bitmap?  It should be clear from the
> name that it shouldn't be used without careful consideration.
> 

even if we rename the variables, I guess we would like to have helper functions
for accessing these values. That would mean the code is finally going to look more
or less the same except the typedef. But the typedef actually save us from serious
misuse of these variables.


-aneesh

Re: [PATCH 1/7] ext4: Introduce le32_t and le16_t

[Dave Kleikamp]

On Tue, 2007-09-25 at 21:15 +0530, Aneesh Kumar K.V wrote:
> 
> Dave Kleikamp wrote:
> > On Tue, 2007-09-25 at 14:33 +0530, Aneesh Kumar K.V wrote:
> >> ext4 file system layout contain different split members
> >> like bg_block_bitmap  and bg_block_bitmap_hi. Introduce
> >> data type le32_t and le16_t to be used as the type of
> >> these split members. This prevents these members frome
> >> being accessed directly. Accesing them directly gives
> >> a compiler warning. This helps in catching some BUGS
> >> due to direct partial access of these split fields.
> > 
> > Ugh.  I don't like this typedef at all. It just makes the data type more
> > confusing.
> 
> 
> Confusing enough to make people look at them more carefully.
> 
> 
> 
> > 
> > Why not just change the name of bg_block_bitmap to something like
> > bg_block_bitmap_lo or _bg_block_bitmap?  It should be clear from the
> > name that it shouldn't be used without careful consideration.
> > 
> 
> even if we rename the variables, I guess we would like to have helper functions
> for accessing these values. That would mean the code is finally going to look more
> or less the same except the typedef.

I see that as a good thing.  The typedef is the part I don't like.

> But the typedef actually save us from serious
> misuse of these variables.

You could just as easily write code that misuses bg_block_bitmap->value,
as you could bg_block_bitmap_lo.  I actually think using a name that
describes that it's only half-a-value is more clear.

Shaggy
-- 
David Kleikamp
IBM Linux Technology Center