Skip to contentSkip to conversation

naturally align struct ext4_allocation_request

PATCH Superseded Eric Sandeen 3 participants 4 messages Jun 24, 2009 – Jul 6, 2009 ext4 file system

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 naturally align struct ext4_allocation_request

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.

---

fs/ext4/ext4.h +6 -6
index cc7d5ed..9a438cf 100644
@@ -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;
 };
#1 lore raw
Quoted 53 lines · 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.

---

fs/ext4/ext4.h +2 -2
index cc7d5ed..9a438cf 100644
@@ -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;

fs/ext4/ext4.h typedef unsigned int ext4_group_t; · line 109

Quoted 10 lines
fs/ext4/ext4.h +4 -2
-       /* 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 [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

#2 lore raw

PATCH v2 1/1 naturally align struct ext4_allocation_request

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.

---

V2: fix comments, thanks to Mike Snitzer

fs/ext4/ext4.h +7 -7
index cc7d5ed..cefd94a 100644
@@ -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;
 };
maintainer
#3 lore raw
Quoted 8 lines · 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...
grouping 32-bit members together closes these holes and shrinks the structure by 12 bytes.

Thanks, added to the ext4 patch queue.

- Ted