From e29d1cde63be0b5f1739416b5574a83c34bf8eeb Mon Sep 17 00:00:00 2001 From: Eric Sandeen Date: Sat, 2 Aug 2008 21:21:02 -0400 Subject: ext4: sync up block and inode bitmap reading functions ext4_read_block_bitmap and read_inode_bitmap do essentially the same thing, and yet they are structured quite differently. I came across this difference while looking at doing bg locking during bg initialization. This patch: * removes unnecessary casts in the error messages * renames read_inode_bitmap to ext4_read_inode_bitmap * and more substantially, restructures the inode bitmap reading function to be more like the block bitmap counterpart. The change to the inode bitmap reader simplifies the locking to be applied in the next patch. Signed-off-by: Eric Sandeen Signed-off-by: Theodore Ts'o --- fs/ext4/balloc.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) (limited to 'fs/ext4/balloc.c') diff --git a/fs/ext4/balloc.c b/fs/ext4/balloc.c index 495ab21b983..386cb79ac4b 100644 --- a/fs/ext4/balloc.c +++ b/fs/ext4/balloc.c @@ -314,8 +314,8 @@ ext4_read_block_bitmap(struct super_block *sb, ext4_group_t block_group) if (unlikely(!bh)) { ext4_error(sb, __func__, "Cannot read block bitmap - " - "block_group = %d, block_bitmap = %llu", - (int)block_group, (unsigned long long)bitmap_blk); + "block_group = %lu, block_bitmap = %llu", + block_group, bitmap_blk); return NULL; } if (bh_uptodate_or_lock(bh)) @@ -331,8 +331,8 @@ ext4_read_block_bitmap(struct super_block *sb, ext4_group_t block_group) put_bh(bh); ext4_error(sb, __func__, "Cannot read block bitmap - " - "block_group = %d, block_bitmap = %llu", - (int)block_group, (unsigned long long)bitmap_blk); + "block_group = %lu, block_bitmap = %llu", + block_group, bitmap_blk); return NULL; } ext4_valid_block_bitmap(sb, desc, block_group, bh); -- cgit v1.2.3-70-g09d2 From b5f10eed8125702929e57cca7e5956b1b9b6d015 Mon Sep 17 00:00:00 2001 From: Eric Sandeen Date: Sat, 2 Aug 2008 21:21:08 -0400 Subject: ext4: lock block groups when initializing I noticed when filling a 1T filesystem with 4 threads using the fs_mark benchmark: fs_mark -d /mnt/test -D 256 -n 100000 -t 4 -s 20480 -F -S 0 that I occasionally got checksum mismatch errors: EXT4-fs error (device sdb): ext4_init_inode_bitmap: Checksum bad for group 6935 etc. I'd reliably get 4-5 of them during the run. It appears that the problem is likely a race to init the bg's when the uninit_bg feature is enabled. With the patch below, which adds sb_bgl_locking around initialization, I was able to complete several runs with no errors or warnings. Signed-off-by: Eric Sandeen Signed-off-by: Theodore Ts'o --- fs/ext4/balloc.c | 3 +++ fs/ext4/ialloc.c | 5 ++++- fs/ext4/mballoc.c | 3 +++ fs/ext4/super.c | 2 ++ 4 files changed, 12 insertions(+), 1 deletion(-) (limited to 'fs/ext4/balloc.c') diff --git a/fs/ext4/balloc.c b/fs/ext4/balloc.c index 386cb79ac4b..1ae5004e93f 100644 --- a/fs/ext4/balloc.c +++ b/fs/ext4/balloc.c @@ -321,12 +321,15 @@ ext4_read_block_bitmap(struct super_block *sb, ext4_group_t block_group) if (bh_uptodate_or_lock(bh)) return bh; + spin_lock(sb_bgl_lock(EXT4_SB(sb), block_group)); if (desc->bg_flags & cpu_to_le16(EXT4_BG_BLOCK_UNINIT)) { ext4_init_block_bitmap(sb, bh, block_group, desc); set_buffer_uptodate(bh); unlock_buffer(bh); + spin_unlock(sb_bgl_lock(EXT4_SB(sb), block_group)); return bh; } + spin_unlock(sb_bgl_lock(EXT4_SB(sb), block_group)); if (bh_submit_read(bh) < 0) { put_bh(bh); ext4_error(sb, __func__, diff --git a/fs/ext4/ialloc.c b/fs/ext4/ialloc.c index 09cdcd5914d..655e760212b 100644 --- a/fs/ext4/ialloc.c +++ b/fs/ext4/ialloc.c @@ -118,12 +118,15 @@ ext4_read_inode_bitmap(struct super_block *sb, ext4_group_t block_group) if (bh_uptodate_or_lock(bh)) return bh; + spin_lock(sb_bgl_lock(EXT4_SB(sb), block_group)); if (desc->bg_flags & cpu_to_le16(EXT4_BG_INODE_UNINIT)) { ext4_init_inode_bitmap(sb, bh, block_group, desc); set_buffer_uptodate(bh); unlock_buffer(bh); + spin_unlock(sb_bgl_lock(EXT4_SB(sb), block_group)); return bh; } + spin_unlock(sb_bgl_lock(EXT4_SB(sb), block_group)); if (bh_submit_read(bh) < 0) { put_bh(bh); ext4_error(sb, __func__, @@ -735,7 +738,7 @@ got: /* When marking the block group with * ~EXT4_BG_INODE_UNINIT we don't want to depend - * on the value of bg_itable_unsed even though + * on the value of bg_itable_unused even though * mke2fs could have initialized the same for us. * Instead we calculated the value below */ diff --git a/fs/ext4/mballoc.c b/fs/ext4/mballoc.c index 8d141a25bbe..4258d3289a6 100644 --- a/fs/ext4/mballoc.c +++ b/fs/ext4/mballoc.c @@ -787,13 +787,16 @@ static int ext4_mb_init_cache(struct page *page, char *incore) if (bh_uptodate_or_lock(bh[i])) continue; + spin_lock(sb_bgl_lock(EXT4_SB(sb), first_group + i)); if (desc->bg_flags & cpu_to_le16(EXT4_BG_BLOCK_UNINIT)) { ext4_init_block_bitmap(sb, bh[i], first_group + i, desc); set_buffer_uptodate(bh[i]); unlock_buffer(bh[i]); + spin_unlock(sb_bgl_lock(EXT4_SB(sb), first_group + i)); continue; } + spin_unlock(sb_bgl_lock(EXT4_SB(sb), first_group + i)); get_bh(bh[i]); bh[i]->b_end_io = end_buffer_read_sync; submit_bh(READ, bh[i]); diff --git a/fs/ext4/super.c b/fs/ext4/super.c index 876e1c62036..511997ef6f0 100644 --- a/fs/ext4/super.c +++ b/fs/ext4/super.c @@ -1621,6 +1621,7 @@ static int ext4_check_descriptors(struct super_block *sb) "(block %llu)!", i, inode_table); return 0; } + spin_lock(sb_bgl_lock(sbi, i)); if (!ext4_group_desc_csum_verify(sbi, i, gdp)) { printk(KERN_ERR "EXT4-fs: ext4_check_descriptors: " "Checksum for group %lu failed (%u!=%u)\n", @@ -1629,6 +1630,7 @@ static int ext4_check_descriptors(struct super_block *sb) if (!(sb->s_flags & MS_RDONLY)) return 0; } + spin_unlock(sb_bgl_lock(sbi, i)); if (!flexbg_flag) first_block += EXT4_BLOCKS_PER_GROUP(sb); } -- cgit v1.2.3-70-g09d2