Group some variables based on their sizes to reduce hole and avoid padding.
On x86_64, this shrinks the size from 112 to 96 bytes.
This should have no real impact on memory allocation because 'struct
spi_message' is mostly used on stack, but it can save a few cycles
when the structure is initialized with spi_message_init() and co.
Signed-off-by: Christophe JAILLET <[email protected]>
---
Using pahole
Before:
======
struct spi_message {
struct list_head transfers; /* 0 16 */
struct spi_device * spi; /* 16 8 */
unsigned int is_dma_mapped:1; /* 24: 0 4 */
/* XXX 31 bits hole, try to pack */
/* XXX 4 bytes hole, try to pack */
void (*complete)(void *); /* 32 8 */
void * context; /* 40 8 */
unsigned int frame_length; /* 48 4 */
unsigned int actual_length; /* 52 4 */
int status; /* 56 4 */
/* XXX 4 bytes hole, try to pack */
/* --- cacheline 1 boundary (64 bytes) --- */
struct list_head queue; /* 64 16 */
void * state; /* 80 8 */
struct list_head resources; /* 88 16 */
bool prepared; /* 104 1 */
/* size: 112, cachelines: 2, members: 12 */
/* sum members: 93, holes: 2, sum holes: 8 */
/* sum bitfield members: 1 bits, bit holes: 1, sum bit holes: 31 bits */
/* padding: 7 */
/* last cacheline: 48 bytes */
};
After:
=====
struct spi_message {
struct list_head transfers; /* 0 16 */
struct spi_device * spi; /* 16 8 */
unsigned int is_dma_mapped:1; /* 24: 0 4 */
/* XXX 7 bits hole, try to pack */
/* Bitfield combined with next fields */
bool prepared; /* 25 1 */
/* XXX 2 bytes hole, try to pack */
int status; /* 28 4 */
void (*complete)(void *); /* 32 8 */
void * context; /* 40 8 */
unsigned int frame_length; /* 48 4 */
unsigned int actual_length; /* 52 4 */
struct list_head queue; /* 56 16 */
/* --- cacheline 1 boundary (64 bytes) was 8 bytes ago --- */
void * state; /* 72 8 */
struct list_head resources; /* 80 16 */
/* size: 96, cachelines: 2, members: 12 */
/* sum members: 93, holes: 1, sum holes: 2 */
/* sum bitfield members: 1 bits, bit holes: 1, sum bit holes: 7 bits */
/* last cacheline: 32 bytes */
};
---
include/linux/spi/spi.h | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/include/linux/spi/spi.h b/include/linux/spi/spi.h
index 4fa26b9a3572..bdb35a91b4bf 100644
--- a/include/linux/spi/spi.h
+++ b/include/linux/spi/spi.h
@@ -1093,6 +1093,9 @@ struct spi_message {
unsigned is_dma_mapped:1;
+ /* spi_prepare_message() was called for this message */
+ bool prepared;
+
/* REVISIT: we might want a flag affecting the behavior of the
* last transfer ... allowing things like "read 16 bit length L"
* immediately followed by "read L bytes". Basically imposing
@@ -1105,11 +1108,11 @@ struct spi_message {
*/
/* Completion is reported through a callback */
+ int status;
void (*complete)(void *context);
void *context;
unsigned frame_length;
unsigned actual_length;
- int status;
/* For optional use by whatever driver currently owns the
* spi_message ... between calls to spi_async and then later
@@ -1120,9 +1123,6 @@ struct spi_message {
/* List of spi_res reources when the spi message is processed */
struct list_head resources;
-
- /* spi_prepare_message() was called for this message */
- bool prepared;
};
static inline void spi_message_init_no_memset(struct spi_message *m)
--
2.34.1
On 3/2/23 1:58 AM, Christophe JAILLET wrote:
> Group some variables based on their sizes to reduce hole and avoid padding.
> On x86_64, this shrinks the size from 112 to 96 bytes.
>
> This should have no real impact on memory allocation because 'struct
> spi_message' is mostly used on stack, but it can save a few cycles
> when the structure is initialized with spi_message_init() and co.
>
> Signed-off-by: Christophe JAILLET <[email protected]>
Tested-by: Muhammad Usama Anjum <[email protected]>
Reviewed-by: Muhammad Usama Anjum <[email protected]>
> ---
> Using pahole
>
> Before:
> ======
> struct spi_message {
> struct list_head transfers; /* 0 16 */
> struct spi_device * spi; /* 16 8 */
> unsigned int is_dma_mapped:1; /* 24: 0 4 */
>
> /* XXX 31 bits hole, try to pack */
> /* XXX 4 bytes hole, try to pack */
>
> void (*complete)(void *); /* 32 8 */
> void * context; /* 40 8 */
> unsigned int frame_length; /* 48 4 */
> unsigned int actual_length; /* 52 4 */
> int status; /* 56 4 */
>
> /* XXX 4 bytes hole, try to pack */
>
> /* --- cacheline 1 boundary (64 bytes) --- */
> struct list_head queue; /* 64 16 */
> void * state; /* 80 8 */
> struct list_head resources; /* 88 16 */
> bool prepared; /* 104 1 */
>
> /* size: 112, cachelines: 2, members: 12 */
> /* sum members: 93, holes: 2, sum holes: 8 */
> /* sum bitfield members: 1 bits, bit holes: 1, sum bit holes: 31 bits */
> /* padding: 7 */
> /* last cacheline: 48 bytes */
> };
>
>
> After:
> =====
> struct spi_message {
> struct list_head transfers; /* 0 16 */
> struct spi_device * spi; /* 16 8 */
> unsigned int is_dma_mapped:1; /* 24: 0 4 */
>
> /* XXX 7 bits hole, try to pack */
> /* Bitfield combined with next fields */
>
> bool prepared; /* 25 1 */
>
> /* XXX 2 bytes hole, try to pack */
>
> int status; /* 28 4 */
> void (*complete)(void *); /* 32 8 */
> void * context; /* 40 8 */
> unsigned int frame_length; /* 48 4 */
> unsigned int actual_length; /* 52 4 */
> struct list_head queue; /* 56 16 */
> /* --- cacheline 1 boundary (64 bytes) was 8 bytes ago --- */
> void * state; /* 72 8 */
> struct list_head resources; /* 80 16 */
>
> /* size: 96, cachelines: 2, members: 12 */
> /* sum members: 93, holes: 1, sum holes: 2 */
> /* sum bitfield members: 1 bits, bit holes: 1, sum bit holes: 7 bits */
> /* last cacheline: 32 bytes */
> };
> ---
> include/linux/spi/spi.h | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/include/linux/spi/spi.h b/include/linux/spi/spi.h
> index 4fa26b9a3572..bdb35a91b4bf 100644
> --- a/include/linux/spi/spi.h
> +++ b/include/linux/spi/spi.h
> @@ -1093,6 +1093,9 @@ struct spi_message {
>
> unsigned is_dma_mapped:1;
>
> + /* spi_prepare_message() was called for this message */
> + bool prepared;
> +
> /* REVISIT: we might want a flag affecting the behavior of the
> * last transfer ... allowing things like "read 16 bit length L"
> * immediately followed by "read L bytes". Basically imposing
> @@ -1105,11 +1108,11 @@ struct spi_message {
> */
>
> /* Completion is reported through a callback */
> + int status;
> void (*complete)(void *context);
> void *context;
> unsigned frame_length;
> unsigned actual_length;
> - int status;
>
> /* For optional use by whatever driver currently owns the
> * spi_message ... between calls to spi_async and then later
> @@ -1120,9 +1123,6 @@ struct spi_message {
>
> /* List of spi_res reources when the spi message is processed */
> struct list_head resources;
> -
> - /* spi_prepare_message() was called for this message */
> - bool prepared;
> };
>
> static inline void spi_message_init_no_memset(struct spi_message *m)
--
BR,
Muhammad Usama Anjum
On Wed, 01 Mar 2023 21:58:52 +0100, Christophe JAILLET wrote:
> Group some variables based on their sizes to reduce hole and avoid padding.
> On x86_64, this shrinks the size from 112 to 96 bytes.
>
> This should have no real impact on memory allocation because 'struct
> spi_message' is mostly used on stack, but it can save a few cycles
> when the structure is initialized with spi_message_init() and co.
>
> [...]
Applied to
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git for-next
Thanks!
[1/1] spi: Reorder fields in 'struct spi_message'
commit: ae2ade4ba58167f165fbf3db19380f9b72c56db8
All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.
You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.
If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.
Please add any relevant lists and maintainers to the CCs when replying
to this mail.
Thanks,
Mark