On Jul 18, 2008 14:58 -0500, Eric Sandeen wrote:
> I noticed when filling a 1T filesystem with 4 threads using the
> fs_mark benchmark that I occasionally got checksum mismatch errors:
>
> 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.
Thanks for finding this.
> Signed-off-by: Eric Sandeen <sandeen@xxxxxxxxxx>
You can add Acked-by: Andreas Dilger <adilger@xxxxxxx>
> @@ -321,12 +321,15 @@ ext4_read_block_bitmap(struct super_bloc
> 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;
> }
Since this flag will only ever change from set to unset, and will in
many cases not be set, we should first check if it is set without the
spin_lock(), and then if set re-check under the lock. This avoids putting
an extra lock in this path. While it may seem that we are already locking
and slow because of the locked buffer_head, the sb_bgl_lock() is hashed
so many groups will share the same lock and we may cause contention on
other CPUs accessing other groups needlessly.
Cheers, Andreas
--
Andreas Dilger
Sr. Staff Engineer, Lustre Group
Sun Microsystems of Canada, Inc.
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
