* [PATCH] HID: Reorder fields in 'struct hid_field'
@ 2023-06-18 8:08 Christophe JAILLET
2023-06-19 5:18 ` Dan Carpenter
0 siblings, 1 reply; 4+ messages in thread
From: Christophe JAILLET @ 2023-06-18 8:08 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires
Cc: linux-kernel, kernel-janitors, Christophe JAILLET, linux-input
Group some variables based on their sizes to reduce hole and avoid padding.
On x86_64, this shrinks the size of 'struct hid_field'
from 136 to 128 bytes.
It saves a few bytes of memory and is more cache-line friendly.
Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
---
Using pahole
Before:
======
struct hid_field {
unsigned int physical; /* 0 4 */
unsigned int logical; /* 4 4 */
unsigned int application; /* 8 4 */
/* XXX 4 bytes hole, try to pack */
struct hid_usage * usage; /* 16 8 */
unsigned int maxusage; /* 24 4 */
unsigned int flags; /* 28 4 */
unsigned int report_offset; /* 32 4 */
unsigned int report_size; /* 36 4 */
unsigned int report_count; /* 40 4 */
unsigned int report_type; /* 44 4 */
__s32 * value; /* 48 8 */
__s32 * new_value; /* 56 8 */
/* --- cacheline 1 boundary (64 bytes) --- */
__s32 * usages_priorities; /* 64 8 */
__s32 logical_minimum; /* 72 4 */
__s32 logical_maximum; /* 76 4 */
__s32 physical_minimum; /* 80 4 */
__s32 physical_maximum; /* 84 4 */
__s32 unit_exponent; /* 88 4 */
unsigned int unit; /* 92 4 */
bool ignored; /* 96 1 */
/* XXX 7 bytes hole, try to pack */
struct hid_report * report; /* 104 8 */
unsigned int index; /* 112 4 */
/* XXX 4 bytes hole, try to pack */
struct hid_input * hidinput; /* 120 8 */
/* --- cacheline 2 boundary (128 bytes) --- */
__u16 dpad; /* 128 2 */
/* XXX 2 bytes hole, try to pack */
unsigned int slot_idx; /* 132 4 */
/* size: 136, cachelines: 3, members: 25 */
/* sum members: 119, holes: 4, sum holes: 17 */
/* last cacheline: 8 bytes */
};
After:
=====
struct hid_field {
unsigned int physical; /* 0 4 */
unsigned int logical; /* 4 4 */
unsigned int application; /* 8 4 */
/* XXX 4 bytes hole, try to pack */
struct hid_usage * usage; /* 16 8 */
unsigned int maxusage; /* 24 4 */
unsigned int flags; /* 28 4 */
unsigned int report_offset; /* 32 4 */
unsigned int report_size; /* 36 4 */
unsigned int report_count; /* 40 4 */
unsigned int report_type; /* 44 4 */
__s32 * value; /* 48 8 */
__s32 * new_value; /* 56 8 */
/* --- cacheline 1 boundary (64 bytes) --- */
__s32 * usages_priorities; /* 64 8 */
__s32 logical_minimum; /* 72 4 */
__s32 logical_maximum; /* 76 4 */
__s32 physical_minimum; /* 80 4 */
__s32 physical_maximum; /* 84 4 */
__s32 unit_exponent; /* 88 4 */
unsigned int unit; /* 92 4 */
struct hid_report * report; /* 96 8 */
unsigned int index; /* 104 4 */
bool ignored; /* 108 1 */
/* XXX 3 bytes hole, try to pack */
struct hid_input * hidinput; /* 112 8 */
__u16 dpad; /* 120 2 */
/* XXX 2 bytes hole, try to pack */
unsigned int slot_idx; /* 124 4 */
/* size: 128, cachelines: 2, members: 25 */
/* sum members: 119, holes: 3, sum holes: 9 */
};
---
include/linux/hid.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/linux/hid.h b/include/linux/hid.h
index 39e21e3815ad..5be5e671c263 100644
--- a/include/linux/hid.h
+++ b/include/linux/hid.h
@@ -480,9 +480,9 @@ struct hid_field {
__s32 physical_maximum;
__s32 unit_exponent;
unsigned unit;
- bool ignored; /* this field is ignored in this event */
struct hid_report *report; /* associated report */
unsigned index; /* index into report->field[] */
+ bool ignored; /* this field is ignored in this event */
/* hidinput data */
struct hid_input *hidinput; /* associated input structure */
__u16 dpad; /* dpad input code */
--
2.34.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] HID: Reorder fields in 'struct hid_field'
2023-06-18 8:08 [PATCH] HID: Reorder fields in 'struct hid_field' Christophe JAILLET
@ 2023-06-19 5:18 ` Dan Carpenter
2023-06-19 18:20 ` Christophe JAILLET
0 siblings, 1 reply; 4+ messages in thread
From: Dan Carpenter @ 2023-06-19 5:18 UTC (permalink / raw)
To: Christophe JAILLET
Cc: Jiri Kosina, Benjamin Tissoires, linux-kernel, kernel-janitors,
linux-input
On Sun, Jun 18, 2023 at 10:08:07AM +0200, Christophe JAILLET wrote:
> diff --git a/include/linux/hid.h b/include/linux/hid.h
> index 39e21e3815ad..5be5e671c263 100644
> --- a/include/linux/hid.h
> +++ b/include/linux/hid.h
> @@ -480,9 +480,9 @@ struct hid_field {
> __s32 physical_maximum;
> __s32 unit_exponent;
> unsigned unit;
> - bool ignored; /* this field is ignored in this event */
> struct hid_report *report; /* associated report */
> unsigned index; /* index into report->field[] */
> + bool ignored; /* this field is ignored in this event */
> /* hidinput data */
> struct hid_input *hidinput; /* associated input structure */
> __u16 dpad; /* dpad input code */
You could move the dpad next to the ignored to save another 4 bytes.
I think it is still grouped logically that way but I don't really know
what dpad is so I might be wrong.
struct hid_report *report; /* associated report */
unsigned index; /* index into report->field[] */
bool ignored; /* this field is ignored in this event */
/* hidinput data */
__u16 dpad; /* dpad input code */
struct hid_input *hidinput; /* associated input structure */
regards,
dan carpenter
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] HID: Reorder fields in 'struct hid_field'
2023-06-19 5:18 ` Dan Carpenter
@ 2023-06-19 18:20 ` Christophe JAILLET
2023-06-20 5:09 ` Dan Carpenter
0 siblings, 1 reply; 4+ messages in thread
From: Christophe JAILLET @ 2023-06-19 18:20 UTC (permalink / raw)
To: Dan Carpenter
Cc: Jiri Kosina, Benjamin Tissoires, linux-kernel, kernel-janitors,
linux-input
Le 19/06/2023 à 07:18, Dan Carpenter a écrit :
> On Sun, Jun 18, 2023 at 10:08:07AM +0200, Christophe JAILLET wrote:
>> diff --git a/include/linux/hid.h b/include/linux/hid.h
>> index 39e21e3815ad..5be5e671c263 100644
>> --- a/include/linux/hid.h
>> +++ b/include/linux/hid.h
>> @@ -480,9 +480,9 @@ struct hid_field {
>> __s32 physical_maximum;
>> __s32 unit_exponent;
>> unsigned unit;
>> - bool ignored; /* this field is ignored in this event */
>> struct hid_report *report; /* associated report */
>> unsigned index; /* index into report->field[] */
>> + bool ignored; /* this field is ignored in this event */
>> /* hidinput data */
>> struct hid_input *hidinput; /* associated input structure */
>> __u16 dpad; /* dpad input code */
>
> You could move the dpad next to the ignored to save another 4 bytes.
> I think it is still grouped logically that way but I don't really know
> what dpad is so I might be wrong.
>
> struct hid_report *report; /* associated report */
> unsigned index; /* index into report->field[] */
> bool ignored; /* this field is ignored in this event */
> /* hidinput data */
> __u16 dpad; /* dpad input code */
> struct hid_input *hidinput; /* associated input structure */
>
> regards,
> dan carpenter
>
>
In fact, not really,
It would fill a hole better, but would generate some padding at the end
of the struct:
bool ignored; /* 108 1 */
/* XXX 1 byte hole, try to pack */
__u16 dpad; /* 110 2 */
struct hid_input * hidinput; /* 112 8 */
unsigned int slot_idx; /* 120 4 */
/* size: 128, cachelines: 2, members: 25 */
/* sum members: 119, holes: 2, sum holes: 5 */
/* padding: 4 */
};
CJ
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] HID: Reorder fields in 'struct hid_field'
2023-06-19 18:20 ` Christophe JAILLET
@ 2023-06-20 5:09 ` Dan Carpenter
0 siblings, 0 replies; 4+ messages in thread
From: Dan Carpenter @ 2023-06-20 5:09 UTC (permalink / raw)
To: Christophe JAILLET
Cc: Jiri Kosina, Benjamin Tissoires, linux-kernel, kernel-janitors,
linux-input
On Mon, Jun 19, 2023 at 08:20:08PM +0200, Christophe JAILLET wrote:
> Le 19/06/2023 à 07:18, Dan Carpenter a écrit :
> > On Sun, Jun 18, 2023 at 10:08:07AM +0200, Christophe JAILLET wrote:
> > > diff --git a/include/linux/hid.h b/include/linux/hid.h
> > > index 39e21e3815ad..5be5e671c263 100644
> > > --- a/include/linux/hid.h
> > > +++ b/include/linux/hid.h
> > > @@ -480,9 +480,9 @@ struct hid_field {
> > > __s32 physical_maximum;
> > > __s32 unit_exponent;
> > > unsigned unit;
> > > - bool ignored; /* this field is ignored in this event */
> > > struct hid_report *report; /* associated report */
> > > unsigned index; /* index into report->field[] */
> > > + bool ignored; /* this field is ignored in this event */
> > > /* hidinput data */
> > > struct hid_input *hidinput; /* associated input structure */
> > > __u16 dpad; /* dpad input code */
> >
> > You could move the dpad next to the ignored to save another 4 bytes.
> > I think it is still grouped logically that way but I don't really know
> > what dpad is so I might be wrong.
> >
> > struct hid_report *report; /* associated report */
> > unsigned index; /* index into report->field[] */
> > bool ignored; /* this field is ignored in this event */
> > /* hidinput data */
> > __u16 dpad; /* dpad input code */
> > struct hid_input *hidinput; /* associated input structure */
> >
> > regards,
> > dan carpenter
> >
> >
>
> In fact, not really,
> It would fill a hole better, but would generate some padding at the end of
> the struct:
>
> bool ignored; /* 108 1 */
>
> /* XXX 1 byte hole, try to pack */
>
> __u16 dpad; /* 110 2 */
> struct hid_input * hidinput; /* 112 8 */
> unsigned int slot_idx; /* 120 4 */
>
> /* size: 128, cachelines: 2, members: 25 */
> /* sum members: 119, holes: 2, sum holes: 5 */
> /* padding: 4 */
> };
Ah. Right. I should have looked at that more closely.
regards,
dan carpenter
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2023-06-20 5:09 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-06-18 8:08 [PATCH] HID: Reorder fields in 'struct hid_field' Christophe JAILLET
2023-06-19 5:18 ` Dan Carpenter
2023-06-19 18:20 ` Christophe JAILLET
2023-06-20 5:09 ` Dan Carpenter
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).