[PATCH] naturally align struct ext4_allocation_request

[PATCH] naturally align struct ext4_allocation_request

[Eric Sandeen]

As Ted noted, the ext4_allocation_request isn't well aligned.

Looking at it with pahole we're wasting space on 64-bit
arches:

struct ext4_allocation_request {
	struct inode *             inode;              /*     0     8 */
	ext4_lblk_t                logical;            /*     8     4 */

	/* XXX 4 bytes hole, try to pack */

	ext4_fsblk_t               goal;               /*    16     8 */
	ext4_lblk_t                lleft;              /*    24     4 */

	/* XXX 4 bytes hole, try to pack */

	ext4_fsblk_t               pleft;              /*    32     8 */
	ext4_lblk_t                lright;             /*    40     4 */

	/* XXX 4 bytes hole, try to pack */

	ext4_fsblk_t               pright;             /*    48     8 */
	unsigned int               len;                /*    56     4 */
	unsigned int               flags;              /*    60     4 */
	/* --- cacheline 1 boundary (64 bytes) --- */

	/* size: 64, cachelines: 1, members: 9 */
	/* sum members: 52, holes: 3, sum holes: 12 */
};

grouping 32-bit members together closes these holes and shrinks
the structure by 12 bytes.

Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---


diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
index cc7d5ed..9a438cf 100644
--- a/fs/ext4/ext4.h
+++ b/fs/ext4/ext4.h
@@ -93,20 +93,20 @@ typedef unsigned int ext4_group_t;
 struct ext4_allocation_request {
 	/* target inode for block we're allocating */
 	struct inode *inode;
+	/* how many blocks we want to allocate */
+	unsigned int len;
 	/* logical block in target inode */
 	ext4_lblk_t logical;
-	/* phys. target (a hint) */
-	ext4_fsblk_t goal;
 	/* the closest logical allocated block to the left */
 	ext4_lblk_t lleft;
-	/* phys. block for ^^^ */
-	ext4_fsblk_t pleft;
 	/* the closest logical allocated block to the right */
 	ext4_lblk_t lright;
+	/* phys. target (a hint) */
+	ext4_fsblk_t goal;
+	/* phys. block for ^^^ */
+	ext4_fsblk_t pleft;
 	/* phys. block for ^^^ */
 	ext4_fsblk_t pright;
-	/* how many blocks we want to allocate */
-	unsigned int len;
 	/* flags. see above EXT4_MB_HINT_* */
 	unsigned int flags;
 };

Re: [PATCH] naturally align struct ext4_allocation_request

[Mike Snitzer]

On Wed, Jun 24, 2009 at 12:30 PM, Eric Sandeen<sandeen@redhat.com> wrote:
> As Ted noted, the ext4_allocation_request isn't well aligned.
>
> Looking at it with pahole we're wasting space on 64-bit
> arches:
>
> struct ext4_allocation_request {
>        struct inode *             inode;              /*     0     8 */
>        ext4_lblk_t                logical;            /*     8     4 */
>
>        /* XXX 4 bytes hole, try to pack */
>
>        ext4_fsblk_t               goal;               /*    16     8 */
>        ext4_lblk_t                lleft;              /*    24     4 */
>
>        /* XXX 4 bytes hole, try to pack */
>
>        ext4_fsblk_t               pleft;              /*    32     8 */
>        ext4_lblk_t                lright;             /*    40     4 */
>
>        /* XXX 4 bytes hole, try to pack */
>
>        ext4_fsblk_t               pright;             /*    48     8 */
>        unsigned int               len;                /*    56     4 */
>        unsigned int               flags;              /*    60     4 */
>        /* --- cacheline 1 boundary (64 bytes) --- */
>
>        /* size: 64, cachelines: 1, members: 9 */
>        /* sum members: 52, holes: 3, sum holes: 12 */
> };
>
> grouping 32-bit members together closes these holes and shrinks
> the structure by 12 bytes.
>
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
> ---
>
>
> diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
> index cc7d5ed..9a438cf 100644
> --- a/fs/ext4/ext4.h
> +++ b/fs/ext4/ext4.h
> @@ -93,20 +93,20 @@ typedef unsigned int ext4_group_t;
>  struct ext4_allocation_request {
>        /* target inode for block we're allocating */
>        struct inode *inode;
> +       /* how many blocks we want to allocate */
> +       unsigned int len;
>        /* logical block in target inode */
>        ext4_lblk_t logical;
> -       /* phys. target (a hint) */
> -       ext4_fsblk_t goal;
>        /* the closest logical allocated block to the left */
>        ext4_lblk_t lleft;
> -       /* phys. block for ^^^ */
> -       ext4_fsblk_t pleft;
>        /* the closest logical allocated block to the right */
>        ext4_lblk_t lright;
> +       /* phys. target (a hint) */
> +       ext4_fsblk_t goal;
> +       /* phys. block for ^^^ */
> +       ext4_fsblk_t pleft;
>        /* phys. block for ^^^ */
>        ext4_fsblk_t pright;

Hey Eric,

Looks like the comments for pleft and pright should be updated to
reference lleft and lright respectively (rather than ^^^).

Mike
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH V2] naturally align struct ext4_allocation_request

[Eric Sandeen]

As Ted noted, the ext4_allocation_request isn't well aligned.

Looking at it with pahole we're wasting space on 64-bit
arches:

struct ext4_allocation_request {
        struct inode *             inode;              /*     0     8 */
        ext4_lblk_t                logical;            /*     8     4 */

        /* XXX 4 bytes hole, try to pack */

        ext4_fsblk_t               goal;               /*    16     8 */
        ext4_lblk_t                lleft;              /*    24     4 */

        /* XXX 4 bytes hole, try to pack */

        ext4_fsblk_t               pleft;              /*    32     8 */
        ext4_lblk_t                lright;             /*    40     4 */

        /* XXX 4 bytes hole, try to pack */

        ext4_fsblk_t               pright;             /*    48     8 */
        unsigned int               len;                /*    56     4 */
        unsigned int               flags;              /*    60     4 */
        /* --- cacheline 1 boundary (64 bytes) --- */

        /* size: 64, cachelines: 1, members: 9 */
        /* sum members: 52, holes: 3, sum holes: 12 */
};

grouping 32-bit members together closes these holes and shrinks
the structure by 12 bytes.

Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---

V2: fix comments, thanks to Mike Snitzer

diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
index cc7d5ed..cefd94a 100644
--- a/fs/ext4/ext4.h
+++ b/fs/ext4/ext4.h
@@ -93,20 +93,20 @@ typedef unsigned int ext4_group_t;
 struct ext4_allocation_request {
 	/* target inode for block we're allocating */
 	struct inode *inode;
+	/* how many blocks we want to allocate */
+	unsigned int len;
 	/* logical block in target inode */
 	ext4_lblk_t logical;
-	/* phys. target (a hint) */
-	ext4_fsblk_t goal;
 	/* the closest logical allocated block to the left */
 	ext4_lblk_t lleft;
-	/* phys. block for ^^^ */
-	ext4_fsblk_t pleft;
 	/* the closest logical allocated block to the right */
 	ext4_lblk_t lright;
-	/* phys. block for ^^^ */
+	/* phys. target (a hint) */
+	ext4_fsblk_t goal;
+	/* phys. block for the closest logical allocated block to the left */
+	ext4_fsblk_t pleft;
+	/* phys. block for the closest logical allocated block to the right */
 	ext4_fsblk_t pright;
-	/* how many blocks we want to allocate */
-	unsigned int len;
 	/* flags. see above EXT4_MB_HINT_* */
 	unsigned int flags;
 };

Re: [PATCH V2] naturally align struct ext4_allocation_request

[Theodore Tso]

On Thu, Jun 25, 2009 at 08:09:40PM -0500, Eric Sandeen wrote:
> As Ted noted, the ext4_allocation_request isn't well aligned.
> 
> Looking at it with pahole we're wasting space on 64-bit
> arches...
> grouping 32-bit members together closes these holes and shrinks
> the structure by 12 bytes.
> 
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>


Thanks, added to the ext4 patch queue.

						- Ted