[Devel] Re: [PATCH 1/7] cgroups: Shrink struct cgroup_subsys
On Sun, Nov 7, 2010 at 9:23 PM, Li Zefan l...@cn.fujitsu.com wrote: bool active; bool disabled; ... ? With alignment 5-8 bool values == 8 bytes in 64-bit machine, compared to 4 bytes with the approach this patch takes. I meant specifying it as: bool active:1; bool disabled:1; i.e. keeping the bit-sized flags but also keeping the bool semantics. Having said that, I'm not really sure why saving 12 bytes per subsystem is worth a patch. Paul ___ Containers mailing list contain...@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/containers ___ Devel mailing list Devel@openvz.org https://openvz.org/mailman/listinfo/devel
[Devel] Re: [PATCH 1/7] cgroups: Shrink struct cgroup_subsys
Paul Menage wrote: On Sun, Nov 7, 2010 at 9:23 PM, Li Zefan l...@cn.fujitsu.com wrote: bool active; bool disabled; ... ? With alignment 5-8 bool values == 8 bytes in 64-bit machine, compared to 4 bytes with the approach this patch takes. I meant specifying it as: bool active:1; bool disabled:1; It won't compile, but unsigned char active:1 will do. ;) i.e. keeping the bit-sized flags but also keeping the bool semantics. Having said that, I'm not really sure why saving 12 bytes per subsystem is worth a patch. Every thing that reduces code size (without sacrifice readability and maintain maintainability) should be worth. This is one of the reasons we accept patches that replacing kmalloc+memset with kzalloc, which just saves 8 bytes in my box. ___ Containers mailing list contain...@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/containers ___ Devel mailing list Devel@openvz.org https://openvz.org/mailman/listinfo/devel
[Devel] Re: [PATCH 1/7] cgroups: Shrink struct cgroup_subsys
On Tue, Nov 9, 2010 at 4:52 PM, Li Zefan l...@cn.fujitsu.com wrote:
bool active:1;
bool disabled:1;
It won't compile, but unsigned char active:1 will do. ;)
Are you sure? I don't have a buildable kernel tree at the moment, but
the following fragment compiled fine for me (with gcc 4.4.3):
struct foo {
_Bool b1:1;
_Bool b2:1;
};
and was sized at one byte. And bool is just a typedef of _Bool in
the kernel headers.
Every thing that reduces code size (without sacrifice readability
and maintain maintainability) should be worth.
Agreed, within reason. But this patch doesn't reduce code size - it
makes the code fractionally more complicated and reduces the *binary*
size by a few bytes.
This is one of the reasons we accept patches that replacing
kmalloc+memset with kzalloc, which just saves 8 bytes in my box.
Replacing two function calls with one function call is a code
simplification and hence (generally) a good thing - the minuscule
reduction in binary size reduction that comes with it is just noise.
Paul
___
Containers mailing list
contain...@lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/containers
___
Devel mailing list
Devel@openvz.org
https://openvz.org/mailman/listinfo/devel
[Devel] Re: [PATCH 1/7] cgroups: Shrink struct cgroup_subsys
Paul Menage wrote:
On Tue, Nov 9, 2010 at 4:52 PM, Li Zefan l...@cn.fujitsu.com wrote:
bool active:1;
bool disabled:1;
It won't compile, but unsigned char active:1 will do. ;)
Are you sure? I don't have a buildable kernel tree at the moment, but
the following fragment compiled fine for me (with gcc 4.4.3):
struct foo {
_Bool b1:1;
_Bool b2:1;
};
and was sized at one byte. And bool is just a typedef of _Bool in
the kernel headers.
Oops, I just used bool outside kernel tree..
Every thing that reduces code size (without sacrifice readability
and maintain maintainability) should be worth.
Agreed, within reason. But this patch doesn't reduce code size - it
I meant binary size.
makes the code fractionally more complicated and reduces the *binary*
size by a few bytes.
It's a commonly used skill in kernel code, so I can't say it makes
code more complicated.
That said, I'll happily drop this patch. It just came to me when I
started to add new bool values to the structure. Or if you prefer
bool xxx:1 or just bool xxx, I can do that.
This is one of the reasons we accept patches that replacing
kmalloc+memset with kzalloc, which just saves 8 bytes in my box.
Replacing two function calls with one function call is a code
simplification and hence (generally) a good thing - the minuscule
reduction in binary size reduction that comes with it is just noise.
___
Containers mailing list
contain...@lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/containers
___
Devel mailing list
Devel@openvz.org
https://openvz.org/mailman/listinfo/devel
[Devel] Re: [PATCH 1/7] cgroups: Shrink struct cgroup_subsys
On Tue, Nov 9, 2010 at 6:06 PM, Li Zefan l...@cn.fujitsu.com wrote: That said, I'll happily drop this patch. It just came to me when I started to add new bool values to the structure. Or if you prefer bool xxx:1 or just bool xxx, I can do that. bool xxx:1 is fine with me - I think it's worth keeping the semantics of these being boolean flags as obvious as possible. I just wouldn't invest much effort in similar patches in the future (given that there are only likely to be, what, 20 instances of cgroup_subsys in the kernel?). Shrinking a structure that had potentially very many instances, such as css_set or cg_cgroup_link, would be different of course. Paul ___ Containers mailing list contain...@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/containers ___ Devel mailing list Devel@openvz.org https://openvz.org/mailman/listinfo/devel
