ext4: Introduce le32_t and le16_t
PATCH 2/7 ext4: Convert bg_inode_bitmap and bg_inode_table to new type
In reply to Aneesh Kumar K.VConvert 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
--- fs/ext4/balloc.c | 2 +- fs/ext4/super.c | 18 +++++++++--------- include/linux/ext4_fs.h | 8 ++++---- 3 files changed, 14 insertions(+), 14 deletions(-)
fs/ext4/balloc.c +1 -1
@@ -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); }
fs/ext4/super.c +9 -9
@@ -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 ); }/*
include/linux/ext4_fs.h +4 -4
@@ -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 */
Signature · 1 line
1.5.3.1.91.gd3392-dirty
PATCH 1/7 ext4: Introduce le32_t and le16_t
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.
--- 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(-)
fs/ext4/super.c +4 -4
@@ -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,
include/linux/ext4_fs.h +2 -2
@@ -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 */ };
include/linux/ext4_fs_i.h +4 -0
@@ -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 */
Signature · 1 line
1.5.3.1.91.gd3392-dirty
PATCH 3/7 ext4: Convert s_blocks_count_hi and s_blocks_count to le32_t
In reply to Aneesh Kumar K.VConvert 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
--- fs/ext4/mballoc.c | 4 ++-- fs/ext4/super.c | 4 ++-- include/linux/ext4_fs.h | 12 ++++++------ 3 files changed, 10 insertions(+), 10 deletions(-)
fs/ext4/mballoc.c +2 -2
@@ -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);
fs/ext4/super.c +2 -2
@@ -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 ;
include/linux/ext4_fs.h +6 -6
@@ -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,
Signature · 1 line
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
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
--- fs/ext4/super.c | 2 +- include/linux/ext4_fs.h | 24 ++++++++++++------------ 2 files changed, 13 insertions(+), 13 deletions(-)
fs/ext4/super.c +1 -1
@@ -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 ;
include/linux/ext4_fs.h +12 -12
@@ -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 ); }
Signature · 1 line
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
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
--- fs/ext4/extents.c | 12 +++++------- fs/ext4/migrate.c | 4 ++-- include/linux/ext4_fs_extents.h | 4 ++-- 3 files changed, 9 insertions(+), 11 deletions(-)
fs/ext4/extents.c +5 -7
@@ -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)
fs/ext4/migrate.c +2 -2
@@ -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,
include/linux/ext4_fs_extents.h +2 -2
@@ -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 */ };/*
Signature · 1 line
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
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
--- fs/ext4/extents.c | 8 ++++---- fs/ext4/migrate.c | 4 ++-- include/linux/ext4_fs_extents.h | 4 ++-- 3 files changed, 8 insertions(+), 8 deletions(-)
fs/ext4/extents.c +4 -4
@@ -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)
fs/ext4/migrate.c +2 -2
@@ -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; }
include/linux/ext4_fs_extents.h +2 -2
@@ -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; };
Signature · 1 line
1.5.3.1.91.gd3392-dirty
PATCH 7/7 ext4: sparse fixes
--- fs/ext4/fsync.c | 2 +- fs/ext4/xattr.c | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-)
fs/ext4/fsync.c +1 -1
@@ -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:
fs/ext4/xattr.c +2 -2
@@ -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;
Signature · 1 line
1.5.3.1.91.gd3392-dirty
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
Quoted 3 lines · Aneesh Kumar K.V
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 <[email protected]> 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.
Quoted 2 lines
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.
Quoted 10 lines · Andreas Dilger
Quoted 3 lines · Aneesh Kumar K.V
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 <[email protected]> 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
Quoted 7 lines · Aneesh Kumar K.V
ext4 file system layout contain different split members like
bg_block_bitmapandbg_block_bitmap_hi. Introduce data typele32_tandle16_tto 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.
Quoted 72 lines
Signed-off-by Aneesh Kumar K.V <[email protected]>--- 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(-)fs/ext4/super.c +4
-4@@ -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,include/linux/ext4_fs.h +2
-2@@ -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 */ };include/linux/ext4_fs_i.h +4
-0@@ -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 */
Signature · 2 lines
David Kleikamp IBM Linux Technology Center
Quoted 11 lines · Dave Kleikamp
Quoted 7 lines · Aneesh Kumar K.V
ext4 file system layout contain different split members like
bg_block_bitmapandbg_block_bitmap_hi. Introduce data typele32_tandle16_tto 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.
Quoted 5 lines
Why not just change the name of
bg_block_bitmapto something likebg_block_bitmap_loor_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
Quoted 20 lines · Aneesh Kumar K.V
Quoted 11 lines · Dave Kleikamp
Quoted 7 lines · Aneesh Kumar K.V
ext4 file system layout contain different split members like
bg_block_bitmapandbg_block_bitmap_hi. Introduce data typele32_tandle16_tto 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.
Quoted 1 line
Quoted 8 lines
Quoted 4 lines
Why not just change the name of
bg_block_bitmapto something likebg_block_bitmap_loor_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.
Quoted 2 lines
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
Signature · 2 lines
David Kleikamp IBM Linux Technology Center