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


Groups > linux.kernel > #1679659 > unrolled thread

[PATCH] zram: constify attribute_group structures.

Started byArvind Yadav <arvind.yadav.cs@gmail.com>
First post2017-07-03 08:20 +0200
Last post2017-07-03 14:10 +0200
Articles 6 — 3 participants

Back to article view | Back to linux.kernel


Contents

  [PATCH] zram: constify attribute_group structures. Arvind Yadav <arvind.yadav.cs@gmail.com> - 2017-07-03 08:20 +0200
    Re: [PATCH] zram: constify attribute_group structures. Minchan Kim <minchan@kernel.org> - 2017-07-03 08:30 +0200
      Re: [PATCH] zram: constify attribute_group structures. Arvind Yadav <arvind.yadav.cs@gmail.com> - 2017-07-03 09:20 +0200
    Re: [PATCH] zram: constify attribute_group structures. Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com> - 2017-07-03 11:10 +0200
      Re: [PATCH] zram: constify attribute_group structures. Arvind Yadav <arvind.yadav.cs@gmail.com> - 2017-07-03 12:40 +0200
        Re: [PATCH] zram: constify attribute_group structures. Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com> - 2017-07-03 14:10 +0200

#1679659 — [PATCH] zram: constify attribute_group structures.

FromArvind Yadav <arvind.yadav.cs@gmail.com>
Date2017-07-03 08:20 +0200
Subject[PATCH] zram: constify attribute_group structures.
Message-ID<tZ2Sf-5BT-11@gated-at.bofh.it>
attribute_groups are not supposed to change at runtime. All functions
working with attribute_groups 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
   8293	    841	      4	   9138	   23b2	drivers/block/zram/zram_drv.o

File size After adding 'const':
   text	   data	    bss	    dec	    hex	filename
   8357	    777	      4	   9138	   23b2	drivers/block/zram/zram_drv.o

Signed-off-by: Arvind Yadav <arvind.yadav.cs@gmail.com>
---
 drivers/block/zram/zram_drv.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c
index debee95..5da5c04 100644
--- a/drivers/block/zram/zram_drv.c
+++ b/drivers/block/zram/zram_drv.c
@@ -1122,7 +1122,7 @@ static int zram_open(struct block_device *bdev, fmode_t mode)
 	NULL,
 };
 
-static struct attribute_group zram_disk_attr_group = {
+static const struct attribute_group zram_disk_attr_group = {
 	.attrs = zram_disk_attrs,
 };
 
-- 
1.9.1

[toc] | [next] | [standalone]


#1679660

FromMinchan Kim <minchan@kernel.org>
Date2017-07-03 08:30 +0200
Message-ID<tZ31T-5Fr-1@gated-at.bofh.it>
In reply to#1679659
Hello,

On Mon, Jul 03, 2017 at 11:43:13AM +0530, Arvind Yadav wrote:
> attribute_groups are not supposed to change at runtime. All functions
> working with attribute_groups provided by <linux/sysfs.h> work with const
> attribute_group. So mark the non-const structs as const.

If so, how about changing all of places where not have used const
as well as zram?

Anyway, I'm okay with this.

> 
> File size before:
>    text	   data	    bss	    dec	    hex	filename
>    8293	    841	      4	   9138	   23b2	drivers/block/zram/zram_drv.o
> 
> File size After adding 'const':
>    text	   data	    bss	    dec	    hex	filename
>    8357	    777	      4	   9138	   23b2	drivers/block/zram/zram_drv.o
> 
> Signed-off-by: Arvind Yadav <arvind.yadav.cs@gmail.com>
Acked-by: Minchan Kim <minchan@kernel.org>

Thanks.

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


#1679693

FromArvind Yadav <arvind.yadav.cs@gmail.com>
Date2017-07-03 09:20 +0200
Message-ID<tZ3Oi-6fi-17@gated-at.bofh.it>
In reply to#1679660
Hi,


On Monday 03 July 2017 11:57 AM, Minchan Kim wrote:
> Hello,
>
> On Mon, Jul 03, 2017 at 11:43:13AM +0530, Arvind Yadav wrote:
>> attribute_groups are not supposed to change at runtime. All functions
>> working with attribute_groups provided by <linux/sysfs.h> work with const
>> attribute_group. So mark the non-const structs as const.
> If so, how about changing all of places where not have used const
> as well as zram?
      Sure, I am going to change other place also.
>
> Anyway, I'm okay with this.
>
>> File size before:
>>     text	   data	    bss	    dec	    hex	filename
>>     8293	    841	      4	   9138	   23b2	drivers/block/zram/zram_drv.o
>>
>> File size After adding 'const':
>>     text	   data	    bss	    dec	    hex	filename
>>     8357	    777	      4	   9138	   23b2	drivers/block/zram/zram_drv.o
>>
>> Signed-off-by: Arvind Yadav <arvind.yadav.cs@gmail.com>
> Acked-by: Minchan Kim <minchan@kernel.org>
>
> Thanks.
Thanks,
~arvind

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


#1679797

FromSergey Senozhatsky <sergey.senozhatsky.work@gmail.com>
Date2017-07-03 11:10 +0200
Message-ID<tZ5wL-7r5-31@gated-at.bofh.it>
In reply to#1679659
On (07/03/17 11:43), Arvind Yadav wrote:
> attribute_groups are not supposed to change at runtime. All functions
> working with attribute_groups 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
>    8293	    841	      4	   9138	   23b2	drivers/block/zram/zram_drv.o
> 
> File size After adding 'const':
>    text	   data	    bss	    dec	    hex	filename
>    8357	    777	      4	   9138	   23b2	drivers/block/zram/zram_drv.o

sorry, what exactly is the improvement here?
we grew .text bigger, why should we be happy about it?

	-ss

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


#1679874

FromArvind Yadav <arvind.yadav.cs@gmail.com>
Date2017-07-03 12:40 +0200
Message-ID<tZ6VS-5S-25@gated-at.bofh.it>
In reply to#1679797
Hi,


On Monday 03 July 2017 02:30 PM, Sergey Senozhatsky wrote:
> On (07/03/17 11:43), Arvind Yadav wrote:
>> attribute_groups are not supposed to change at runtime. All functions
>> working with attribute_groups 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
>>     8293	    841	      4	   9138	   23b2	drivers/block/zram/zram_drv.o
>>
>> File size After adding 'const':
>>     text	   data	    bss	    dec	    hex	filename
>>     8357	    777	      4	   9138	   23b2	drivers/block/zram/zram_drv.o
> sorry, what exactly is the improvement here?
> we grew .text bigger, why should we be happy about it?
Yes, .text size is increased but .data size got decreased.
Overall size will be same. the text segment is often read-only,
to prevent a program from accidentally modifying its instructions.
Here, attribute_group should be const.
> 	-ss
Thanks,
~arvind

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


#1679942

FromSergey Senozhatsky <sergey.senozhatsky.work@gmail.com>
Date2017-07-03 14:10 +0200
Message-ID<tZ8kW-1aZ-3@gated-at.bofh.it>
In reply to#1679874
On (07/03/17 16:00), Arvind Yadav wrote:
> On Monday 03 July 2017 02:30 PM, Sergey Senozhatsky wrote:
> > On (07/03/17 11:43), Arvind Yadav wrote:
> > > attribute_groups are not supposed to change at runtime. All functions
> > > working with attribute_groups 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
> > >     8293	    841	      4	   9138	   23b2	drivers/block/zram/zram_drv.o
> > > 
> > > File size After adding 'const':
> > >     text	   data	    bss	    dec	    hex	filename
> > >     8357	    777	      4	   9138	   23b2	drivers/block/zram/zram_drv.o
> > sorry, what exactly is the improvement here?
> > we grew .text bigger, why should we be happy about it?
> Yes, .text size is increased but .data size got decreased.
> Overall size will be same. the text segment is often read-only,
> to prevent a program from accidentally modifying its instructions.

well, to the best of my knowledge, nothing in C standard guarantees
that a variable with a C keyword 'const' will be placed into .text
or .rodata. not even sure if all gcc versions have the same opinion
on this.

but ok, we can move that struct to one of the RO sections (up to the
compiler to decide).

	-ss

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web