Discussion:
[PATCH 3/5] x86/PCI: Mark constants of pci_mmcfg_nvidia_mcp55() as __initconst
Mathias Krause
2014-08-25 21:26:37 UTC
Permalink
The constants in pci_mmcfg_nvidia_mcp55() need to be marked as
__initconst or they will remain in memory after init memory was
released.

Signed-off-by: Mathias Krause <***@googlemail.com>
---
arch/x86/pci/mmconfig-shared.c | 24 ++++++++++++++----------
1 file changed, 14 insertions(+), 10 deletions(-)

diff --git a/arch/x86/pci/mmconfig-shared.c b/arch/x86/pci/mmconfig-shared.c
index 163ef6bf16..63fc0d4d58 100644
--- a/arch/x86/pci/mmconfig-shared.c
+++ b/arch/x86/pci/mmconfig-shared.c
@@ -240,16 +240,20 @@ static const char *__init pci_mmcfg_nvidia_mcp55(void)
int bus;
int mcp55_mmconf_found = 0;

- static const u32 extcfg_regnum = 0x90;
- static const u32 extcfg_regsize = 4;
- static const u32 extcfg_enable_mask = 1<<31;
- static const u32 extcfg_start_mask = 0xff<<16;
- static const int extcfg_start_shift = 16;
- static const u32 extcfg_size_mask = 0x3<<28;
- static const int extcfg_size_shift = 28;
- static const int extcfg_sizebus[] = {0x100, 0x80, 0x40, 0x20};
- static const u32 extcfg_base_mask[] = {0x7ff8, 0x7ffc, 0x7ffe, 0x7fff};
- static const int extcfg_base_lshift = 25;
+ static const u32 extcfg_regnum __initconst = 0x90;
+ static const u32 extcfg_regsize __initconst = 4;
+ static const u32 extcfg_enable_mask __initconst = 1 << 31;
+ static const u32 extcfg_start_mask __initconst = 0xff << 16;
+ static const int extcfg_start_shift __initconst = 16;
+ static const u32 extcfg_size_mask __initconst = 0x3 << 28;
+ static const int extcfg_size_shift __initconst = 28;
+ static const int extcfg_sizebus[] __initconst = {
+ 0x100, 0x80, 0x40, 0x20
+ };
+ static const u32 extcfg_base_mask[] __initconst = {
+ 0x7ff8, 0x7ffc, 0x7ffe, 0x7fff
+ };
+ static const int extcfg_base_lshift __initconst = 25;

/*
* do check if amd fam10h already took over
--
1.7.10.4
Mathias Krause
2014-08-25 21:26:35 UTC
Permalink
The DMI tables are only used in __init code, thereby can be marked as
initialization data, too. The same is true for the callback functions
referenced from the DMI tables.

This moves ~9.6 kB of code and r/o data to the init sections, marking
the memory for release after initialization.

Signed-off-by: Mathias Krause <***@googlemail.com>
---
arch/x86/pci/common.c | 20 ++++++++++----------
1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/arch/x86/pci/common.c b/arch/x86/pci/common.c
index 059a76c297..7b20bccf36 100644
--- a/arch/x86/pci/common.c
+++ b/arch/x86/pci/common.c
@@ -81,14 +81,14 @@ struct pci_ops pci_root_ops = {
*/
DEFINE_RAW_SPINLOCK(pci_config_lock);

-static int can_skip_ioresource_align(const struct dmi_system_id *d)
+static int __init can_skip_ioresource_align(const struct dmi_system_id *d)
{
pci_probe |= PCI_CAN_SKIP_ISA_ALIGN;
printk(KERN_INFO "PCI: %s detected, can skip ISA alignment\n", d->ident);
return 0;
}

-static const struct dmi_system_id can_skip_pciprobe_dmi_table[] = {
+static const struct dmi_system_id can_skip_pciprobe_dmi_table[] __initconst = {
/*
* Systems where PCI IO resource ISA alignment can be skipped
* when the ISA enable bit in the bridge control is not set
@@ -186,7 +186,7 @@ void pcibios_remove_bus(struct pci_bus *bus)
* on the kernel command line (which was parsed earlier).
*/

-static int set_bf_sort(const struct dmi_system_id *d)
+static int __init set_bf_sort(const struct dmi_system_id *d)
{
if (pci_bf_sort == pci_bf_sort_default) {
pci_bf_sort = pci_dmi_bf;
@@ -195,8 +195,8 @@ static int set_bf_sort(const struct dmi_system_id *d)
return 0;
}

-static void read_dmi_type_b1(const struct dmi_header *dm,
- void *private_data)
+static void __init read_dmi_type_b1(const struct dmi_header *dm,
+ void *private_data)
{
u8 *d = (u8 *)dm + 4;

@@ -217,7 +217,7 @@ static void read_dmi_type_b1(const struct dmi_header *dm,
}
}

-static int find_sort_method(const struct dmi_system_id *d)
+static int __init find_sort_method(const struct dmi_system_id *d)
{
dmi_walk(read_dmi_type_b1, NULL);

@@ -232,7 +232,7 @@ static int find_sort_method(const struct dmi_system_id *d)
* Enable renumbering of PCI bus# ranges to reach all PCI busses (Cardbus)
*/
#ifdef __i386__
-static int assign_all_busses(const struct dmi_system_id *d)
+static int __init assign_all_busses(const struct dmi_system_id *d)
{
pci_probe |= PCI_ASSIGN_ALL_BUSSES;
printk(KERN_INFO "%s detected: enabling PCI bus# renumbering"
@@ -241,7 +241,7 @@ static int assign_all_busses(const struct dmi_system_id *d)
}
#endif

-static int set_scan_all(const struct dmi_system_id *d)
+static int __init set_scan_all(const struct dmi_system_id *d)
{
printk(KERN_INFO "PCI: %s detected, enabling pci=pcie_scan_all\n",
d->ident);
@@ -249,7 +249,7 @@ static int set_scan_all(const struct dmi_system_id *d)
return 0;
}

-static const struct dmi_system_id pciprobe_dmi_table[] = {
+static const struct dmi_system_id pciprobe_dmi_table[] __initconst = {
#ifdef __i386__
/*
* Laptops which need pci=assign-busses to see Cardbus cards
@@ -512,7 +512,7 @@ int __init pcibios_init(void)
return 0;
}

-char * __init pcibios_setup(char *str)
+char *__init pcibios_setup(char *str)
{
if (!strcmp(str, "off")) {
pci_probe = 0;
--
1.7.10.4
Mathias Krause
2014-08-25 21:26:36 UTC
Permalink
According to include/linux/init.h the __init annotation should be added
immediately before the function name. However, for quite a few functions
in mmconfig-shared.c this is not the case. It's either before the return
type or even in the middle of it. Beside gcc still getting it right, we
should change them to comply to the rules of include/linux/init.h.

Signed-off-by: Mathias Krause <***@googlemail.com>
---
arch/x86/pci/mmconfig-shared.c | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/arch/x86/pci/mmconfig-shared.c b/arch/x86/pci/mmconfig-shared.c
index 248642f4ba..163ef6bf16 100644
--- a/arch/x86/pci/mmconfig-shared.c
+++ b/arch/x86/pci/mmconfig-shared.c
@@ -31,7 +31,7 @@ static DEFINE_MUTEX(pci_mmcfg_lock);

LIST_HEAD(pci_mmcfg_list);

-static __init void pci_mmconfig_remove(struct pci_mmcfg_region *cfg)
+static void __init pci_mmconfig_remove(struct pci_mmcfg_region *cfg)
{
if (cfg->res.parent)
release_resource(&cfg->res);
@@ -39,7 +39,7 @@ static __init void pci_mmconfig_remove(struct pci_mmcfg_region *cfg)
kfree(cfg);
}

-static __init void free_all_mmcfg(void)
+static void __init free_all_mmcfg(void)
{
struct pci_mmcfg_region *cfg, *tmp;

@@ -93,7 +93,7 @@ static struct pci_mmcfg_region *pci_mmconfig_alloc(int segment, int start,
return new;
}

-static __init struct pci_mmcfg_region *pci_mmconfig_add(int segment, int start,
+static struct pci_mmcfg_region *__init pci_mmconfig_add(int segment, int start,
int end, u64 addr)
{
struct pci_mmcfg_region *new;
@@ -125,7 +125,7 @@ struct pci_mmcfg_region *pci_mmconfig_lookup(int segment, int bus)
return NULL;
}

-static const char __init *pci_mmcfg_e7520(void)
+static const char *__init pci_mmcfg_e7520(void)
{
u32 win;
raw_pci_ops->read(0, 0, PCI_DEVFN(0, 0), 0xce, 2, &win);
@@ -140,7 +140,7 @@ static const char __init *pci_mmcfg_e7520(void)
return "Intel Corporation E7520 Memory Controller Hub";
}

-static const char __init *pci_mmcfg_intel_945(void)
+static const char *__init pci_mmcfg_intel_945(void)
{
u32 pciexbar, mask = 0, len = 0;

@@ -184,7 +184,7 @@ static const char __init *pci_mmcfg_intel_945(void)
return "Intel Corporation 945G/GZ/P/PL Express Memory Controller Hub";
}

-static const char __init *pci_mmcfg_amd_fam10h(void)
+static const char *__init pci_mmcfg_amd_fam10h(void)
{
u32 low, high, address;
u64 base, msr;
@@ -235,7 +235,7 @@ static const char __init *pci_mmcfg_amd_fam10h(void)
}

static bool __initdata mcp55_checked;
-static const char __init *pci_mmcfg_nvidia_mcp55(void)
+static const char *__init pci_mmcfg_nvidia_mcp55(void)
{
int bus;
int mcp55_mmconf_found = 0;
--
1.7.10.4
Mathias Krause
2014-08-25 21:26:39 UTC
Permalink
The pci_find_bios() function is only ever called from initialization
code, therefore can be marked as such, too. This, in turn, allows
marking other functions called only in this context as well.

The bios32_indirect variable can be marked as __initdata as it is only
referenced from __init functions now.

Signed-off-by: Mathias Krause <***@googlemail.com>
---
arch/x86/pci/pcbios.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/arch/x86/pci/pcbios.c b/arch/x86/pci/pcbios.c
index c77b24a8b2..9b83b9051a 100644
--- a/arch/x86/pci/pcbios.c
+++ b/arch/x86/pci/pcbios.c
@@ -79,13 +79,13 @@ union bios32 {
static struct {
unsigned long address;
unsigned short segment;
-} bios32_indirect = { 0, __KERNEL_CS };
+} bios32_indirect __initdata = { 0, __KERNEL_CS };

/*
* Returns the entry point for the given service, NULL on error
*/

-static unsigned long bios32_service(unsigned long service)
+static unsigned long __init bios32_service(unsigned long service)
{
unsigned char return_code; /* %al */
unsigned long address; /* %ebx */
@@ -124,7 +124,7 @@ static struct {

static int pci_bios_present;

-static int check_pcibios(void)
+static int __init check_pcibios(void)
{
u32 signature, eax, ebx, ecx;
u8 status, major_ver, minor_ver, hw_mech;
@@ -312,7 +312,7 @@ static const struct pci_raw_ops pci_bios_access = {
* Try to find PCI BIOS.
*/

-static const struct pci_raw_ops *pci_find_bios(void)
+static const struct pci_raw_ops *__init pci_find_bios(void)
{
union bios32 *check;
unsigned char sum;
--
1.7.10.4
Mathias Krause
2014-08-25 21:26:38 UTC
Permalink
The pci_mmcfg_probes[] array is only ever read, therefore make it const.

Signed-off-by: Mathias Krause <***@googlemail.com>
---
arch/x86/pci/mmconfig-shared.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/pci/mmconfig-shared.c b/arch/x86/pci/mmconfig-shared.c
index 63fc0d4d58..326198a443 100644
--- a/arch/x86/pci/mmconfig-shared.c
+++ b/arch/x86/pci/mmconfig-shared.c
@@ -306,7 +306,7 @@ struct pci_mmcfg_hostbridge_probe {
const char *(*probe)(void);
};

-static struct pci_mmcfg_hostbridge_probe pci_mmcfg_probes[] __initdata = {
+static const struct pci_mmcfg_hostbridge_probe pci_mmcfg_probes[] __initconst = {
{ 0, PCI_DEVFN(0, 0), PCI_VENDOR_ID_INTEL,
PCI_DEVICE_ID_INTEL_E7520_MCH, pci_mmcfg_e7520 },
{ 0, PCI_DEVFN(0, 0), PCI_VENDOR_ID_INTEL,
--
1.7.10.4
Bjorn Helgaas
2014-09-22 20:27:48 UTC
Permalink
Hi Bjorn,
this series is a collection of patches trying to mark initialization
code and data of the x86 specific PCI code as such. It also contains
__init annotation cleanups to move them to the spot they belong to
according to include/linux/init.h.
The annotation leads to a nice reduction of run-time memory size. At
least two additional pages can be released after initialization which
are otherwise occupied by code and data no longer needed.
Please apply!
Thanks,
x86/PCI: Mark DMI tables as initialization data
x86/PCI: Move __init annotation were it belongs to
x86/PCI: Mark constants of pci_mmcfg_nvidia_mcp55() as __initconst
x86/PCI: Constify pci_mmcfg_probes[] array
x86/PCI: Mark PCI BIOS initialization code as such
All applied to pci/initdata for v3.18, thanks!
arch/x86/pci/common.c | 20 ++++++++--------
arch/x86/pci/mmconfig-shared.c | 40 ++++++++++++++++++--------------
arch/x86/pci/pcbios.c | 8 +++----
3 files changed, 36 insertions(+), 32 deletions(-)
--
1.7.10.4
Bjorn Helgaas
2014-09-22 20:34:28 UTC
Permalink
[+cc x86 folks]
Post by Bjorn Helgaas
Hi Bjorn,
this series is a collection of patches trying to mark initialization
code and data of the x86 specific PCI code as such. It also contains
__init annotation cleanups to move them to the spot they belong to
according to include/linux/init.h.
The annotation leads to a nice reduction of run-time memory size. At
least two additional pages can be released after initialization which
are otherwise occupied by code and data no longer needed.
Please apply!
Thanks,
x86/PCI: Mark DMI tables as initialization data
x86/PCI: Move __init annotation were it belongs to
x86/PCI: Mark constants of pci_mmcfg_nvidia_mcp55() as __initconst
x86/PCI: Constify pci_mmcfg_probes[] array
x86/PCI: Mark PCI BIOS initialization code as such
All applied to pci/initdata for v3.18, thanks!
Oops, I didn't notice that you hadn't cc'd the x86 folks.

Guys, if these should go through -tip, here's my ack:

Acked-by: Bjorn Helgaas <***@google.com>

I don't have any other arch/x86 changes in my tree, so I don't care which
way these go.
Post by Bjorn Helgaas
arch/x86/pci/common.c | 20 ++++++++--------
arch/x86/pci/mmconfig-shared.c | 40 ++++++++++++++++++--------------
arch/x86/pci/pcbios.c | 8 +++----
3 files changed, 36 insertions(+), 32 deletions(-)
--
1.7.10.4
Mathias Krause
2014-09-23 15:43:29 UTC
Permalink
Post by Bjorn Helgaas
[+cc x86 folks]
Post by Bjorn Helgaas
Hi Bjorn,
[...]
x86/PCI: Mark DMI tables as initialization data
x86/PCI: Move __init annotation were it belongs to
x86/PCI: Mark constants of pci_mmcfg_nvidia_mcp55() as __initconst
x86/PCI: Constify pci_mmcfg_probes[] array
x86/PCI: Mark PCI BIOS initialization code as such
All applied to pci/initdata for v3.18, thanks!
Oops, I didn't notice that you hadn't cc'd the x86 folks.
The reason for me not cc'ing the x86 maintainers, but linux-pci
instead was probably because you're listed as maintainer for these
files in the MAINTAINERS file. But, true, x86 maintainers should be
cc'ed for all patches to code below arch/x86/. Just an oversight of
mine :/
Post by Bjorn Helgaas
I don't have any other arch/x86 changes in my tree, so I don't care which
way these go.
Regards,
Mathias
Ingo Molnar
2014-09-24 07:28:54 UTC
Permalink
Post by Bjorn Helgaas
[+cc x86 folks]
Post by Bjorn Helgaas
Hi Bjorn,
this series is a collection of patches trying to mark initialization
code and data of the x86 specific PCI code as such. It also contains
__init annotation cleanups to move them to the spot they belong to
according to include/linux/init.h.
The annotation leads to a nice reduction of run-time memory size. At
least two additional pages can be released after initialization which
are otherwise occupied by code and data no longer needed.
Please apply!
Thanks,
x86/PCI: Mark DMI tables as initialization data
x86/PCI: Move __init annotation were it belongs to
x86/PCI: Mark constants of pci_mmcfg_nvidia_mcp55() as __initconst
x86/PCI: Constify pci_mmcfg_probes[] array
x86/PCI: Mark PCI BIOS initialization code as such
All applied to pci/initdata for v3.18, thanks!
Oops, I didn't notice that you hadn't cc'd the x86 folks.
I don't have any other arch/x86 changes in my tree, so I don't care which
way these go.
Post by Bjorn Helgaas
arch/x86/pci/common.c | 20 ++++++++--------
arch/x86/pci/mmconfig-shared.c | 40 ++++++++++++++++++--------------
arch/x86/pci/pcbios.c | 8 +++----
3 files changed, 36 insertions(+), 32 deletions(-)
They are looking good to me in principle, and merging them via
the PCI tree is fine with me as well:

Acked-by: Ingo Molnar <***@kernel.org>

Thanks,

Ingo

Loading...