On Mon, 2006-09-18 at 15:36 -0500, Dave Kleikamp wrote: > EXT2: Remove superblock lock contention in ext2_statfs > > Fix a performance degradation introduced in 2.6.17. (30% degradation running > dbench with 16 threads) > > Patch 21730eed11de42f22afcbd43f450a1872a0b5ea1, which claims to make > EXT2_DEBUG work again, moves the taking of the kernel lock out of debug-only > code in ext2_count_free_inodes and ext2_count_free_blocks and into > ext2_statfs. This patch reverses that part of the patch. > > Signed-off-by: Dave Kleikamp <shaggy@xxxxxxxxxxxxxx> Eric Sandeen pointed out to me that taking the superblock lock in ext2_count_free_* will cause a deadlock when EXT2FS_DEBUG is enabled, since the superblock is locked in write_super(). We found that the same problem was fixed in ext3 with this patch (forgive the long link): http://git.kernel.org/git/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commitdiff;h=5b11687924e40790deb0d5f959247ade82196665;hp=2384f55f8aa520172c995965bd2f8a9740d53095 The patch below just removes the use of the superblock lock in the debug code. > diff --git a/fs/ext2/balloc.c b/fs/ext2/balloc.c > index d487043..fddefff 100644 > --- a/fs/ext2/balloc.c > +++ b/fs/ext2/balloc.c > @@ -539,7 +539,6 @@ unsigned long ext2_count_free (struct bu > > #endif /* EXT2FS_DEBUG */ > > -/* Superblock must be locked */ > unsigned long ext2_count_free_blocks (struct super_block * sb) > { > struct ext2_group_desc * desc; > @@ -549,6 +548,7 @@ #ifdef EXT2FS_DEBUG > unsigned long bitmap_count, x; > struct ext2_super_block *es; > > + lock_super(sb); > es = EXT2_SB(sb)->s_es; > desc_count = 0; > bitmap_count = 0; > @@ -572,6 +572,7 @@ #ifdef EXT2FS_DEBUG > printk("ext2_count_free_blocks: stored = %lu, computed = %lu, %lu\n", > (long)le32_to_cpu(es->s_free_blocks_count), > desc_count, bitmap_count); > + unlock_super(sb); > return bitmap_count; > #else > for (i = 0; i < EXT2_SB(sb)->s_groups_count; i++) { > diff --git a/fs/ext2/ialloc.c b/fs/ext2/ialloc.c > index de85c61..5d1d1c9 100644 > --- a/fs/ext2/ialloc.c > +++ b/fs/ext2/ialloc.c > @@ -637,7 +637,6 @@ fail: > return ERR_PTR(err); > } > > -/* Superblock must be locked */ > unsigned long ext2_count_free_inodes (struct super_block * sb) > { > struct ext2_group_desc *desc; > @@ -649,6 +648,7 @@ #ifdef EXT2FS_DEBUG > unsigned long bitmap_count = 0; > struct buffer_head *bitmap_bh = NULL; > > + lock_super(sb); > es = EXT2_SB(sb)->s_es; > for (i = 0; i < EXT2_SB(sb)->s_groups_count; i++) { > unsigned x; > @@ -671,6 +671,7 @@ #ifdef EXT2FS_DEBUG > printk("ext2_count_free_inodes: stored = %lu, computed = %lu, %lu\n", > percpu_counter_read(&EXT2_SB(sb)->s_freeinodes_counter), > desc_count, bitmap_count); > + unlock_super(sb); > return desc_count; > #else > for (i = 0; i < EXT2_SB(sb)->s_groups_count; i++) { > diff --git a/fs/ext2/super.c b/fs/ext2/super.c > index ca5bfb6..4286ff6 100644 > --- a/fs/ext2/super.c > +++ b/fs/ext2/super.c > @@ -1083,7 +1083,6 @@ static int ext2_statfs (struct dentry * > unsigned long overhead; > int i; > > - lock_super(sb); > if (test_opt (sb, MINIX_DF)) > overhead = 0; > else { > @@ -1124,7 +1123,6 @@ static int ext2_statfs (struct dentry * > buf->f_files = le32_to_cpu(sbi->s_es->s_inodes_count); > buf->f_ffree = ext2_count_free_inodes (sb); > buf->f_namelen = EXT2_NAME_LEN; > - unlock_super(sb); > return 0; > } > EXT2: Remove superblock lock contention in ext2_statfs Fix a performance degradation introduced in 2.6.17. (30% degradation running dbench with 16 threads) Patch 21730eed11de42f22afcbd43f450a1872a0b5ea1, which claims to make EXT2_DEBUG work again, moves the taking of the kernel lock out of debug-only code in ext2_count_free_inodes and ext2_count_free_blocks and into ext2_statfs. The same problem was fixed in ext3 by removing the lock completely (patch 5b11687924e40790deb0d5f959247ade82196665) Signed-off-by: Dave Kleikamp <shaggy@xxxxxxxxxxxxxx> diff --git a/fs/ext2/balloc.c b/fs/ext2/balloc.c index d487043..b1981d0 100644 --- a/fs/ext2/balloc.c +++ b/fs/ext2/balloc.c @@ -539,7 +539,6 @@ unsigned long ext2_count_free (struct bu #endif /* EXT2FS_DEBUG */ -/* Superblock must be locked */ unsigned long ext2_count_free_blocks (struct super_block * sb) { struct ext2_group_desc * desc; diff --git a/fs/ext2/ialloc.c b/fs/ext2/ialloc.c index de85c61..695f69c 100644 --- a/fs/ext2/ialloc.c +++ b/fs/ext2/ialloc.c @@ -637,7 +637,6 @@ fail: return ERR_PTR(err); } -/* Superblock must be locked */ unsigned long ext2_count_free_inodes (struct super_block * sb) { struct ext2_group_desc *desc; diff --git a/fs/ext2/super.c b/fs/ext2/super.c index ca5bfb6..4286ff6 100644 --- a/fs/ext2/super.c +++ b/fs/ext2/super.c @@ -1083,7 +1083,6 @@ static int ext2_statfs (struct dentry * unsigned long overhead; int i; - lock_super(sb); if (test_opt (sb, MINIX_DF)) overhead = 0; else { @@ -1124,7 +1123,6 @@ static int ext2_statfs (struct dentry * buf->f_files = le32_to_cpu(sbi->s_es->s_inodes_count); buf->f_ffree = ext2_count_free_inodes (sb); buf->f_namelen = EXT2_NAME_LEN; - unlock_super(sb); return 0; } -- David Kleikamp IBM Linux Technology Center - 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
