| Message ID | 20240912131730.588673-1-colin.i.king@gmail.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [next] EDAC, pnd2: Make read-only const array intlv static | expand |
> From: Colin Ian King <colin.i.king@gmail.com> > [...] > Subject: [PATCH][next] EDAC, pnd2: Make read-only const array intlv static > > Don't populate the const read-only array intlv on the stack at run time, instead > make it static. > > Signed-off-by: Colin Ian King <colin.i.king@gmail.com> > --- > drivers/edac/pnd2_edac.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/edac/pnd2_edac.c b/drivers/edac/pnd2_edac.c index > f93f2f2b1cf2..af14c8a3279f 100644 > --- a/drivers/edac/pnd2_edac.c > +++ b/drivers/edac/pnd2_edac.c > @@ -372,7 +372,7 @@ static int gen_asym_mask(struct > b_cr_slice_channel_hash *p, > struct b_cr_asym_mem_region1_mchbar *as1, > struct b_cr_asym_2way_mem_region_mchbar > *as2way) { > - const int intlv[] = { 0x5, 0xA, 0x3, 0xC }; > + static const int intlv[] = { 0x5, 0xA, 0x3, 0xC }; > int mask = 0; > > if (as2way->asym_2way_interleave_enable) > @@ -489,7 +489,7 @@ static int dnv_get_registers(void) > */ > static int get_registers(void) > { > - const int intlv[] = { 10, 11, 12, 12 }; > + static const int intlv[] = { 10, 11, 12, 12 }; > I didn't see the why and the benefits of these changes. Could you elaborate more?
On 19/09/2024 08:19, Zhuo, Qiuxu wrote: >> From: Colin Ian King <colin.i.king@gmail.com> >> [...] >> Subject: [PATCH][next] EDAC, pnd2: Make read-only const array intlv static >> >> Don't populate the const read-only array intlv on the stack at run time, instead >> make it static. >> >> Signed-off-by: Colin Ian King <colin.i.king@gmail.com> >> --- >> drivers/edac/pnd2_edac.c | 4 ++-- >> 1 file changed, 2 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/edac/pnd2_edac.c b/drivers/edac/pnd2_edac.c index >> f93f2f2b1cf2..af14c8a3279f 100644 >> --- a/drivers/edac/pnd2_edac.c >> +++ b/drivers/edac/pnd2_edac.c >> @@ -372,7 +372,7 @@ static int gen_asym_mask(struct >> b_cr_slice_channel_hash *p, >> struct b_cr_asym_mem_region1_mchbar *as1, >> struct b_cr_asym_2way_mem_region_mchbar >> *as2way) { >> - const int intlv[] = { 0x5, 0xA, 0x3, 0xC }; >> + static const int intlv[] = { 0x5, 0xA, 0x3, 0xC }; >> int mask = 0; >> >> if (as2way->asym_2way_interleave_enable) >> @@ -489,7 +489,7 @@ static int dnv_get_registers(void) >> */ >> static int get_registers(void) >> { >> - const int intlv[] = { 10, 11, 12, 12 }; >> + static const int intlv[] = { 10, 11, 12, 12 }; >> > > I didn't see the why and the benefits of these changes. > Could you elaborate more? The non-const construct will generate code to put the array on the stack and this occurs on each call, so there is a small amount of extra object code overhead to do this at run time. Making it static will put the data into a data section so there is run-time penalty. So this change potentially shrinks the object code and run time overhead a very small amount. Colin
> From: Colin King (gmail) <colin.i.king@gmail.com> > [...] > >> - const int intlv[] = { 10, 11, 12, 12 }; > >> + static const int intlv[] = { 10, 11, 12, 12 }; > >> > > > > I didn't see the why and the benefits of these changes. > > Could you elaborate more? > > The non-const construct will generate code to put the array on the stack and Typo? s/non-const/non-static/ > this occurs on each call, so there is a small amount of extra object code > overhead to do this at run time. Making it static will put the data into a data > section so there is run-time penalty. So this change potentially shrinks the > object code and run time overhead a very small amount. > Thanks! As these are not in hot paths, there should be no measurable performance difference. But, I do see that the text size is reduced by 12 bytes [1] after these changes. Could you add a short description of this benefit in the commit message? Other than that, Reviewed-by: Qiuxu Zhuo <qiuxu.zhuo@intel.com> [1] $ size pnd2_edac.o.* text data bss dec hex filename 15632 264 1384 17280 4380 pnd2_edac.o.new 15644 264 1384 17292 438c pnd2_edac.o.old - Qiuxu
diff --git a/drivers/edac/pnd2_edac.c b/drivers/edac/pnd2_edac.c index f93f2f2b1cf2..af14c8a3279f 100644 --- a/drivers/edac/pnd2_edac.c +++ b/drivers/edac/pnd2_edac.c @@ -372,7 +372,7 @@ static int gen_asym_mask(struct b_cr_slice_channel_hash *p, struct b_cr_asym_mem_region1_mchbar *as1, struct b_cr_asym_2way_mem_region_mchbar *as2way) { - const int intlv[] = { 0x5, 0xA, 0x3, 0xC }; + static const int intlv[] = { 0x5, 0xA, 0x3, 0xC }; int mask = 0; if (as2way->asym_2way_interleave_enable) @@ -489,7 +489,7 @@ static int dnv_get_registers(void) */ static int get_registers(void) { - const int intlv[] = { 10, 11, 12, 12 }; + static const int intlv[] = { 10, 11, 12, 12 }; if (RD_REG(&tolud, b_cr_tolud_pci) || RD_REG(&touud_lo, b_cr_touud_lo_pci) ||
Don't populate the const read-only array intlv on the stack at run time, instead make it static. Signed-off-by: Colin Ian King <colin.i.king@gmail.com> --- drivers/edac/pnd2_edac.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)