Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1692498 > unrolled thread

[PATCH 0/7] constify x86 attribute_group structures.

Back to article view | Back to linux.kernel

Contents

  [PATCH 0/7] constify x86 attribute_group structures. Arvind Yadav <arvind.yadav.cs@gmail.com> - 2017-07-20 09:30 +0200
    [PATCH 3/7] perf: x86: intel: uncore_snb: constify attribute_group structures. Arvind Yadav <arvind.yadav.cs@gmail.com> - 2017-07-20 09:30 +0200
    [PATCH 7/7] x86: microcode: constify attribute_group structures. Arvind Yadav <arvind.yadav.cs@gmail.com> - 2017-07-20 09:30 +0200
      Re: [PATCH 7/7] x86: microcode: constify attribute_group structures. Borislav Petkov <bp@alien8.de> - 2017-07-20 10:20 +0200
        Re: [PATCH 7/7] x86: microcode: constify attribute_group structures. Arvind Yadav <arvind.yadav.cs@gmail.com> - 2017-07-20 10:20 +0200
          Re: [PATCH 7/7] x86: microcode: constify attribute_group structures. Borislav Petkov <bp@alien8.de> - 2017-07-20 10:40 +0200
            Re: [PATCH 7/7] x86: microcode: constify attribute_group structures. Arvind Yadav <arvind.yadav.cs@gmail.com> - 2017-07-20 11:10 +0200
              Re: [PATCH 7/7] x86: microcode: constify attribute_group structures. Borislav Petkov <bp@alien8.de> - 2017-07-20 12:40 +0200
                Re: [PATCH 7/7] x86: microcode: constify attribute_group structures. Arvind Yadav <arvind.yadav.cs@gmail.com> - 2017-07-20 13:10 +0200
    [PATCH 6/7] x86: therm_throt: constify attribute_group structures. Arvind Yadav <arvind.yadav.cs@gmail.com> - 2017-07-20 09:30 +0200

#1692498 — [PATCH 0/7] constify x86 attribute_group structures.

attribute_group are not supposed to change at runtime. All functions
working with attribute_group provided by <linux/sysfs.h> and 'uncore.h'
work with const attribute_group. So mark the non-const structs as const.

Arvind Yadav (7):
  [PATCH 1/7] perf: x86: intel: uncore: constify attribute_group structures.
  [PATCH 2/7] perf: x86: intel: uncore_nhmex: constify attribute_group structures.
  [PATCH 3/7] perf: x86: intel: uncore_snb: constify attribute_group structures.
  [PATCH 4/7] perf: x86: intel: uncore_snbep: constify attribute_group structures.
  [PATCH 5/7] x86: ksysfs: constify attribute_group structures.
  [PATCH 6/7] x86: therm_throt: constify attribute_group structures.
  [PATCH 7/7] x86: microcode: constify attribute_group structures.

 arch/x86/events/intel/uncore.c           |  2 +-
 arch/x86/events/intel/uncore_nhmex.c     | 12 ++++-----
 arch/x86/events/intel/uncore_snb.c       |  6 ++---
 arch/x86/events/intel/uncore_snbep.c     | 42 ++++++++++++++++----------------
 arch/x86/kernel/cpu/mcheck/therm_throt.c |  2 +-
 arch/x86/kernel/cpu/microcode/core.c     |  4 +--
 arch/x86/kernel/ksysfs.c                 |  4 +--
 7 files changed, 36 insertions(+), 36 deletions(-)

-- 
1.9.1

[toc] | [next] | [standalone]

#1692499 — [PATCH 3/7] perf: x86: intel: uncore_snb: constify attribute_group structures.

attribute_group are not supposed to change at runtime. All functions
working with attribute_group provided by 'uncore.h' work with const
attribute_group. So mark the non-const structs as const.

File size before:
   text	   data	    bss	    dec	    hex	filename
   4158	   4792	      0	   8950	   22f6	arch/x86/events/intel/uncore_snb.o

File size After adding 'const':
   text	   data	    bss	    dec	    hex	filename
   4350	   4600	      0	   8950	   22f6	arch/x86/events/intel/uncore_snb.o

Signed-off-by: Arvind Yadav <arvind.yadav.cs@gmail.com>
---
 arch/x86/events/intel/uncore_snb.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/x86/events/intel/uncore_snb.c b/arch/x86/events/intel/uncore_snb.c
index a3dcc12..db1127c 100644
--- a/arch/x86/events/intel/uncore_snb.c
+++ b/arch/x86/events/intel/uncore_snb.c
@@ -130,7 +130,7 @@ static void snb_uncore_msr_exit_box(struct intel_uncore_box *box)
 	NULL,
 };
 
-static struct attribute_group snb_uncore_format_group = {
+static const struct attribute_group snb_uncore_format_group = {
 	.name		= "format",
 	.attrs		= snb_uncore_formats_attr,
 };
@@ -289,7 +289,7 @@ enum {
 	NULL,
 };
 
-static struct attribute_group snb_uncore_imc_format_group = {
+static const struct attribute_group snb_uncore_imc_format_group = {
 	.name = "format",
 	.attrs = snb_uncore_imc_formats_attr,
 };
@@ -769,7 +769,7 @@ static void nhm_uncore_msr_enable_event(struct intel_uncore_box *box, struct per
 	NULL,
 };
 
-static struct attribute_group nhm_uncore_format_group = {
+static const struct attribute_group nhm_uncore_format_group = {
 	.name = "format",
 	.attrs = nhm_uncore_formats_attr,
 };
-- 
1.9.1

[toc] | [prev] | [next] | [standalone]

#1692500 — [PATCH 7/7] x86: microcode: constify attribute_group structures.

attribute_group are not supposed to change at runtime. All functions
working with attribute_group provided by <linux/sysfs.h> work with
const attribute_group. So mark the non-const structs as const.

File size before:
   text	   data	    bss	    dec	    hex	filename
   3614	    545	   1560	   5719	   1657	x86/kernel/cpu/microcode/core.o

File size After adding 'const':
   text	   data	    bss	    dec	    hex	filename
   3742	    417	   1560	   5719	   1657	x86/kernel/cpu/microcode/core.o

Signed-off-by: Arvind Yadav <arvind.yadav.cs@gmail.com>
---
 arch/x86/kernel/cpu/microcode/core.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kernel/cpu/microcode/core.c b/arch/x86/kernel/cpu/microcode/core.c
index 9cb98ee..86e8f0b 100644
--- a/arch/x86/kernel/cpu/microcode/core.c
+++ b/arch/x86/kernel/cpu/microcode/core.c
@@ -561,7 +561,7 @@ static ssize_t pf_show(struct device *dev,
 	NULL
 };
 
-static struct attribute_group mc_attr_group = {
+static const struct attribute_group mc_attr_group = {
 	.attrs			= mc_default_attrs,
 	.name			= "microcode",
 };
@@ -707,7 +707,7 @@ static int mc_cpu_down_prep(unsigned int cpu)
 	NULL
 };
 
-static struct attribute_group cpu_root_microcode_group = {
+static const struct attribute_group cpu_root_microcode_group = {
 	.name  = "microcode",
 	.attrs = cpu_root_microcode_attrs,
 };
-- 
1.9.1

[toc] | [prev] | [next] | [standalone]

#1692541 — Re: [PATCH 7/7] x86: microcode: constify attribute_group structures.

On Thu, Jul 20, 2017 at 12:51:45PM +0530, Arvind Yadav wrote:
> attribute_group are not supposed to change at runtime. All functions
> working with attribute_group provided by <linux/sysfs.h> work with
> const attribute_group. So mark the non-const structs as const.
> 
> File size before:
>    text	   data	    bss	    dec	    hex	filename
>    3614	    545	   1560	   5719	   1657	x86/kernel/cpu/microcode/core.o
> 
> File size After adding 'const':
>    text	   data	    bss	    dec	    hex	filename
>    3742	    417	   1560	   5719	   1657	x86/kernel/cpu/microcode/core.o

Again, what are those before/after sizes supposed to show?

-- 
Regards/Gruss,
    Boris.

ECO tip #101: Trim your mails when you reply.
--

[toc] | [prev] | [next] | [standalone]

#1692542 — Re: [PATCH 7/7] x86: microcode: constify attribute_group structures.

Hi,


On Thursday 20 July 2017 01:42 PM, Borislav Petkov wrote:
> On Thu, Jul 20, 2017 at 12:51:45PM +0530, Arvind Yadav wrote:
>> attribute_group are not supposed to change at runtime. All functions
>> working with attribute_group provided by <linux/sysfs.h> work with
>> const attribute_group. So mark the non-const structs as const.
>>
>> File size before:
>>     text	   data	    bss	    dec	    hex	filename
>>     3614	    545	   1560	   5719	   1657	x86/kernel/cpu/microcode/core.o
>>
>> File size After adding 'const':
>>     text	   data	    bss	    dec	    hex	filename
>>     3742	    417	   1560	   5719	   1657	x86/kernel/cpu/microcode/core.o
> Again, what are those before/after sizes supposed to show?
It's just show, how much bytes move from data segment to text segment.
~arvind

[toc] | [prev] | [next] | [standalone]

#1692565 — Re: [PATCH 7/7] x86: microcode: constify attribute_group structures.

On Thu, Jul 20, 2017 at 01:48:17PM +0530, Arvind Yadav wrote:
> It's just show, how much bytes move from data segment to text segment.
> ~arvind

     3614	    545	   1560	   5719	   1657	x86/kernel/cpu/microcode/core.o

3614 + 545 = 4159

     3742	    417	   1560	   5719	   1657	x86/kernel/cpu/microcode/core.o

3742 + 417 = 4159

3742 - 3614 = 128

So 128 bytes moved to text.

I fail to see how this is relevant. Your commit message contains not
really relevant info for such a simple change. Rather, it should just be
a one-liner:

"Constify attribute_group structs because they don't change at runtime."

That's it.

-- 
Regards/Gruss,
    Boris.

ECO tip #101: Trim your mails when you reply.
--

[toc] | [prev] | [next] | [standalone]

#1692638 — Re: [PATCH 7/7] x86: microcode: constify attribute_group structures.

Hi,


On Thursday 20 July 2017 01:58 PM, Borislav Petkov wrote:
> On Thu, Jul 20, 2017 at 01:48:17PM +0530, Arvind Yadav wrote:
>> It's just show, how much bytes move from data segment to text segment.
>> ~arvind
>       3614	    545	   1560	   5719	   1657	x86/kernel/cpu/microcode/core.o
>
> 3614 + 545 = 4159
>
>       3742	    417	   1560	   5719	   1657	x86/kernel/cpu/microcode/core.o
>
> 3742 + 417 = 4159
>
> 3742 - 3614 = 128
>
> So 128 bytes moved to text.
Yes,
>
> I fail to see how this is relevant. Your commit message contains not
> really relevant info for such a simple change. Rather, it should just be
> a one-liner:
I have mentioned all this in commit message. I have added below
message in patch. Please check my patch.

"attribute_group are not supposed to change at runtime. All functions
working with attribute_group provided by <linux/sysfs.h> work with
const attribute_group. So mark the non-const structs as const."

> "Constify attribute_group structs because they don't change at runtime."
I can add this as subject. But we should avoid lengthy subject.
Even checkpatch.pl will through warning.
>
> That's it.
>
~arvind

[toc] | [prev] | [next] | [standalone]

#1692780 — Re: [PATCH 7/7] x86: microcode: constify attribute_group structures.

On Thu, Jul 20, 2017 at 02:31:09PM +0530, Arvind Yadav wrote:
> > "Constify attribute_group structs because they don't change at runtime."
> I can add this as subject. But we should avoid lengthy subject.

Are you actually reading what I'm writing to you? I said this should be
the commit message, not the subject.

What I'm saying is, shorten your commit message - no need for the
irrelevant before/after sizes. I'm not the first reviewer to ask WTF do
those things even mean.

-- 
Regards/Gruss,
    Boris.

ECO tip #101: Trim your mails when you reply.
--

[toc] | [prev] | [next] | [standalone]

#1692793 — Re: [PATCH 7/7] x86: microcode: constify attribute_group structures.

Hi,


On Thursday 20 July 2017 04:01 PM, Borislav Petkov wrote:
> On Thu, Jul 20, 2017 at 02:31:09PM +0530, Arvind Yadav wrote:
>>> "Constify attribute_group structs because they don't change at runtime."
>> I can add this as subject. But we should avoid lengthy subject.
> Are you actually reading what I'm writing to you? I said this should be
> the commit message, not the subject.
>
> What I'm saying is, shorten your commit message - no need for the
> irrelevant before/after sizes. I'm not the first reviewer to ask WTF do
> those things even mean.
>
Thanks :) for your suggestion. I will remove before/after size comparison.
and send you back for review.

~arvind

[toc] | [prev] | [next] | [standalone]

#1692501 — [PATCH 6/7] x86: therm_throt: constify attribute_group structures.

attribute_group are not supposed to change at runtime. All functions
working with attribute_group provided by <linux/sysfs.h> work with
const attribute_group. So mark the non-const structs as const.

File size before:
   text	   data	    bss	    dec	    hex	filename
   3586	    864	     32	   4482	   1182	x86/kernel/cpu/mcheck/therm_throt.o

File size After adding 'const':
   text	   data	    bss	    dec	    hex	filename
   3550	    928	     32	   4482	   1182	x86/kernel/cpu/mcheck/therm_throt.o

Signed-off-by: Arvind Yadav <arvind.yadav.cs@gmail.com>
---
 arch/x86/kernel/cpu/mcheck/therm_throt.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/kernel/cpu/mcheck/therm_throt.c b/arch/x86/kernel/cpu/mcheck/therm_throt.c
index d7cc190..f7370ab 100644
--- a/arch/x86/kernel/cpu/mcheck/therm_throt.c
+++ b/arch/x86/kernel/cpu/mcheck/therm_throt.c
@@ -122,7 +122,7 @@ struct thermal_state {
 	NULL
 };
 
-static struct attribute_group thermal_attr_group = {
+static const struct attribute_group thermal_attr_group = {
 	.attrs	= thermal_throttle_attrs,
 	.name	= "thermal_throttle"
 };
-- 
1.9.1

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web