lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  PHC 
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:	Fri, 14 Nov 2008 14:26:06 -0700
From:	Andreas Dilger <>
To:	Pekka J Enberg <>
Subject: Re: [PATCH] ext2/ext3: allocate ->s_blockgroup_lock separately to
	avoid wasting space

On Nov 14, 2008  11:17 +0200, Pekka J Enberg wrote:
> As spotted by kmemtrace, struct ext2_sb_info is 17024 bytes and ext3_sb_info is
> 17152 bytes on 64-bit which makes them a very bad fit for SLAB allocators. In
> fact, both allocations are round up to the next available page size of 
> order 3 which is 32 KB.
> The culprit if the wasted memory is the ->s_blockgroup_lock which can be as
> big as 16 KB when CONFIG_NR_CPUS is set to 32. As struct blockgroup_lock is a
> perfect fit for order 2 page in the worst case, allocate ->s_blockgroup_lock
> separately to avoid wasting space.
> The change shrinks struct ext2_sb_info to 592 bytes and struct ext3_sb_info to
> 640 bytes which fits into a 1024 byte slab cache so now we allocate 16 KB + 1
> KB instead of 32 KB saving 15 KB of memory!
> Signed-off-by: Pekka Enberg <>

This looks very reasonable, with some minor comments below.
Could you please also include a patch for ext4.  Also, Andrew prefers that
the patches for ext2/ext3/ext4 are in separate emails.

> --- a/include/linux/blockgroup_lock.h
> +++ b/include/linux/blockgroup_lock.h
>  #define sb_bgl_lock(sb, block_group) \
> -	(&(sb)->s_blockgroup_lock.locks[(block_group) & (NR_BG_LOCKS-1)].lock)
> +	(&(sb)->s_blockgroup_lock->locks[(block_group) & (NR_BG_LOCKS-1)].lock)

How the struct is allocated seems like an implementation detail that doesn't
belong in blockgroup_lock.h at all, because "sb" is not "struct superblock"
but rather "struct ext[23]_sb_info".  In fact, changing this without also
patching ext4 would cause ext4 to break.

I would suggest to change this to take the s_blockgroup_lock as a parameter,

#define bgl_lock_ptr(bgl, block_group)
	(bgl->locks[(block_group) & (NR_BG_LOCKS - 1)].lock)

and then in ext[234]_fs_sb.h add a new helper in the same (first) patch:

#define sb_bgl_lock(sbi, block_group)
	bgl_lock_ptr(&sbi->s_blockgroup_lock, block_group)

and remove sb_bgl_lock() from blockgroup_lock.h entirely.  As part of the
later patches to change the s_blockgroup_lock allocations for each of
ext[234] this changes in ext[234]_fs_sb.h to:

#define sb_bgl_lock(sbi, block_group)
	bgl_lock_ptr(sbi->s_blockgroup_lock, block_group)

This allows each of the later patches to be landed separately without
breaking the build.

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
More majordomo info at

Powered by blists - more mailing lists