Re: [PATCH 407/493] infiniband: remove use of __devexit

Re: [PATCH 407/493] infiniband: remove use of __devexit

[Jason Gunthorpe]

On Mon, Nov 19, 2012 at 12:19:38PM -0800, Greg KH wrote:
> On Mon, Nov 19, 2012 at 01:09:21PM -0700, Jason Gunthorpe wrote:
> > On Mon, Nov 19, 2012 at 01:25:56PM -0500, Bill Pemberton wrote:
> > > CONFIG_HOTPLUG is going away as an option so __devexit is no
> > > longer needed.
> > 
> > I'm sad to hear this, it is an easy space saver on my non-modular
> > emebedded systems :(
> 
> Really?  I asked for details, and it was reported that this only saved
> 1-200 bytes or so.  See the lkml archives for the details of this.

I just checked for you:

Old 2.6.16 powerpc 32 kernel:

   text    data     bss     dec     hex filename
2399368  222224  156124 2777716  2a6274 build/vmlinux
2394634  221804  156124 2772562  2a4e52 build-nhp/vmlinux

That looks like around 5154 bytes to me

New 3.6 powerpc 32 kernel:

   text    data     bss     dec     hex filename
3352356  162812  218132 3733300  38f734 build/vmlinux
3347644  162648  217860 3728152  38e318 build-nhp/vmlinux

And that is about 5148 bytes.

In both cases the only difference is adding CONFIG_HOTPLUG=y to the
config.

5k isn't a lot, but in the context of 'I have to figure out how to
trim ~1MB off the 3.6 kernel to run it in our smallest hardware' it is
the wrong direction :(

Regards,
Jason

Re: [PATCH 407/493] infiniband: remove use of __devexit

[Greg KH]

On Mon, Nov 19, 2012 at 02:49:22PM -0700, Jason Gunthorpe wrote:
> On Mon, Nov 19, 2012 at 12:19:38PM -0800, Greg KH wrote:
> > On Mon, Nov 19, 2012 at 01:09:21PM -0700, Jason Gunthorpe wrote:
> > > On Mon, Nov 19, 2012 at 01:25:56PM -0500, Bill Pemberton wrote:
> > > > CONFIG_HOTPLUG is going away as an option so __devexit is no
> > > > longer needed.
> > > 
> > > I'm sad to hear this, it is an easy space saver on my non-modular
> > > emebedded systems :(
> > 
> > Really?  I asked for details, and it was reported that this only saved
> > 1-200 bytes or so.  See the lkml archives for the details of this.
> 
> I just checked for you:
> 
> Old 2.6.16 powerpc 32 kernel:
> 
>    text    data     bss     dec     hex filename
> 2399368  222224  156124 2777716  2a6274 build/vmlinux
> 2394634  221804  156124 2772562  2a4e52 build-nhp/vmlinux
> 
> That looks like around 5154 bytes to me
> 
> New 3.6 powerpc 32 kernel:
> 
>    text    data     bss     dec     hex filename
> 3352356  162812  218132 3733300  38f734 build/vmlinux
> 3347644  162648  217860 3728152  38e318 build-nhp/vmlinux
> 
> And that is about 5148 bytes.
> 
> In both cases the only difference is adding CONFIG_HOTPLUG=y to the
> config.
> 
> 5k isn't a lot, but in the context of 'I have to figure out how to
> trim ~1MB off the 3.6 kernel to run it in our smallest hardware' it is
> the wrong direction :(

It is only 0.138% in the "wrong" direction.  Seriously, that's a very
tiny percentage here, for an option that people _always_ get wrong, and
almost no system does not need.  The number of bugs we have had in this
area is huge, and by fixing this option like this, it takes them all
away.

Yes, there are going to be exceptions, like yours, that somehow do run
properly without CONFIG_HOTPLUG enabled.  But really, are you worried
about 0.138% right now?  The amount of time we just spent writing these
emails, memory sizes increased this much for your next platform, for the
same cost as your last platform.

Also, as you point out above, it's a constant number that this option is
affecting, which is good for you, because as time goes on, it's a
smaller and smaller percentage of the overall space used by your kernel.

Thanks for running the numbers, I appreciate it.  I'm amazed that your
kernel growth from 2.6.16 to 3.6 is 1Mb.  Why would you want to run the
3.6 kernel in a system designed for the 2.6.16 kernel (i.e. one designed
over 6 _years_ ago)?

A lot happens in 6 years :)

thanks,

greg k-h

Re: [PATCH 407/493] infiniband: remove use of __devexit

[Jason Gunthorpe]

On Mon, Nov 19, 2012 at 02:06:32PM -0800, Greg KH wrote:

> > 5k isn't a lot, but in the context of 'I have to figure out how to
> > trim ~1MB off the 3.6 kernel to run it in our smallest hardware' it is
> > the wrong direction :(
> 
> It is only 0.138% in the "wrong" direction.  Seriously, that's a very
> tiny percentage here, for an option that people _always_ get wrong, and
> almost no system does not need.  The number of bugs we have had in this
> area is huge, and by fixing this option like this, it takes them all
> away.

Sure, it isn't a lot. I don't have any idea about problems
CONFIG_HOTPLUG causes, but it seems alot of work has gone into making
this option over the years (the patch to remove it is huge), is there
no other way to get what you want?

> Yes, there are going to be exceptions, like yours, that somehow do run
> properly without CONFIG_HOTPLUG enabled.  But really, are you worried
> about 0.138% right now?  The amount of time we just spent writing these
> emails, memory sizes increased this much for your next platform, for the
> same cost as your last platform.

Well, I'm not worried looking forward, as you say, it just doesn't
matter. The systems we designed today have 128M and that was pretty
much the *smallest* dram we could provision.

> Thanks for running the numbers, I appreciate it.  I'm amazed that your
> kernel growth from 2.6.16 to 3.6 is 1Mb.  Why would you want to run the
> 3.6 kernel in a system designed for the 2.6.16 kernel (i.e. one designed
> over 6 _years_ ago)?

I haven't had any time to inspect the 1MB growth, so I can't comment
on what is going on there.. A chunk of that will be some initrd
growth too..

As for why the interest in upgrading? Well, we don't make consumer
products, our stuff has long life cycles and the 6 year old system
continues to be manufactured and sold today.

Our latest platform is ARM based, and we've had to re-spin and re-test
*everything*. The work to bundle PPC along in that effort is fairly
minor, so we are looking at keeping it up-to-date with our unified
firmware.

Regards,
Jason

Re: [PATCH 407/493] infiniband: remove use of __devexit

[Greg KH]

On Mon, Nov 19, 2012 at 03:48:45PM -0700, Jason Gunthorpe wrote:
> On Mon, Nov 19, 2012 at 02:06:32PM -0800, Greg KH wrote:
> 
> > > 5k isn't a lot, but in the context of 'I have to figure out how to
> > > trim ~1MB off the 3.6 kernel to run it in our smallest hardware' it is
> > > the wrong direction :(
> > 
> > It is only 0.138% in the "wrong" direction.  Seriously, that's a very
> > tiny percentage here, for an option that people _always_ get wrong, and
> > almost no system does not need.  The number of bugs we have had in this
> > area is huge, and by fixing this option like this, it takes them all
> > away.
> 
> Sure, it isn't a lot. I don't have any idea about problems
> CONFIG_HOTPLUG causes, but it seems alot of work has gone into making
> this option over the years (the patch to remove it is huge), is there
> no other way to get what you want?

The patch overall, is only 7kb or so.  It's the fact that it is just
removing the __dev* markings all over the tree that makes it so "big".
The big change is just in a very few places in the kernel core that
actually do something different depending on CONFIG_HOTPLUG or not.

I could just leave things alone, with CONFIG_HOTPLUG always enabled, but
then people will continue to blindly use the __dev* markings, getting it
wrong at times, but never realizing that they don't do anything anymore.

Because they don't do anything anymore, the right thing to do is just
remove them, which is what Bill's patches do.

Hope this helps explain things better.

thanks,

greg k-h

Re: [PATCH 407/493] infiniband: remove use of __devexit

[Jason Gunthorpe]

On Mon, Nov 19, 2012 at 03:00:06PM -0800, Greg KH wrote:

> I could just leave things alone, with CONFIG_HOTPLUG always enabled, but
> then people will continue to blindly use the __dev* markings, getting it
> wrong at times, but never realizing that they don't do anything anymore.

Well, I was thinking more along the lines of leaving the capability to
remove the __dev* marked functions from the link, as is today,
independent of the other stuff CONFIG_HOTPLUG controls. But reviewing
your comments from the archive makes me think a motivation is misuse
of the __dev* markings..

Thanks for the insight,
Jason