diff mbox series

[v3,4/4] x86/mce: Avoid unnecessary padding in struct mce_bank

Message ID 20220211223442.254489-5-Smita.KoralahalliChannabasappa@amd.com (mailing list archive)
State New, archived
Headers show
Series x86/mce, EDAC/mce_amd: Support extended MCA_ADDR address on SMCA systems | expand

Commit Message

Smita Koralahalli Feb. 11, 2022, 10:34 p.m. UTC 
Include struct mce_bank member "init" in the bitfield by changing its type
from bool to get rid of unnecessary padding and to reduce the overall
struct size.

Outputs collected before and after the change.

$ pahole -C mce_bank arch/x86/kernel/cpu/mce/core.o

before:

	/* size: 24, cachelines: 1, members: 5 */
	/* bit holes: 1, sum bit holes: 62 bits */
	/* bit_padding: 2 bits */
	/* last cacheline: 24 bytes */

after:

	/* size: 16, cachelines: 1, members: 5 */
	/* last cacheline: 16 bytes */

No functional changes.

Signed-off-by: Smita Koralahalli <Smita.KoralahalliChannabasappa@amd.com>
Reviewed-by: Yazen Ghannam <yazen.ghannam@amd.com>
---
 arch/x86/kernel/cpu/mce/internal.h | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

Comments

Borislav Petkov Feb. 22, 2022, 3:36 p.m. UTC | #1 
On Fri, Feb 11, 2022 at 04:34:42PM -0600, Smita Koralahalli wrote:
> Include struct mce_bank member "init" in the bitfield by changing its type
> from bool to get rid of unnecessary padding and to reduce the overall
> struct size.
> 
> Outputs collected before and after the change.
> 
> $ pahole -C mce_bank arch/x86/kernel/cpu/mce/core.o
> 
> before:
> 
> 	/* size: 24, cachelines: 1, members: 5 */
> 	/* bit holes: 1, sum bit holes: 62 bits */
> 	/* bit_padding: 2 bits */
> 	/* last cacheline: 24 bytes */
> 
> after:
> 
> 	/* size: 16, cachelines: 1, members: 5 */
> 	/* last cacheline: 16 bytes */

I don't mind cleanups like that but when you send them as part of a set
adding new functionality, the usual rule is to put bug fixes, cleanups,
improvements, etc to the existing code *first*, and then, ontop you add
your new code.

IOW, this patch should be first in your set.

Thx.
Koralahalli Channabasappa, Smita Feb. 22, 2022, 9:02 p.m. UTC | #2 
Hi Boris,

On 2/22/22 9:36 AM, Borislav Petkov wrote:

> On Fri, Feb 11, 2022 at 04:34:42PM -0600, Smita Koralahalli wrote:
>> Include struct mce_bank member "init" in the bitfield by changing its type
>> from bool to get rid of unnecessary padding and to reduce the overall
>> struct size.
>>
>> Outputs collected before and after the change.
>>
>> $ pahole -C mce_bank arch/x86/kernel/cpu/mce/core.o
>>
>> before:
>>
>> 	/* size: 24, cachelines: 1, members: 5 */
>> 	/* bit holes: 1, sum bit holes: 62 bits */
>> 	/* bit_padding: 2 bits */
>> 	/* last cacheline: 24 bytes */
>>
>> after:
>>
>> 	/* size: 16, cachelines: 1, members: 5 */
>> 	/* last cacheline: 16 bytes */
> I don't mind cleanups like that but when you send them as part of a set
> adding new functionality, the usual rule is to put bug fixes, cleanups,
> improvements, etc to the existing code *first*, and then, ontop you add
> your new code.
>
> IOW, this patch should be first in your set.
>
> Thx.
>
Thanks for letting me know.
Will correct and move it to first in the next series.

Thanks,
Smita
diff mbox series

Patch

diff --git a/arch/x86/kernel/cpu/mce/internal.h b/arch/x86/kernel/cpu/mce/internal.h
index 422387f8699d..0b0f55f0c585 100644
--- a/arch/x86/kernel/cpu/mce/internal.h
+++ b/arch/x86/kernel/cpu/mce/internal.h
@@ -177,13 +177,14 @@  extern struct mce_vendor_flags mce_flags;
 
 struct mce_bank {
 	u64			ctl;			/* subevents to enable */
-	bool			init;			/* initialise bank? */
+
+	__u64 init			: 1,		/* initialise bank? */
 
 	/*
 	 * (AMD) MCA_CONFIG[McaLsbInStatusSupported]: This bit indicates
 	 * the LSB field is found in MCA_STATUS, when set.
 	 */
-	__u64 lsb_in_status		: 1,
+	      lsb_in_status		: 1,
 
 	/*
 	 * (AMD) MCA_CONFIG[McaX]: This bit when set indicates a given
@@ -195,7 +196,7 @@  struct mce_bank {
 	 */
 	      mcax			: 1,
 
-	      __reserved_1		: 62;
+	      __reserved_1		: 61;
 };
 
 DECLARE_PER_CPU_READ_MOSTLY(struct mce_bank[MAX_NR_BANKS], mce_banks_array);