LinuxLists.cc - [2.6 patch] scsi/qlogicpti.c section fixes

[permalink] [raw]

Subject: Re: [2.6 patch] scsi/qlogicpti.c section fixes

On Wed, 2008-01-30 at 22:03 +0200, Adrian Bunk wrote:
> This patch fixes the following section mismatches:
>
> <-- snip -->
>
> ...
> WARNING: drivers/scsi/qlogicpti.o(.devexit.text+0x8): Section mismatch in reference from the function qpti_sbus_remove() to the function .init.text:qpti_chain_del()
> WARNING: drivers/scsi/qlogicpti.o(.devinit.text+0x56c): Section mismatch in reference from the function qpti_sbus_probe() to the function .init.text:qpti_map_regs()
> WARNING: drivers/scsi/qlogicpti.o(.devinit.text+0x580): Section mismatch in reference from the function qpti_sbus_probe() to the function .init.text:qpti_register_irq()
> WARNING: drivers/scsi/qlogicpti.o(.devinit.text+0x594): Section mismatch in reference from the function qpti_sbus_probe() to the function .init.text:qpti_get_scsi_id()
> WARNING: drivers/scsi/qlogicpti.o(.devinit.text+0x5b8): Section mismatch in reference from the function qpti_sbus_probe() to the function .init.text:qpti_map_queues()
> WARNING: drivers/scsi/qlogicpti.o(.devinit.text+0x780): Section mismatch in reference from the function qpti_sbus_probe() to the function .init.text:qpti_chain_add()
> ...

OK, look, this is really getting out of hand.

__init is possibly justifiable with a few hundred k savings on boot.
__devinit and the rest are surely killable on the grounds they provide
little benefit for all the pain they cause.

all __exit seems to do is set us up for unreferenced pointers in
discarded sections, so could we kill that too?

James

2008-01-30 21:23:40

[permalink] [raw]

Subject: Re: [2.6 patch] scsi/qlogicpti.c section fixes

On Wed, Jan 30, 2008 at 03:00:16PM -0600, James Bottomley wrote:
>
> On Wed, 2008-01-30 at 22:03 +0200, Adrian Bunk wrote:
> > This patch fixes the following section mismatches:
> >
> > <-- snip -->
> >
> > ...
> > WARNING: drivers/scsi/qlogicpti.o(.devexit.text+0x8): Section mismatch in reference from the function qpti_sbus_remove() to the function .init.text:qpti_chain_del()
> > WARNING: drivers/scsi/qlogicpti.o(.devinit.text+0x56c): Section mismatch in reference from the function qpti_sbus_probe() to the function .init.text:qpti_map_regs()
> > WARNING: drivers/scsi/qlogicpti.o(.devinit.text+0x580): Section mismatch in reference from the function qpti_sbus_probe() to the function .init.text:qpti_register_irq()
> > WARNING: drivers/scsi/qlogicpti.o(.devinit.text+0x594): Section mismatch in reference from the function qpti_sbus_probe() to the function .init.text:qpti_get_scsi_id()
> > WARNING: drivers/scsi/qlogicpti.o(.devinit.text+0x5b8): Section mismatch in reference from the function qpti_sbus_probe() to the function .init.text:qpti_map_queues()
> > WARNING: drivers/scsi/qlogicpti.o(.devinit.text+0x780): Section mismatch in reference from the function qpti_sbus_probe() to the function .init.text:qpti_chain_add()
> > ...
>
> OK, look, this is really getting out of hand.
We now at last have some infrastructure that does proper consistency
check of the use of annotation.
So it is by no means getting out-of-hands..

To my understanding Adrian just fixed a potential oops.
You have a driver that surely are thought to be hotplugable -
otherwise qpti_sbus_probe() would not have been annotated __devinit.
And had it ever been hotplugged then you had called a function in
a __init section that was long gone.

> __init is possibly justifiable with a few hundred k savings on boot.
> __devinit and the rest are surely killable on the grounds they provide
> little benefit for all the pain they cause.
For the embedded people a few kb here and there is worth it.

> all __exit seems to do is set us up for unreferenced pointers in
> discarded sections, so could we kill that too?
Again - savings when we build-in the drivers.
And without the checks we see 'funny' linker errors on the architectues
that can continue to add the .exit.text in /DISCARD/

Sam

2008-01-30 21:30:15

[permalink] [raw]

Subject: Re: [2.6 patch] scsi/qlogicpti.c section fixes

On Wed, Jan 30, 2008 at 10:20:11PM +0100, Sam Ravnborg wrote:
> On Wed, Jan 30, 2008 at 03:00:16PM -0600, James Bottomley wrote:
>...
> To my understanding Adrian just fixed a potential oops.
> You have a driver that surely are thought to be hotplugable -
> otherwise qpti_sbus_probe() would not have been annotated __devinit.
> And had it ever been hotplugged then you had called a function in
> a __init section that was long gone.
>...

s/had it ever been hotplugged/had it been hotplugged with a kernel
compiled with gcc 3.2 or 3.3/

> Sam

cu
Adrian

--

"Is there not promise of rain?" Ling Tan asked suddenly out
of the darkness. There had been need of rain for many days.
"Only a promise," Lao Er said.
Pearl S. Buck - Dragon Seed

2008-01-30 21:41:58

[permalink] [raw]

Subject: Re: [2.6 patch] scsi/qlogicpti.c section fixes

On Wed, 2008-01-30 at 22:20 +0100, Sam Ravnborg wrote:
> On Wed, Jan 30, 2008 at 03:00:16PM -0600, James Bottomley wrote:
> >
> > On Wed, 2008-01-30 at 22:03 +0200, Adrian Bunk wrote:
> > > This patch fixes the following section mismatches:
> > >
> > > <-- snip -->
> > >
> > > ...
> > > WARNING: drivers/scsi/qlogicpti.o(.devexit.text+0x8): Section mismatch in reference from the function qpti_sbus_remove() to the function .init.text:qpti_chain_del()
> > > WARNING: drivers/scsi/qlogicpti.o(.devinit.text+0x56c): Section mismatch in reference from the function qpti_sbus_probe() to the function .init.text:qpti_map_regs()
> > > WARNING: drivers/scsi/qlogicpti.o(.devinit.text+0x580): Section mismatch in reference from the function qpti_sbus_probe() to the function .init.text:qpti_register_irq()
> > > WARNING: drivers/scsi/qlogicpti.o(.devinit.text+0x594): Section mismatch in reference from the function qpti_sbus_probe() to the function .init.text:qpti_get_scsi_id()
> > > WARNING: drivers/scsi/qlogicpti.o(.devinit.text+0x5b8): Section mismatch in reference from the function qpti_sbus_probe() to the function .init.text:qpti_map_queues()
> > > WARNING: drivers/scsi/qlogicpti.o(.devinit.text+0x780): Section mismatch in reference from the function qpti_sbus_probe() to the function .init.text:qpti_chain_add()
> > > ...
> >
> > OK, look, this is really getting out of hand.
> We now at last have some infrastructure that does proper consistency
> check of the use of annotation.
> So it is by no means getting out-of-hands..
>
> To my understanding Adrian just fixed a potential oops.
> You have a driver that surely are thought to be hotplugable -
> otherwise qpti_sbus_probe() would not have been annotated __devinit.
> And had it ever been hotplugged then you had called a function in
> a __init section that was long gone.

I never said it didn't fix a bug. I'm just reflecting that all of the
problems would go away and we'd save thousands of person hours on the
infrastructure and bug fixing if we simply #defined most of the
sectional annotations to be nops.

My basic question is "is the value of what all of this buys us
(basically a few hundred k in savings)" worth all the investment we're
putting into it. Particularly as it looks like that investment is
becoming increasingly expensive.

> > __init is possibly justifiable with a few hundred k savings on boot.
> > __devinit and the rest are surely killable on the grounds they provide
> > little benefit for all the pain they cause.
> For the embedded people a few kb here and there is worth it.
>
> > all __exit seems to do is set us up for unreferenced pointers in
> > discarded sections, so could we kill that too?
> Again - savings when we build-in the drivers.
> And without the checks we see 'funny' linker errors on the architectues
> that can continue to add the .exit.text in /DISCARD/

Perhaps you have different figures, but my standard kernel linking ones
tell me that the discard sections only save tens of k (not hundreds that
the init ones save), so I really do think they have no real benefit and
land us huge problems of pointer references into discarded sections.

I don't deny we can invest large amounts of work to fix our current
issues and build large scriptable checks to ensure we keep it fixed ...
I'm just asking if, at the end of the day, it's really worth it.

James

2008-01-30 22:00:26

[permalink] [raw]

Subject: Re: [2.6 patch] scsi/qlogicpti.c section fixes

> I'm just reflecting that all of the
> problems would go away and we'd save thousands of person hours on the
> infrastructure and bug fixing if we simply #defined most of the
> sectional annotations to be nops.

So far I have only seen three persons being really active in fixing the
section mismatch bugs. And thats Randy, Adrian and myself.
Granted a few other have done some sporadic efforts in their
area of interest but nothing covering the tree as a whole.
And most isssues are easy fixable - I would estimate less than
half an hour in average for the fixes Adrian posted today.
There are some tricky corners that are dfficult to deal with
but I have not seen these occur in drivers.
It is mostly when we mix assembler and C plus
some of the core functionality.
The hotplug cpu stuff for example look scary and here
I may try to road you propose but for the hotplug stuff
I think it is worth it.

Sam

2008-01-30 22:28:34

[permalink] [raw]

Subject: Re: [2.6 patch] scsi/qlogicpti.c section fixes

On Wed, Jan 30, 2008 at 03:00:16PM -0600, James Bottomley wrote:
>
> On Wed, 2008-01-30 at 22:03 +0200, Adrian Bunk wrote:
> > This patch fixes the following section mismatches:
> >
> > <-- snip -->
> >
> > ...
> > WARNING: drivers/scsi/qlogicpti.o(.devexit.text+0x8): Section mismatch in reference from the function qpti_sbus_remove() to the function .init.text:qpti_chain_del()
> > WARNING: drivers/scsi/qlogicpti.o(.devinit.text+0x56c): Section mismatch in reference from the function qpti_sbus_probe() to the function .init.text:qpti_map_regs()
> > WARNING: drivers/scsi/qlogicpti.o(.devinit.text+0x580): Section mismatch in reference from the function qpti_sbus_probe() to the function .init.text:qpti_register_irq()
> > WARNING: drivers/scsi/qlogicpti.o(.devinit.text+0x594): Section mismatch in reference from the function qpti_sbus_probe() to the function .init.text:qpti_get_scsi_id()
> > WARNING: drivers/scsi/qlogicpti.o(.devinit.text+0x5b8): Section mismatch in reference from the function qpti_sbus_probe() to the function .init.text:qpti_map_queues()
> > WARNING: drivers/scsi/qlogicpti.o(.devinit.text+0x780): Section mismatch in reference from the function qpti_sbus_probe() to the function .init.text:qpti_chain_add()
> > ...
>
> OK, look, this is really getting out of hand.
>
> __init is possibly justifiable with a few hundred k savings on boot.
> __devinit and the rest are surely killable on the grounds they provide
> little benefit for all the pain they cause.
>
> all __exit seems to do is set us up for unreferenced pointers in
> discarded sections, so could we kill that too?

When you are on x86 what you see as "Freeing unused kernel memory: "
at the end of booting contains both __init and __exit code.

> James

cu
Adrian

--

"Is there not promise of rain?" Ling Tan asked suddenly out
of the darkness. There had been need of rain for many days.
"Only a promise," Lao Er said.
Pearl S. Buck - Dragon Seed

2008-01-30 22:31:56

[permalink] [raw]

Subject: Value of __*{init,exit} anotations?

On Wed, Jan 30, 2008 at 03:41:35PM -0600, James Bottomley wrote:
> On Wed, 2008-01-30 at 22:20 +0100, Sam Ravnborg wrote:
> > On Wed, Jan 30, 2008 at 03:00:16PM -0600, James Bottomley wrote:
>...
> > > __init is possibly justifiable with a few hundred k savings on boot.
> > > __devinit and the rest are surely killable on the grounds they provide
> > > little benefit for all the pain they cause.
> > For the embedded people a few kb here and there is worth it.
> >
> > > all __exit seems to do is set us up for unreferenced pointers in
> > > discarded sections, so could we kill that too?
> > Again - savings when we build-in the drivers.
> > And without the checks we see 'funny' linker errors on the architectues
> > that can continue to add the .exit.text in /DISCARD/
>
> Perhaps you have different figures, but my standard kernel linking ones
> tell me that the discard sections only save tens of k (not hundreds that
> the init ones save), so I really do think they have no real benefit and
> land us huge problems of pointer references into discarded sections.
>
> I don't deny we can invest large amounts of work to fix our current
> issues and build large scriptable checks to ensure we keep it fixed ...
> I'm just asking if, at the end of the day, it's really worth it.

Some people consider it worth it for their memory restricted systems
and would like to drive the annotations even further. [1]

My experience while fixing section bugs during the last years is that
the __dev{init,exit}* are actually the main question since they are both
the majority of annotations and the ones that bring benefits only
in a case that has become very exotic (CONFIG_HOTPLUG=n).

All the other annotations either both bring value for everyone
(plain __init* and __exit*) or are nothing normal drivers would
use (__cpu* and _mem*).

People at linux-arch (Cc'ed) might be better at explaining how often
CONFIG_HOTPLUG gets used in real-life systems and how big the savings
are there.

That might be a good basis for deciding whether it's worth it.

> James

cu
Adrian

[1] http://lkml.org/lkml/2007/10/12/297

--

"Is there not promise of rain?" Ling Tan asked suddenly out
of the darkness. There had been need of rain for many days.
"Only a promise," Lao Er said.
Pearl S. Buck - Dragon Seed

2008-01-30 22:44:34

[permalink] [raw]

Subject: Re: Value of __*{init,exit} anotations?

On Thu, 2008-01-31 at 00:32 +0200, Adrian Bunk wrote:
> On Wed, Jan 30, 2008 at 03:41:35PM -0600, James Bottomley wrote:
> > On Wed, 2008-01-30 at 22:20 +0100, Sam Ravnborg wrote:
> > > On Wed, Jan 30, 2008 at 03:00:16PM -0600, James Bottomley wrote:
> >...
> > > > __init is possibly justifiable with a few hundred k savings on boot.
> > > > __devinit and the rest are surely killable on the grounds they provide
> > > > little benefit for all the pain they cause.
> > > For the embedded people a few kb here and there is worth it.
> > >
> > > > all __exit seems to do is set us up for unreferenced pointers in
> > > > discarded sections, so could we kill that too?
> > > Again - savings when we build-in the drivers.
> > > And without the checks we see 'funny' linker errors on the architectues
> > > that can continue to add the .exit.text in /DISCARD/
> >
> > Perhaps you have different figures, but my standard kernel linking ones
> > tell me that the discard sections only save tens of k (not hundreds that
> > the init ones save), so I really do think they have no real benefit and
> > land us huge problems of pointer references into discarded sections.
> >
> > I don't deny we can invest large amounts of work to fix our current
> > issues and build large scriptable checks to ensure we keep it fixed ...
> > I'm just asking if, at the end of the day, it's really worth it.
>
> Some people consider it worth it for their memory restricted systems
> and would like to drive the annotations even further. [1]
>
> My experience while fixing section bugs during the last years is that
> the __dev{init,exit}* are actually the main question since they are both
> the majority of annotations and the ones that bring benefits only
> in a case that has become very exotic (CONFIG_HOTPLUG=n).
>
> All the other annotations either both bring value for everyone
> (plain __init* and __exit*) or are nothing normal drivers would
> use (__cpu* and _mem*).
>
> People at linux-arch (Cc'ed) might be better at explaining how often
> CONFIG_HOTPLUG gets used in real-life systems and how big the savings
> are there.
>
> That might be a good basis for deciding whether it's worth it.

I'll certainly buy this. Perhaps killing everything other than __init
and __exit (meaning discardable whether the system is hotplug, suspend
or whatever) might get rid of 90% of the problem while still preserving
90% of the benefits. I think a lot of the issues do come from confusion
over whether it should be __init, __devinint etc .

We can argue later over the benefit of __exit ...

James

2008-01-30 22:52:01

by Russell King

[permalink] [raw]

Subject: Re: Value of __*{init,exit} anotations?

On Wed, Jan 30, 2008 at 04:44:12PM -0600, James Bottomley wrote:
> On Thu, 2008-01-31 at 00:32 +0200, Adrian Bunk wrote:
> > People at linux-arch (Cc'ed) might be better at explaining how often
> > CONFIG_HOTPLUG gets used in real-life systems and how big the savings
> > are there.
> >
> > That might be a good basis for deciding whether it's worth it.
>
> I'll certainly buy this. Perhaps killing everything other than __init
> and __exit (meaning discardable whether the system is hotplug, suspend
> or whatever) might get rid of 90% of the problem while still preserving
> 90% of the benefits. I think a lot of the issues do come from confusion
> over whether it should be __init, __devinint etc .

Just looking at HOTPLUG on ARM by a simple grep, 57 default configurations
for various machines are hotplug enabled out of 75 - so it's roughly 75%.
Whether that 25% remainder cares or not, I'm not sure.

--
Russell King
Linux kernel 2.6 ARM Linux - http://www.arm.linux.org.uk/
maintainer of:

2008-01-31 05:43:22

by Andi Kleen

[permalink] [raw]

Subject: Re: Value of __*{init,exit} anotations?

> Some people consider it worth it for their memory restricted systems
> and would like to drive the annotations even further. [1]

They could get much better bang-for-the-buck (as in memory saved
for amount of work invested) by tackling some the dynamic memory allocation
pigs.

In general it's a trade off between how much work and patch churn versus
benefit, and some of the annotations really don't look too good on this
scale.

> People at linux-arch (Cc'ed) might be better at explaining how often
> CONFIG_HOTPLUG gets used in real-life systems and how big the savings
> are there.

CONFIG_HOTPLUG is widely used for suspend on multi core systems.

-Andi

2008-01-31 07:44:48

[permalink] [raw]

Subject: Re: Value of __*{init,exit} anotations?

> >
> > I don't deny we can invest large amounts of work to fix our current
> > issues and build large scriptable checks to ensure we keep it fixed ...
> > I'm just asking if, at the end of the day, it's really worth it.
>
> Some people consider it worth it for their memory restricted systems
> and would like to drive the annotations even further. [1]
>
> My experience while fixing section bugs during the last years is that
> the __dev{init,exit}* are actually the main question since they are both
> the majority of annotations and the ones that bring benefits only
> in a case that has become very exotic (CONFIG_HOTPLUG=n).

Some numbers...
I my current tree with an allyesconfig build for x86 64bit I see:
136 Section mismatch warnings in total
99 Section mismatch warnings from drivers/

This is with a few patches of mine applied but none of the recently
posted fixes by Adrian.

I am concentrating on:
drivers/pci/
kernel/
mm/

Will post my patches during the weekend if things goes well.
There are some low hanging fruits in drivers/ but I stay away
from these from now expecting others to fix these.

Sam

2008-01-31 15:57:59

[permalink] [raw]

Subject: [PATCH] kill hotplug init/exit section annotations

No-one seems to see much value in these, and they cause about 90% of our
problems with __init/__exit markers, so simply eliminate them. Rather
than run over the whole tree removing them, this patch #defines them to
be nops.

Signed-off-by: James Bottomley <[email protected]>

---

I'll probably be going after __exit after this one, but it makes sense
to split them up, since the hotplug annotation removal looks
uncontroversial, whereas __exit and discard section removal might
produce more robust debate. I also think doing the hotplug removal
gives us 90% of the benefits and removes 90% of the section mismatch
problems.

James

diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h
index f784d2f..5099021 100644
--- a/include/asm-generic/vmlinux.lds.h
+++ b/include/asm-generic/vmlinux.lds.h
@@ -9,46 +9,11 @@
/* Align . to a 8 byte boundary equals to maximum function alignment. */
#define ALIGN_FUNCTION() . = ALIGN(8)

-/* The actual configuration determine if the init/exit sections
- * are handled as text/data or they can be discarded (which
- * often happens at runtime)
- */
-#ifdef CONFIG_HOTPLUG
-#define DEV_KEEP(sec) *(.dev##sec)
-#define DEV_DISCARD(sec)
-#else
-#define DEV_KEEP(sec)
-#define DEV_DISCARD(sec) *(.dev##sec)
-#endif
-
-#ifdef CONFIG_HOTPLUG_CPU
-#define CPU_KEEP(sec) *(.cpu##sec)
-#define CPU_DISCARD(sec)
-#else
-#define CPU_KEEP(sec)
-#define CPU_DISCARD(sec) *(.cpu##sec)
-#endif
-
-#if defined(CONFIG_MEMORY_HOTPLUG)
-#define MEM_KEEP(sec) *(.mem##sec)
-#define MEM_DISCARD(sec)
-#else
-#define MEM_KEEP(sec)
-#define MEM_DISCARD(sec) *(.mem##sec)
-#endif
-
-
/* .data section */
#define DATA_DATA \
*(.data) \
*(.data.init.refok) \
*(.ref.data) \
- DEV_KEEP(init.data) \
- DEV_KEEP(exit.data) \
- CPU_KEEP(init.data) \
- CPU_KEEP(exit.data) \
- MEM_KEEP(init.data) \
- MEM_KEEP(exit.data) \
. = ALIGN(8); \
VMLINUX_SYMBOL(__start___markers) = .; \
*(__markers) \
@@ -171,12 +136,6 @@
/* __*init sections */ \
__init_rodata : AT(ADDR(__init_rodata) - LOAD_OFFSET) { \
*(.ref.rodata) \
- DEV_KEEP(init.rodata) \
- DEV_KEEP(exit.rodata) \
- CPU_KEEP(init.rodata) \
- CPU_KEEP(exit.rodata) \
- MEM_KEEP(init.rodata) \
- MEM_KEEP(exit.rodata) \
} \
\
/* Built-in module parameters. */ \
@@ -208,12 +167,6 @@
*(.ref.text) \
*(.text.init.refok) \
*(.exit.text.refok) \
- DEV_KEEP(init.text) \
- DEV_KEEP(exit.text) \
- CPU_KEEP(init.text) \
- CPU_KEEP(exit.text) \
- MEM_KEEP(init.text) \
- MEM_KEEP(exit.text)

/* sched.text is aling to function alignment to secure we have same
@@ -241,33 +194,15 @@
/* init and exit section handling */
#define INIT_DATA \
*(.init.data) \
- DEV_DISCARD(init.data) \
- DEV_DISCARD(init.rodata) \
- CPU_DISCARD(init.data) \
- CPU_DISCARD(init.rodata) \
- MEM_DISCARD(init.data) \
- MEM_DISCARD(init.rodata)

#define INIT_TEXT \
*(.init.text) \
- DEV_DISCARD(init.text) \
- CPU_DISCARD(init.text) \
- MEM_DISCARD(init.text)

#define EXIT_DATA \
*(.exit.data) \
- DEV_DISCARD(exit.data) \
- DEV_DISCARD(exit.rodata) \
- CPU_DISCARD(exit.data) \
- CPU_DISCARD(exit.rodata) \
- MEM_DISCARD(exit.data) \
- MEM_DISCARD(exit.rodata)

#define EXIT_TEXT \
*(.exit.text) \
- DEV_DISCARD(exit.text) \
- CPU_DISCARD(exit.text) \
- MEM_DISCARD(exit.text)

/* DWARF debug sections.
Symbols in the DWARF debugging sections are relative to
diff --git a/include/linux/init.h b/include/linux/init.h
index 2efbda0..225bd1c 100644
--- a/include/linux/init.h
+++ b/include/linux/init.h
@@ -81,29 +81,28 @@

#define __exit __section(.exit.text) __exitused __cold

-/* Used for HOTPLUG */
-#define __devinit __section(.devinit.text) __cold
-#define __devinitdata __section(.devinit.data)
-#define __devinitconst __section(.devinit.rodata)
-#define __devexit __section(.devexit.text) __exitused __cold
-#define __devexitdata __section(.devexit.data)
-#define __devexitconst __section(.devexit.rodata)
-
-/* Used for HOTPLUG_CPU */
-#define __cpuinit __section(.cpuinit.text) __cold
-#define __cpuinitdata __section(.cpuinit.data)
-#define __cpuinitconst __section(.cpuinit.rodata)
-#define __cpuexit __section(.cpuexit.text) __exitused __cold
-#define __cpuexitdata __section(.cpuexit.data)
-#define __cpuexitconst __section(.cpuexit.rodata)
+/* Legacy: originally used for HOTPLUG */
+#define __devinit
+#define __devinitdata
+#define __devinitconst
+#define __devexit
+#define __devexitdata
+#define __devexitconst
+
+#define __cpuinit
+#define __cpuinitdata
+#define __cpuinitconst
+#define __cpuexit
+#define __cpuexitdata
+#define __cpuexitconst

/* Used for MEMORY_HOTPLUG */
-#define __meminit __section(.meminit.text) __cold
-#define __meminitdata __section(.meminit.data)
-#define __meminitconst __section(.meminit.rodata)
-#define __memexit __section(.memexit.text) __exitused __cold
-#define __memexitdata __section(.memexit.data)
-#define __memexitconst __section(.memexit.rodata)
+#define __meminit
+#define __meminitdata
+#define __meminitconst
+#define __memexit
+#define __memexitdata
+#define __memexitconst

/* For assembly routines */
#define __INIT .section ".init.text","ax"
@@ -111,14 +110,14 @@

#define __INITDATA .section ".init.data","aw"

-#define __DEVINIT .section ".devinit.text", "ax"
-#define __DEVINITDATA .section ".devinit.data", "aw"
+#define __DEVINIT
+#define __DEVINITDATA

-#define __CPUINIT .section ".cpuinit.text", "ax"
-#define __CPUINITDATA .section ".cpuinit.data", "aw"
+#define __CPUINIT
+#define __CPUINITDATA

-#define __MEMINIT .section ".meminit.text", "ax"
-#define __MEMINITDATA .section ".meminit.data", "aw"
+#define __MEMINIT
+#define __MEMINITDATA

/* silence warnings when references are OK */
#define __REF .section ".ref.text", "ax"

2008-01-31 16:11:55

[permalink] [raw]

Subject: Re: [PATCH] kill hotplug init/exit section annotations

On Thu, 31 Jan 2008 09:57:31 -0600
James Bottomley <[email protected]> wrote:

> No-one seems to see much value in these, and they cause about 90% of
> our problems with __init/__exit markers, so simply eliminate them.
> Rather than run over the whole tree removing them, this patch
> #defines them to be nops.
>
> Signed-off-by: James Bottomley <[email protected]>
>
> ---
>
> I'll probably be going after __exit after this one, but it makes sense
> to split them up, since the hotplug annotation removal looks
> uncontroversial, whereas __exit and discard section removal might
> produce more robust debate. I also think doing the hotplug removal
> gives us 90% of the benefits and removes 90% of the section mismatch
> problems.

Since hotplug is so fundamental nowadays the value no longer outweighs the pain/cost
to me, so

Acked-by: Arjan van de Ven <[email protected]>

--
If you want to reach me at my work email, use [email protected]
For development, discussion and tips for power savings,
visit http://www.lesswatts.org

2008-01-31 16:21:22

[permalink] [raw]

Subject: Re: [PATCH] kill hotplug init/exit section annotations

On Thu, Jan 31, 2008 at 08:11:20AM -0800, Arjan van de Ven wrote:
> On Thu, 31 Jan 2008 09:57:31 -0600
> James Bottomley <[email protected]> wrote:
>
> > No-one seems to see much value in these, and they cause about 90% of
> > our problems with __init/__exit markers, so simply eliminate them.
> > Rather than run over the whole tree removing them, this patch
> > #defines them to be nops.
> >
> > Signed-off-by: James Bottomley <[email protected]>
> >
> > ---
> >
> > I'll probably be going after __exit after this one, but it makes sense
> > to split them up, since the hotplug annotation removal looks
> > uncontroversial, whereas __exit and discard section removal might
> > produce more robust debate. I also think doing the hotplug removal
> > gives us 90% of the benefits and removes 90% of the section mismatch
> > problems.
>
>
> Since hotplug is so fundamental nowadays the value no longer outweighs the pain/cost
> to me, so

Granted for normal hotplug.

But my computer has neither CPU hotplug not memory hotplug, and I don't
see the point for removing these annotations (and they are anyway not
what causes problems in normal drivers).

> Acked-by: Arjan van de Ven <[email protected]>

cu
Adrian

--

"Is there not promise of rain?" Ling Tan asked suddenly out
of the darkness. There had been need of rain for many days.
"Only a promise," Lao Er said.
Pearl S. Buck - Dragon Seed

2008-01-31 17:08:26

[permalink] [raw]

Subject: Re: [PATCH] kill hotplug init/exit section annotations

On Thu, 31 Jan 2008 18:21:42 +0200
Adrian Bunk <[email protected]> wrote:

> On Thu, Jan 31, 2008 at 08:11:20AM -0800, Arjan van de Ven wrote:
> > On Thu, 31 Jan 2008 09:57:31 -0600
> > James Bottomley <[email protected]> wrote:
> >
> > > No-one seems to see much value in these, and they cause about 90%
> > > of our problems with __init/__exit markers, so simply eliminate
> > > them. Rather than run over the whole tree removing them, this
> > > patch #defines them to be nops.
> > >
> > > Signed-off-by: James Bottomley
> > > <[email protected]>
> > >
> > > ---
> > >
> > > I'll probably be going after __exit after this one, but it makes
> > > sense to split them up, since the hotplug annotation removal looks
> > > uncontroversial, whereas __exit and discard section removal might
> > > produce more robust debate. I also think doing the hotplug
> > > removal gives us 90% of the benefits and removes 90% of the
> > > section mismatch problems.
> >
> >
> > Since hotplug is so fundamental nowadays the value no longer
> > outweighs the pain/cost to me, so
>
> Granted for normal hotplug.
>
> But my computer has neither CPU hotplug

cpuhotplug is required for suspend/resume.

--
If you want to reach me at my work email, use [email protected]
For development, discussion and tips for power savings,
visit http://www.lesswatts.org

2008-01-31 17:14:20

[permalink] [raw]

Subject: Re: [PATCH] kill hotplug init/exit section annotations

On Thu, Jan 31, 2008 at 09:07:49AM -0800, Arjan van de Ven wrote:
> On Thu, 31 Jan 2008 18:21:42 +0200
> Adrian Bunk <[email protected]> wrote:
>
> > On Thu, Jan 31, 2008 at 08:11:20AM -0800, Arjan van de Ven wrote:
> > > On Thu, 31 Jan 2008 09:57:31 -0600
> > > James Bottomley <[email protected]> wrote:
> > >
> > > > No-one seems to see much value in these, and they cause about 90%
> > > > of our problems with __init/__exit markers, so simply eliminate
> > > > them. Rather than run over the whole tree removing them, this
> > > > patch #defines them to be nops.
> > > >
> > > > Signed-off-by: James Bottomley
> > > > <[email protected]>
> > > >
> > > > ---
> > > >
> > > > I'll probably be going after __exit after this one, but it makes
> > > > sense to split them up, since the hotplug annotation removal looks
> > > > uncontroversial, whereas __exit and discard section removal might
> > > > produce more robust debate. I also think doing the hotplug
> > > > removal gives us 90% of the benefits and removes 90% of the
> > > > section mismatch problems.
> > >
> > >
> > > Since hotplug is so fundamental nowadays the value no longer
> > > outweighs the pain/cost to me, so
> >
> > Granted for normal hotplug.
> >
> > But my computer has neither CPU hotplug
>
> cpuhotplug is required for suspend/resume.

Not on UP computers.

cu
Adrian

--

"Is there not promise of rain?" Ling Tan asked suddenly out
of the darkness. There had been need of rain for many days.
"Only a promise," Lao Er said.
Pearl S. Buck - Dragon Seed

2008-01-31 17:45:43

by Chris Wedgwood

[permalink] [raw]

Subject: Re: [PATCH] kill hotplug init/exit section annotations

On Thu, Jan 31, 2008 at 07:14:36PM +0200, Adrian Bunk wrote:

> > cpuhotplug is required for suspend/resume.
>
> Not on UP computers.

those are less and less common now, most modern laptops are dual core

2008-01-31 17:48:37

[permalink] [raw]

Subject: Re: [PATCH] kill hotplug init/exit section annotations

On Thu, 31 Jan 2008 19:14:36 +0200
Adrian Bunk <[email protected]> wrote:
> > cpuhotplug is required for suspend/resume.
>
> Not on UP computers.
>

great! someone who still has one of those and uses a kernel without it.
Can you look at your system.map and see how many kilobytes you've gained?
Eg how many kilobytes are in these sections exactly?
(but also subtract any cost due to page aligning this stuff ;-)

> cu
> Adrian
>

--
If you want to reach me at my work email, use [email protected]
For development, discussion and tips for power savings,
visit http://www.lesswatts.org

2008-01-31 17:55:35

[permalink] [raw]

Subject: Re: [PATCH] kill hotplug init/exit section annotations

On Thu, Jan 31, 2008 at 09:45:26AM -0800, Chris Wedgwood wrote:
> On Thu, Jan 31, 2008 at 07:14:36PM +0200, Adrian Bunk wrote:
>
> > > cpuhotplug is required for suspend/resume.
> >
> > Not on UP computers.
>
> those are less and less common now, most modern laptops are dual core

Who was talking about laptops?

cu
Adrian

--

"Is there not promise of rain?" Ling Tan asked suddenly out
of the darkness. There had been need of rain for many days.
"Only a promise," Lao Er said.
Pearl S. Buck - Dragon Seed

2008-01-31 18:34:36

[permalink] [raw]

Subject: Re: [PATCH] kill hotplug init/exit section annotations

On Thu, Jan 31, 2008 at 09:48:01AM -0800, Arjan van de Ven wrote:
> On Thu, 31 Jan 2008 19:14:36 +0200
> Adrian Bunk <[email protected]> wrote:
> > > cpuhotplug is required for suspend/resume.
> >
> > Not on UP computers.
> >
>
> great! someone who still has one of those and uses a kernel without it.
> Can you look at your system.map and see how many kilobytes you've gained?
> Eg how many kilobytes are in these sections exactly?
I have one. A Atmel AT91 board equipped with an 9263.
So lets take a look at the defconfig build for the evaluation board.

o-arm/vmlinux.o: file format elf32-littlearm

0 .text 001cdefc 00000000 00000000 00000400 2**10
2 .init.text 000165e8 00000000 00000000 001ce6c0 2**5
26 .init.data 000032ec 00000000 00000000 002578cc 2**2

---

4 .devinit.text 00001558 00000000 00000000 001e5270 2**2
9 .exit.text 00000bc8 00000000 00000000 001e8d2c 2**2
10 .cpuinit.text 00000924 00000000 00000000 001e98f4 2**2
11 .meminit.text 000004cc 00000000 00000000 001ea218 2**2
12 .devexit.text 00000160 00000000 00000000 001ea6e4 2**2
38 .cpuinit.data 00000040 00000000 00000000 0025afc0 2**2
39 .meminit.data 0000000c 00000000 00000000 0025b000 2**2

__devinit alone gives a net win of 5464 bytes.
That is only ~3% of total .text size but this is non-swapable
memory where everything is worth it.
And the configuration selected is by no means optimal
with respect to minimal size.

Sam

2008-01-31 18:39:35

by Chris Wedgwood

[permalink] [raw]

Subject: Re: [PATCH] kill hotplug init/exit section annotations

On Thu, Jan 31, 2008 at 07:55:43PM +0200, Adrian Bunk wrote:

> Who was talking about laptops?

If laptops are mostly MP these days, then 'desktops' and 'servers'
certainly are --- so pretty much everyone needs CPU hotplug.

2008-01-31 18:48:50

[permalink] [raw]

Subject: Re: [PATCH] kill hotplug init/exit section annotations

On Thu, 31 Jan 2008 19:34:25 +0100
Sam Ravnborg <[email protected]> wrote:

> On Thu, Jan 31, 2008 at 09:48:01AM -0800, Arjan van de Ven wrote:
> > On Thu, 31 Jan 2008 19:14:36 +0200
> > Adrian Bunk <[email protected]> wrote:
> > > > cpuhotplug is required for suspend/resume.
> > >
> > > Not on UP computers.
> > >
> >
> > great! someone who still has one of those and uses a kernel without
> > it. Can you look at your system.map and see how many kilobytes
> > you've gained? Eg how many kilobytes are in these sections exactly?
> I have one. A Atmel AT91 board equipped with an 9263.
> So lets take a look at the defconfig build for the evaluation board.
>
>
> o-arm/vmlinux.o: file format elf32-littlearm
>
> 0 .text 001cdefc 00000000 00000000 00000400 2**10
> 2 .init.text 000165e8 00000000 00000000 001ce6c0 2**5
> 26 .init.data 000032ec 00000000 00000000 002578cc 2**2
>
> ---
>
> 4 .devinit.text 00001558 00000000 00000000 001e5270 2**2
> 9 .exit.text 00000bc8 00000000 00000000 001e8d2c 2**2
> 10 .cpuinit.text 00000924 00000000 00000000 001e98f4 2**2
> 11 .meminit.text 000004cc 00000000 00000000 001ea218 2**2
> 12 .devexit.text 00000160 00000000 00000000 001ea6e4 2**2
> 38 .cpuinit.data 00000040 00000000 00000000 0025afc0 2**2
> 39 .meminit.data 0000000c 00000000 00000000 0025b000 2**2
>
>
> __devinit alone gives a net win of 5464 bytes.
> That is only ~3% of total .text size but this is non-swapable
> memory where everything is worth it.

now how much of this is lost again because you have to round the stuff to pagesize?

2008-01-31 18:53:14

by Geert Uytterhoeven

[permalink] [raw]

Subject: Re: [PATCH] kill hotplug init/exit section annotations

On Thu, 31 Jan 2008, Chris Wedgwood wrote:
> On Thu, Jan 31, 2008 at 07:55:43PM +0200, Adrian Bunk wrote:
> > Who was talking about laptops?
>
> If laptops are mostly MP these days, then 'desktops' and 'servers'
> certainly are --- so pretty much everyone needs CPU hotplug.

<irony>
Thank you for giving an exhaustive list of classes of machines Linux runs on!
</irony>

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

2008-01-31 19:41:25