Remove packed when possible #16
Conversation
For a lot of the IBA structures, the packed attribute has no effect.
List of impacted struct and checks were done this way:
Before applying this patch
- Generate a single file with all the IBA structs
(echo -e "#include <iba/ib_types.h>\n#include <stdio.h>\n\n\n"; for struct in $(git grep '^} PACK_SUFFIX' -- include/iba/ib_types.h | awk '{ print $NF }' | sed -e 's/;//'); do echo -e "$struct a_$struct;"; done) > ib_sizes.c
- Compile for both 32 and 64b
gcc -o ib_sizes.64.o -g3 -c ib_sizes.c -I./include/
gcc -o ib_sizes.32.o -g3 -m32 -c ib_sizes.c -I./include/
- Generate structure data using pahole
pahole ib_sizes.64.o > sizes.64.org
pahole ib_sizes.32.o > sizes.32.org
After applying this patch:
- Compile for both 32 and 64b
gcc -o ib_sizes.64.o -g3 -c ib_sizes.c -I./include/
gcc -o ib_sizes.32.o -g3 -m32 -c ib_sizes.c -I./include/
- Generate structure data using pahole
pahole ib_sizes.64.o > sizes.64.new
pahole ib_sizes.32.o > sizes.32.new
Diff pahole results:
diff sizes.64.org sizes.64.new
diff sizes.32.org sizes.32.new
As the patch remove effect-less attribute, no diff shows up
Signed-off-by: Nicolas Morey-Chaisemartin <nmoreychaisemartin@suse.com>
I visually inspected all struct changes where packing is removed and it looks fine to me. What is next step ? Should I test these changes out ? Are there more to come (and if so, would this patch be modified or would it be a subsequent patch to this) ? |
|
These changes should be 100% safe. Pahole checked all the struct in 32 and 64 bits and sees no change. There should probably be more changes but they are tricky. Not sure if they are worth doing |
I did some initial testing with this patch and my simple tests worked. I'll merge patch and some regression tests will be run probably over the weekend.
OK |
|
Manually merged |
|
Unfortunately some regressions are now failing. I started to investigate these failures. Not sure if this is an issue with the regressions or whether it's caused by this patch. Still investigating... |
|
It is likely just a regression issue. |
|
Closing |
WIP on issue #15
This removes the packed attribute from all structs that do not need it.
All remaining structs are impacted either by a size change or hole inserted for alignment.