linux-kernel - Re: [PATCH v3 6/6] iio: adc: ad7173: Reduce device info struct size

lists.openwall.net		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 linux-cve-announce 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: Wed, 29 May 2024 15:32:51 -0500
From: David Lechner <dlechner@...libre.com>
To: Nuno Sá <noname.nuno@...il.com>,
 dumitru.ceclan@...log.com
Cc: Lars-Peter Clausen <lars@...afoo.de>,
 Michael Hennerich <Michael.Hennerich@...log.com>,
 Jonathan Cameron <jic23@...nel.org>, Rob Herring <robh@...nel.org>,
 Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
 Conor Dooley <conor+dt@...nel.org>, linux-iio@...r.kernel.org,
 devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
 Dumitru Ceclan <mitrutzceclan@...il.com>
Subject: Re: [PATCH v3 6/6] iio: adc: ad7173: Reduce device info struct size

On 5/29/24 7:23 AM, Nuno Sá wrote:
> On Mon, 2024-05-27 at 20:02 +0300, Dumitru Ceclan via B4 Relay wrote:
>> From: Dumitru Ceclan <dumitru.ceclan@...log.com>
>>
>> Reduce the size used by the device info struct by packing the bool
>>  fields within the same byte. This reduces the struct size from 52 bytes
>>  to 44 bytes.
>>
>> Signed-off-by: Dumitru Ceclan <dumitru.ceclan@...log.com>
>> ---
>>  drivers/iio/adc/ad7173.c | 16 ++++++++--------
>>  1 file changed, 8 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/iio/adc/ad7173.c b/drivers/iio/adc/ad7173.c
>> index 328685ce25e0..e8357a21d513 100644
>> --- a/drivers/iio/adc/ad7173.c
>> +++ b/drivers/iio/adc/ad7173.c
>> @@ -179,15 +179,15 @@ struct ad7173_device_info {
>>  	unsigned int clock;
>>  	unsigned int id;
>>  	char *name;
>> -	bool has_current_inputs;
>> -	bool has_vcom_input;
>> -	bool has_temp;
>> +	bool has_current_inputs		:1;
>> +	bool has_vcom_input		:1;
>> +	bool has_temp			:1;
>>  	/* ((AVDD1 − AVSS)/5) */
>> -	bool has_common_input;
>> -	bool has_input_buf;
>> -	bool has_int_ref;
>> -	bool has_ref2;
>> -	bool higher_gpio_bits;
>> +	bool has_common_input		:1;
>> +	bool has_input_buf		:1;
>> +	bool has_int_ref		:1;
>> +	bool has_ref2			:1;
>> +	bool higher_gpio_bits		:1;
>>  	u8 num_gpios;
>>  };
>>  
>>
> 
> This is really a very micro optimization... I would drop it tbh but no strong
> feelings about it.
> 
> - Nuno Sá

This only considers RAM size and not code size too. At least on ARM arch
every time we read or write to one of these fields, the code is now
implicitly `((field & 0x1) >> bits)` so two extra assembly instructions
for each read and write. This could be bigger than the size saved in
the structs.