On Wed, Jan 02, 2019 at 01:25:27PM -0800, Matt Ranostay wrote:
>
> > On Dec 24, 2018, at 01:58, Himanshu Jha <himanshujha199640@xxxxxxxxx> wrote:
> >
> >> On Fri, Dec 21, 2018 at 03:26:26PM -0800, Amir Mahdi Ghorbanian wrote:
> >> Replaced bool in struct with unsigned int bitfield to conserve space and
> >> more clearly define size of varibales
>
> Important thing to note is depending on padding, alignment, and position of field it probably won’t save any space.
Well, then what do you say about the following results ;-)
Before:
------
himanshu@himanshu-Vostro-3559:~/gsoc/linux$ pahole -C ad7192_platform_data drivers/staging/iio/adc/ad7192.o
struct ad7192_platform_data {
u16 vref_mv; /* 0 2 */
u8 clock_source_sel; /* 2 1 */
/* XXX 1 byte hole, try to pack */
u32 ext_clk_hz; /* 4 4 */
bool refin2_en; /* 8 1 */
bool rej60_en; /* 9 1 */
bool sinc3_en; /* 10 1 */
bool chop_en; /* 11 1 */
bool buf_en; /* 12 1 */
bool unipolar_en; /* 13 1 */
bool burnout_curr_en; /* 14 1 */
/* size: 16, cachelines: 1, members: 10 */
/* sum members: 14, holes: 1, sum holes: 1 */
/* padding: 1 */
/* last cacheline: 16 bytes */
};
After:
-----
himanshu@himanshu-Vostro-3559:~/gsoc/linux$ pahole -C ad7192_platform_data drivers/staging/iio/adc/ad7192.o
struct ad7192_platform_data {
u16 vref_mv; /* 0 2 */
u8 clock_source_sel; /* 2 1 */
/* XXX 1 byte hole, try to pack */
u32 ext_clk_hz; /* 4 4 */
unsigned int refin2_en:1; /* 8:31 4 */
unsigned int rej60_en:1; /* 8:30 4 */
unsigned int sinc3_en:1; /* 8:29 4 */
unsigned int chop_en:1; /* 8:28 4 */
unsigned int buf_en:1; /* 8:27 4 */
unsigned int unipolar_en:1; /* 8:26 4 */
unsigned int burnout_curr_en:1; /* 8:25 4 */
/* size: 12, cachelines: 1, members: 10 */
/* sum members: 11, holes: 1, sum holes: 1 */
/* bit_padding: 25 bits */
/* last cacheline: 12 bytes */
};
> Also for internal unpacked structures it really makes little sense to save a few bytes of data. Don’t read into that packed structures are good.. they usually aren’t :)
Yes, agreed, but I often see patches to save few bytes by marking
array as static.
There is some effort in this direction:
https://lore.kernel.org/lkml/20181221235230.GC13168@xxxxxxxx/
Very likely to get more patches if the above patch gets merged.
--
Himanshu Jha
Undergraduate Student
Department of Electronics & Communication
Guru Tegh Bahadur Institute of Technology
_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
