2012-11-29 18:45:15

by Greg Kroah-Hartman

[permalink] [raw]
Subject: [PATCH 0/2] Remove __dev* sections from the kernel

With the recent work to remove CONFIG_HOTPLUG, we are starting to get a
bunch of __devinit section warnings, despite CONFIG_HOTPLUG always being
enabled. So lets remove these false warnings by removing the check for
them in the modpost.c tool. Also, lets stop marking the sections
entirely, so define away the section markings in init.h

I'll queue these two patches up through my driver-core git tree, which
has the bulk of the __dev* marking removal patches in it. If anyone has
any objections to this, please let me know.

thanks,

greg k-h

---
include/linux/init.h | 18 +++++++-----------
scripts/mod/modpost.c | 24 ++++++++++--------------
2 files changed, 17 insertions(+), 25 deletions(-)


2012-11-29 18:45:23

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH 1/2] modpost.c: Stop checking __dev* section mismatches

From: Greg Kroah-Hartman <[email protected]>

Now that the __dev* sections are not being generated, we don't need to
check for them in modpost.c.

Signed-off-by: Greg Kroah-Hartman <[email protected]>

---
scripts/mod/modpost.c | 24 ++++++++++--------------
1 file changed, 10 insertions(+), 14 deletions(-)

diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
index 0d93856..ff36c50 100644
--- a/scripts/mod/modpost.c
+++ b/scripts/mod/modpost.c
@@ -858,25 +858,23 @@ static void check_section(const char *modname, struct elf_info *elf,

#define ALL_INIT_DATA_SECTIONS \
".init.setup$", ".init.rodata$", \
- ".devinit.rodata$", ".cpuinit.rodata$", ".meminit.rodata$", \
- ".init.data$", ".devinit.data$", ".cpuinit.data$", ".meminit.data$"
+ ".cpuinit.rodata$", ".meminit.rodata$", \
+ ".init.data$", ".cpuinit.data$", ".meminit.data$"
#define ALL_EXIT_DATA_SECTIONS \
- ".exit.data$", ".devexit.data$", ".cpuexit.data$", ".memexit.data$"
+ ".exit.data$", ".cpuexit.data$", ".memexit.data$"

#define ALL_INIT_TEXT_SECTIONS \
- ".init.text$", ".devinit.text$", ".cpuinit.text$", ".meminit.text$"
+ ".init.text$", ".cpuinit.text$", ".meminit.text$"
#define ALL_EXIT_TEXT_SECTIONS \
- ".exit.text$", ".devexit.text$", ".cpuexit.text$", ".memexit.text$"
+ ".exit.text$", ".cpuexit.text$", ".memexit.text$"

#define ALL_PCI_INIT_SECTIONS \
".pci_fixup_early$", ".pci_fixup_header$", ".pci_fixup_final$", \
".pci_fixup_enable$", ".pci_fixup_resume$", \
".pci_fixup_resume_early$", ".pci_fixup_suspend$"

-#define ALL_XXXINIT_SECTIONS DEV_INIT_SECTIONS, CPU_INIT_SECTIONS, \
- MEM_INIT_SECTIONS
-#define ALL_XXXEXIT_SECTIONS DEV_EXIT_SECTIONS, CPU_EXIT_SECTIONS, \
- MEM_EXIT_SECTIONS
+#define ALL_XXXINIT_SECTIONS CPU_INIT_SECTIONS, MEM_INIT_SECTIONS
+#define ALL_XXXEXIT_SECTIONS CPU_EXIT_SECTIONS, MEM_EXIT_SECTIONS

#define ALL_INIT_SECTIONS INIT_SECTIONS, ALL_XXXINIT_SECTIONS
#define ALL_EXIT_SECTIONS EXIT_SECTIONS, ALL_XXXEXIT_SECTIONS
@@ -885,12 +883,10 @@ static void check_section(const char *modname, struct elf_info *elf,
#define TEXT_SECTIONS ".text$"

#define INIT_SECTIONS ".init.*"
-#define DEV_INIT_SECTIONS ".devinit.*"
#define CPU_INIT_SECTIONS ".cpuinit.*"
#define MEM_INIT_SECTIONS ".meminit.*"

#define EXIT_SECTIONS ".exit.*"
-#define DEV_EXIT_SECTIONS ".devexit.*"
#define CPU_EXIT_SECTIONS ".cpuexit.*"
#define MEM_EXIT_SECTIONS ".memexit.*"

@@ -979,7 +975,7 @@ const struct sectioncheck sectioncheck[] = {
.mismatch = DATA_TO_ANY_EXIT,
.symbol_white_list = { DEFAULT_SYMBOL_WHITE_LIST, NULL },
},
-/* Do not reference init code/data from devinit/cpuinit/meminit code/data */
+/* Do not reference init code/data from cpuinit/meminit code/data */
{
.fromsec = { ALL_XXXINIT_SECTIONS, NULL },
.tosec = { INIT_SECTIONS, NULL },
@@ -1000,7 +996,7 @@ const struct sectioncheck sectioncheck[] = {
.mismatch = XXXINIT_TO_SOME_INIT,
.symbol_white_list = { DEFAULT_SYMBOL_WHITE_LIST, NULL },
},
-/* Do not reference exit code/data from devexit/cpuexit/memexit code/data */
+/* Do not reference exit code/data from cpuexit/memexit code/data */
{
.fromsec = { ALL_XXXEXIT_SECTIONS, NULL },
.tosec = { EXIT_SECTIONS, NULL },
@@ -1089,7 +1085,7 @@ static const struct sectioncheck *section_mismatch(
* Pattern 2:
* Many drivers utilise a *driver container with references to
* add, remove, probe functions etc.
- * These functions may often be marked __devinit and we do not want to
+ * These functions may often be marked __cpuinit and we do not want to
* warn here.
* the pattern is identified by:
* tosec = init or exit section

2012-11-29 18:45:45

by Greg Kroah-Hartman

[permalink] [raw]
Subject: [PATCH 1/2] init.h: Remove __dev* sections from the kernel

From: Greg Kroah-Hartman <[email protected]>

With the recent work to remove CONFIG_HOTPLUG, we are starting to get a
bunch of __devinit section warnings, despite CONFIG_HOTPLUG always being
enabled. So, stop marking the sections entirely, by defining them away
the section markings in init.h

Signed-off-by: Greg Kroah-Hartman <[email protected]>

---
include/linux/init.h | 18 +++++++-----------
1 file changed, 7 insertions(+), 11 deletions(-)

diff --git a/include/linux/init.h b/include/linux/init.h
index e59041e..f63692d 100644
--- a/include/linux/init.h
+++ b/include/linux/init.h
@@ -93,13 +93,13 @@

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

-/* Used for HOTPLUG */
-#define __devinit __section(.devinit.text) __cold notrace
-#define __devinitdata __section(.devinit.data)
-#define __devinitconst __constsection(.devinit.rodata)
-#define __devexit __section(.devexit.text) __exitused __cold notrace
-#define __devexitdata __section(.devexit.data)
-#define __devexitconst __constsection(.devexit.rodata)
+/* Used for HOTPLUG, but that is always enabled now, so just make them noops */
+#define __devinit
+#define __devinitdata
+#define __devinitconst
+#define __devexit
+#define __devexitdata
+#define __devexitconst

/* Used for HOTPLUG_CPU */
#define __cpuinit __section(.cpuinit.text) __cold notrace
@@ -126,10 +126,6 @@
#define __INITRODATA .section ".init.rodata","a",%progbits
#define __FINITDATA .previous

-#define __DEVINIT .section ".devinit.text", "ax"
-#define __DEVINITDATA .section ".devinit.data", "aw"
-#define __DEVINITRODATA .section ".devinit.rodata", "a"
-
#define __CPUINIT .section ".cpuinit.text", "ax"
#define __CPUINITDATA .section ".cpuinit.data", "aw"
#define __CPUINITRODATA .section ".cpuinit.rodata", "a"

2012-11-30 18:12:02

by Sam Ravnborg

[permalink] [raw]
Subject: Re: [PATCH 1/2] init.h: Remove __dev* sections from the kernel

On Thu, Nov 29, 2012 at 10:43:28AM -0800, Greg Kroah-Hartman wrote:
> From: Greg Kroah-Hartman <[email protected]>
>
> With the recent work to remove CONFIG_HOTPLUG, we are starting to get a
> bunch of __devinit section warnings, despite CONFIG_HOTPLUG always being
> enabled. So, stop marking the sections entirely, by defining them away
> the section markings in init.h
>
> Signed-off-by: Greg Kroah-Hartman <[email protected]>
Acked-by: Sam Ravnborg <[email protected]>
>
> ---
> include/linux/init.h | 18 +++++++-----------
> 1 file changed, 7 insertions(+), 11 deletions(-)
>
> diff --git a/include/linux/init.h b/include/linux/init.h
> index e59041e..f63692d 100644
> --- a/include/linux/init.h
> +++ b/include/linux/init.h
> @@ -93,13 +93,13 @@
>
> #define __exit __section(.exit.text) __exitused __cold notrace
>
> -/* Used for HOTPLUG */
> -#define __devinit __section(.devinit.text) __cold notrace
> -#define __devinitdata __section(.devinit.data)
> -#define __devinitconst __constsection(.devinit.rodata)
> -#define __devexit __section(.devexit.text) __exitused __cold notrace
> -#define __devexitdata __section(.devexit.data)
> -#define __devexitconst __constsection(.devexit.rodata)
> +/* Used for HOTPLUG, but that is always enabled now, so just make them noops */

This comment should be explicit that these are provided only for backward compatibility
and their usage is discouarged.

Sam

2012-11-30 18:13:24

by Sam Ravnborg

[permalink] [raw]
Subject: Re: [PATCH 1/2] modpost.c: Stop checking __dev* section mismatches

On Thu, Nov 29, 2012 at 10:45:02AM -0800, Greg Kroah-Hartman wrote:
> From: Greg Kroah-Hartman <[email protected]>
>
> Now that the __dev* sections are not being generated, we don't need to
> check for them in modpost.c.
>
> Signed-off-by: Greg Kroah-Hartman <[email protected]>
Acked-by: Sam Ravnborg <[email protected]>

2012-11-30 18:28:20

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH 1/2] init.h: Remove __dev* sections from the kernel

On Fri, Nov 30, 2012 at 07:11:56PM +0100, Sam Ravnborg wrote:
> On Thu, Nov 29, 2012 at 10:43:28AM -0800, Greg Kroah-Hartman wrote:
> > From: Greg Kroah-Hartman <[email protected]>
> >
> > With the recent work to remove CONFIG_HOTPLUG, we are starting to get a
> > bunch of __devinit section warnings, despite CONFIG_HOTPLUG always being
> > enabled. So, stop marking the sections entirely, by defining them away
> > the section markings in init.h
> >
> > Signed-off-by: Greg Kroah-Hartman <[email protected]>
> Acked-by: Sam Ravnborg <[email protected]>

Thanks for the review of this and the other patch.

greg k-h

2012-11-30 20:40:14

by H. Peter Anvin

[permalink] [raw]
Subject: Re: [PATCH 1/2] init.h: Remove __dev* sections from the kernel

On 11/29/2012 10:43 AM, Greg Kroah-Hartman wrote:
> From: Greg Kroah-Hartman <[email protected]>
>
> With the recent work to remove CONFIG_HOTPLUG, we are starting to get a
> bunch of __devinit section warnings, despite CONFIG_HOTPLUG always being
> enabled. So, stop marking the sections entirely, by defining them away
> the section markings in init.h
>

Could we run a sed script over the kernel tree and generate patches to
remove the markings completely?

-hpa

--
H. Peter Anvin, Intel Open Source Technology Center
I work for Intel. I don't speak on their behalf.

2012-11-30 20:40:49

by H. Peter Anvin

[permalink] [raw]
Subject: Re: [PATCH 1/2] init.h: Remove __dev* sections from the kernel

On 11/29/2012 10:43 AM, Greg Kroah-Hartman wrote:
>
> -/* Used for HOTPLUG */
> -#define __devinit __section(.devinit.text) __cold notrace
> -#define __devinitdata __section(.devinit.data)
> -#define __devinitconst __constsection(.devinit.rodata)
> -#define __devexit __section(.devexit.text) __exitused __cold notrace
> -#define __devexitdata __section(.devexit.data)
> -#define __devexitconst __constsection(.devexit.rodata)
> +/* Used for HOTPLUG, but that is always enabled now, so just make them noops */
> +#define __devinit
> +#define __devinitdata
> +#define __devinitconst
> +#define __devexit
> +#define __devexitdata
> +#define __devexitconst
>

Second question... what about the __exitused __cold notrace annotations?

-hpa


--
H. Peter Anvin, Intel Open Source Technology Center
I work for Intel. I don't speak on their behalf.

2012-11-30 20:43:12

by wfp5p

[permalink] [raw]
Subject: Re: [PATCH 1/2] init.h: Remove __dev* sections from the kernel

H. Peter Anvin writes:
>
> On 11/29/2012 10:43 AM, Greg Kroah-Hartman wrote:
> > From: Greg Kroah-Hartman <[email protected]>
> >
> > With the recent work to remove CONFIG_HOTPLUG, we are starting to get a
> > bunch of __devinit section warnings, despite CONFIG_HOTPLUG always being
> > enabled. So, stop marking the sections entirely, by defining them away
> > the section markings in init.h
> >
>
> Could we run a sed script over the kernel tree and generate patches to
> remove the markings completely?
>

That's how I've been generating the patches to remove the markings --
except it's a perl script.

--
Bill

2012-11-30 20:45:31

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH 1/2] init.h: Remove __dev* sections from the kernel

On Fri, Nov 30, 2012 at 12:39:01PM -0800, H. Peter Anvin wrote:
> On 11/29/2012 10:43 AM, Greg Kroah-Hartman wrote:
> >From: Greg Kroah-Hartman <[email protected]>
> >
> >With the recent work to remove CONFIG_HOTPLUG, we are starting to get a
> >bunch of __devinit section warnings, despite CONFIG_HOTPLUG always being
> >enabled. So, stop marking the sections entirely, by defining them away
> >the section markings in init.h
> >
>
> Could we run a sed script over the kernel tree and generate patches
> to remove the markings completely?

Yes, we can, and Bill's been generating patches that way. A number of
them are already in linux-next and I'll sweep the tree after 3.8-rc1 is
out and catch the rest.

thanks,

greg k-h

2012-11-30 20:47:43

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH 1/2] init.h: Remove __dev* sections from the kernel

On Fri, Nov 30, 2012 at 12:39:43PM -0800, H. Peter Anvin wrote:
> On 11/29/2012 10:43 AM, Greg Kroah-Hartman wrote:
> >
> >-/* Used for HOTPLUG */
> >-#define __devinit __section(.devinit.text) __cold notrace
> >-#define __devinitdata __section(.devinit.data)
> >-#define __devinitconst __constsection(.devinit.rodata)
> >-#define __devexit __section(.devexit.text) __exitused __cold notrace
> >-#define __devexitdata __section(.devexit.data)
> >-#define __devexitconst __constsection(.devexit.rodata)
> >+/* Used for HOTPLUG, but that is always enabled now, so just make them noops */
> >+#define __devinit
> >+#define __devinitdata
> >+#define __devinitconst
> >+#define __devexit
> >+#define __devexitdata
> >+#define __devexitconst
> >
>
> Second question... what about the __exitused __cold notrace annotations?

__exitused shouldn't matter, we shouldn't be optimizing these away at
all anymore. I don't know about __cold and notrace, but given that
these could be traced, I don't see why that marking got there in the
first place.

In reading compiler-gcc4.h, and knowing just how infrequent these
functions are ever called, I would be amazed if the __cold marking ever
even helped anything out here.

thanks,

greg k-h