[PATCH 00/20] powerpc: Define eligible functions as __init
Christophe Leroy
christophe.leroy at csgroup.eu
Thu Dec 16 20:44:01 AEDT 2021
Le 15/12/2021 à 17:12, Nick Child a écrit :
> Hello all,
>
> This patchset focuses on redefining/declaring functions that could be
> labeled with the macro `__init`. From my understanding, an initialization
> function is one which is only needed during the initial phases of booting,
> after which it's resources can be freed. I figure that any function which is
> only called by other initialization functions may also be labeled as an
> initialization function. There are several (mostly static) functions which
> can and should be labeled as `__init`. I created some scripts to help
> identify these functions. It scans all functions defined in `arch/powerpc`
> and, if it is only called by functions with the `__init` attribute, it will
> go on to adjust the prototype and definition to include the `__init`
> declaration.
>
> Some notes about the scripts:
> 1. Trying to handle things like functions assigned to macros and function
> pointers proved to be a bit too complicated to track. I ended up just
> halting the investigation of these functions. In the future, I would
> like to be able to attempt to `__init` them but for now we leave them
> alone.
>
> 2. The proper syntax for adding the `__init` macro is followed as it is
> defined in `include/linux/init.h`.
It says:
You should add __init immediately before the function name
And
If the function has a prototype somewhere, you can also add __init
between closing brace of the prototype and semicolon
As far as I can see the second alternative is almost never followed in
arch/powerpc/include/asm/, so I would suggest to apply first, taking
into account that a 'should' is more important than a child.
On the other hand I don't think the __init flag is needed at all for
prototypes, so maybe we could avoid the changes.
Whatever you choose to do, you should take the opportunity to remove the
pointless 'extern' keyword on the lines you modifify in .h files.
checkpatch reports the following on your first patch:
CHECK: extern prototypes should be avoided in .h files
#50: FILE: arch/powerpc/include/asm/btext.h:26:
+extern void btext_drawhex(unsigned long v) __init;
CHECK: extern prototypes should be avoided in .h files
#51: FILE: arch/powerpc/include/asm/btext.h:27:
+extern void btext_drawtext(const char *c, unsigned int len) __init;
CHECK: extern prototypes should be avoided in .h files
#56: FILE: arch/powerpc/include/asm/btext.h:29:
+extern void btext_clearscreen(void) __init;
CHECK: extern prototypes should be avoided in .h files
#57: FILE: arch/powerpc/include/asm/btext.h:30:
+extern void btext_flushscreen(void) __init;
CHECK: extern prototypes should be avoided in .h files
#58: FILE: arch/powerpc/include/asm/btext.h:31:
+extern void btext_flushline(void) __init;
CHECK: extern prototypes should be avoided in .h files
#102: FILE: arch/powerpc/include/asm/kexec.h:87:
+extern int overlaps_crashkernel(unsigned long start, unsigned long
size) __init;
CHECK: extern prototypes should be avoided in .h files
#128: FILE: arch/powerpc/include/asm/pci.h:51:
+extern void set_pci_dma_ops(const struct dma_map_ops *dma_ops) __init;
CHECK: extern prototypes should be avoided in .h files
#164: FILE: arch/powerpc/include/asm/udbg.h:26:
+extern void udbg_uart_init_mmio(void __iomem *addr, unsigned int
stride) __init;
CHECK: extern prototypes should be avoided in .h files
#165: FILE: arch/powerpc/include/asm/udbg.h:27:
+extern void udbg_uart_init_pio(unsigned long port, unsigned int stride)
__init;
CHECK: extern prototypes should be avoided in .h files
#169: FILE: arch/powerpc/include/asm/udbg.h:29:
+extern void udbg_uart_setup(unsigned int speed, unsigned int clock) __init;
CHECK: extern prototypes should be avoided in .h files
#170: FILE: arch/powerpc/include/asm/udbg.h:30:
+extern unsigned int udbg_probe_uart_speed(unsigned int clock) __init;
CHECK: Alignment should match open parenthesis
#371: FILE: arch/powerpc/kernel/nvram_64.c:759:
+static int __init nvram_can_remove_partition(struct nvram_partition *part,
const char *name, int sig, const char *exceptions[])
WARNING: externs should be avoided in .c files
#396: FILE: arch/powerpc/kernel/pci_32.c:40:
+void __init pcibios_make_OF_bus_map(void);
CHECK: Alignment should match open parenthesis
#549: FILE: arch/powerpc/kernel/setup_64.c:503:
+static void __init init_cache_info(struct ppc_cache_info *info, u32
size, u32 lsize,
u32 bsize, u32 sets)
WARNING: line length of 123 exceeds 100 columns
#561: FILE: arch/powerpc/kernel/smp.c:899:
+static int __init update_mask_from_threadgroup(cpumask_var_t *mask,
struct thread_groups *tg, int cpu, int cpu_group_start)
WARNING: From:/Signed-off-by: email address mismatch: 'From: Nick Child
<nnac123 at gmail.com>' != 'Signed-off-by: Nick Child <nick.child at ibm.com>'
total: 0 errors, 3 warnings, 13 checks, 489 lines checked
NOTE: For some of the reported defects, checkpatch may be able to
mechanically convert to the typical style using --fix or
--fix-inplace.
Commit 97a61cac5b72 ("powerpc/kernel: Add __init attribute to eligible
functions") has style problems, please review.
NOTE: If any of the errors are false positives, please report
them to the maintainer, see CHECKPATCH in MAINTAINERS.
>
> 3. There are some odd looking functions that look something like:
> `char *__init foo(...`. I have found that this happens in many other
> places in the kernel source prior to running the scripts. Additionally,
> after running the scripts. I have successfully built all powerpc
> defconfigs without error. For these reasons, I assume the odd syntax is
> valid but I am interested in other opinions on this.
>
> 5. I have run my scripts on the 12/3/2021 `merge` branch (specifically
> 78e00acdd35c)
Many CONFLICTs with latests.
12/3/2021 means 12 March 2021 for me
(https://en.wikipedia.org/wiki/Date_format_by_country)
>
> After running my script for 3 rounds, they have identified about 250
> functions so I tried to break the patches up into related platforms and
> subdirectories. There are only two functions with a prototype outside of
> `arch/powerpc`, these are `find_via_cuda` and `find_via_pmu`. I put those in
> their own patch.
>
> Other than building all the ppc defconfigs, I have tested the changes by
> building kernels with and without the patches using defconfigs ppc64le_guest,
> powernv and pseries_le. There were no size differences reported in the boot
> logs. However, when using `readelf -t` we see the following differences in
> the size of the .text section:
> ppc64le_guest -> .text decreased in size by 12.8k, init.text unchanged
> powernv -> .text decreased in size by 7.7k, init.text unchanged
> pseries_le -> .text decreased in size by 9.6k, init.text increased 65.5k
>
> I figured the size differences (or lack of) in .init.text might have something
> to do with the requirement that the .init.text "section ends on a page
> boundary", as described in `arch/powerpc/kernel/vmlinux.lds.S`.
>
> I am excited to say this is my first attempt at kernel development. I know
> this patchset is in no way important but I figured it was a good starting
> point. Please let me know if there is anything I am doing wrong or if I can
> do anything to help meet the kernel contribution guidelines.
>
> Special thanks to Daniel Axtens for mentoring me and helping me get my first
> patchset out!
>
> Nick Child (20):
> powerpc/kernel: Add __init attribute to eligible functions
> powerpc/lib: Add __init attribute to eligible functions
> powerpc/mm: Add __init attribute to eligible functions
> powerpc/perf: Add __init attribute to eligible functions
> powerpc/sysdev: Add __init attribute to eligible functions
> powerpc/xmon: Add __init attribute to eligible functions
> powerpc/cell: Add __init attribute to eligible functions
> powerpc/chrp: Add __init attribute to eligible functions
> powerpc/pasemi: Add __init attribute to eligible functions
> powerpc/powermac: Add __init attribute to eligible functions
> powerpc/powernv: Add __init attribute to eligible functions
> powerpc/pseries: Add __init attribute to eligible functions
> powerpc/ps3: Add __init attribute to eligible functions
> powerpc/4xx: Add __init attribute to eligible functions
> powerpc/44x: Add __init attribute to eligible functions
> powerpc/embedded6xx: Add __init attribute to eligible functions
> powerpc/83xx: Add __init attribute to eligible functions
> powerpc/85xx: Add __init attribute to eligible functions
> powerpc/512x: Add __init attribute to eligible functions
> cuda/pmu: Make find_via_cuda/pmu init functions
>
> arch/powerpc/include/asm/book3s/64/mmu.h | 2 +-
> arch/powerpc/include/asm/btext.h | 10 ++--
> arch/powerpc/include/asm/cpm2.h | 6 +--
> arch/powerpc/include/asm/cpuidle.h | 2 +-
> arch/powerpc/include/asm/eeh.h | 2 +-
> arch/powerpc/include/asm/fadump-internal.h | 6 +--
> arch/powerpc/include/asm/hugetlb.h | 2 +-
> arch/powerpc/include/asm/i8259.h | 2 +-
> arch/powerpc/include/asm/iommu.h | 2 +-
> arch/powerpc/include/asm/ipic.h | 2 +-
> arch/powerpc/include/asm/kexec.h | 2 +-
> arch/powerpc/include/asm/kvm_guest.h | 2 +-
> arch/powerpc/include/asm/mmu_context.h | 2 +-
> arch/powerpc/include/asm/mpic.h | 2 +-
> arch/powerpc/include/asm/opal.h | 2 +-
> arch/powerpc/include/asm/pci.h | 2 +-
> arch/powerpc/include/asm/perf_event_server.h | 2 +-
> arch/powerpc/include/asm/setup.h | 8 +--
> arch/powerpc/include/asm/smu.h | 2 +-
> arch/powerpc/include/asm/udbg.h | 10 ++--
> arch/powerpc/include/asm/xics.h | 4 +-
> arch/powerpc/include/asm/xmon.h | 2 +-
> arch/powerpc/kernel/btext.c | 12 ++---
> arch/powerpc/kernel/dt_cpu_ftrs.c | 2 +-
> arch/powerpc/kernel/eeh_cache.c | 2 +-
> arch/powerpc/kernel/fadump.c | 18 +++----
> arch/powerpc/kernel/nvram_64.c | 6 +--
> arch/powerpc/kernel/pci-common.c | 2 +-
> arch/powerpc/kernel/pci_32.c | 4 +-
> arch/powerpc/kernel/prom.c | 4 +-
> arch/powerpc/kernel/prom_init.c | 12 ++---
> arch/powerpc/kernel/rtasd.c | 6 +--
> arch/powerpc/kernel/security.c | 4 +-
> arch/powerpc/kernel/setup_64.c | 2 +-
> arch/powerpc/kernel/smp.c | 4 +-
> arch/powerpc/kernel/sysfs.c | 10 ++--
> arch/powerpc/kernel/udbg_16550.c | 10 ++--
> arch/powerpc/kexec/core.c | 2 +-
> arch/powerpc/lib/code-patching.c | 2 +-
> arch/powerpc/lib/feature-fixups.c | 26 +++++-----
> arch/powerpc/mm/book3s32/mmu.c | 2 +-
> arch/powerpc/mm/book3s64/hash_hugetlbpage.c | 2 +-
> arch/powerpc/mm/book3s64/hash_utils.c | 6 +--
> arch/powerpc/mm/book3s64/mmu_context.c | 2 +-
> arch/powerpc/mm/book3s64/pkeys.c | 2 +-
> arch/powerpc/mm/book3s64/radix_pgtable.c | 4 +-
> arch/powerpc/mm/nohash/44x.c | 4 +-
> arch/powerpc/mm/nohash/fsl_book3e.c | 2 +-
> arch/powerpc/mm/nohash/tlb.c | 4 +-
> arch/powerpc/mm/numa.c | 6 +--
> arch/powerpc/mm/ptdump/ptdump.c | 2 +-
> arch/powerpc/perf/core-book3s.c | 2 +-
> arch/powerpc/perf/generic-compat-pmu.c | 2 +-
> arch/powerpc/perf/internal.h | 18 +++----
> arch/powerpc/perf/power10-pmu.c | 2 +-
> arch/powerpc/perf/power5+-pmu.c | 2 +-
> arch/powerpc/perf/power5-pmu.c | 2 +-
> arch/powerpc/perf/power6-pmu.c | 2 +-
> arch/powerpc/perf/power7-pmu.c | 2 +-
> arch/powerpc/perf/power8-pmu.c | 2 +-
> arch/powerpc/perf/power9-pmu.c | 2 +-
> arch/powerpc/perf/ppc970-pmu.c | 2 +-
> arch/powerpc/platforms/44x/fsp2.c | 4 +-
> arch/powerpc/platforms/4xx/cpm.c | 4 +-
> arch/powerpc/platforms/4xx/pci.c | 2 +-
> arch/powerpc/platforms/512x/clock-commonclk.c | 52 +++++++++----------
> arch/powerpc/platforms/512x/mpc512x.h | 4 +-
> arch/powerpc/platforms/512x/mpc512x_shared.c | 4 +-
> arch/powerpc/platforms/83xx/km83xx.c | 2 +-
> arch/powerpc/platforms/83xx/mpc834x_mds.c | 2 +-
> arch/powerpc/platforms/83xx/mpc837x_mds.c | 2 +-
> arch/powerpc/platforms/83xx/mpc837x_rdb.c | 2 +-
> arch/powerpc/platforms/83xx/mpc83xx.h | 6 +--
> arch/powerpc/platforms/83xx/usb.c | 6 +--
> arch/powerpc/platforms/85xx/ge_imp3a.c | 2 +-
> arch/powerpc/platforms/85xx/mpc85xx_cds.c | 2 +-
> .../platforms/85xx/socrates_fpga_pic.c | 2 +-
> .../platforms/85xx/socrates_fpga_pic.h | 2 +-
> arch/powerpc/platforms/85xx/xes_mpc85xx.c | 4 +-
> arch/powerpc/platforms/cell/cbe_regs.c | 2 +-
> arch/powerpc/platforms/cell/iommu.c | 14 ++---
> arch/powerpc/platforms/cell/spu_base.c | 6 +--
> arch/powerpc/platforms/cell/spu_manage.c | 16 +++---
> arch/powerpc/platforms/cell/spufs/inode.c | 2 +-
> arch/powerpc/platforms/chrp/pegasos_eth.c | 2 +-
> arch/powerpc/platforms/embedded6xx/hlwd-pic.c | 4 +-
> arch/powerpc/platforms/embedded6xx/hlwd-pic.h | 2 +-
> arch/powerpc/platforms/embedded6xx/holly.c | 2 +-
> .../platforms/embedded6xx/usbgecko_udbg.c | 4 +-
> arch/powerpc/platforms/embedded6xx/wii.c | 2 +-
> arch/powerpc/platforms/pasemi/msi.c | 2 +-
> arch/powerpc/platforms/pasemi/pasemi.h | 2 +-
> arch/powerpc/platforms/pasemi/pci.c | 2 +-
> arch/powerpc/platforms/pasemi/setup.c | 2 +-
> arch/powerpc/platforms/powermac/feature.c | 2 +-
> arch/powerpc/platforms/powermac/nvram.c | 2 +-
> arch/powerpc/platforms/powermac/pfunc_base.c | 6 +--
> arch/powerpc/platforms/powermac/setup.c | 6 +--
> arch/powerpc/platforms/powermac/smp.c | 4 +-
> arch/powerpc/platforms/powermac/udbg_scc.c | 2 +-
> arch/powerpc/platforms/powernv/idle.c | 6 +--
> arch/powerpc/platforms/powernv/opal-core.c | 6 +--
> arch/powerpc/platforms/powernv/opal-fadump.c | 2 +-
> arch/powerpc/platforms/powernv/opal-msglog.c | 4 +-
> arch/powerpc/platforms/powernv/opal-power.c | 2 +-
> .../powerpc/platforms/powernv/opal-powercap.c | 2 +-
> arch/powerpc/platforms/powernv/opal-rtc.c | 2 +-
> .../platforms/powernv/opal-sensor-groups.c | 4 +-
> arch/powerpc/platforms/powernv/opal.c | 8 +--
> arch/powerpc/platforms/powernv/pci-ioda.c | 4 +-
> arch/powerpc/platforms/powernv/powernv.h | 4 +-
> arch/powerpc/platforms/powernv/rng.c | 2 +-
> arch/powerpc/platforms/powernv/setup.c | 6 +--
> arch/powerpc/platforms/ps3/gelic_udbg.c | 2 +-
> arch/powerpc/platforms/ps3/mm.c | 4 +-
> arch/powerpc/platforms/ps3/os-area.c | 4 +-
> arch/powerpc/platforms/ps3/platform.h | 14 ++---
> arch/powerpc/platforms/ps3/repository.c | 20 +++----
> arch/powerpc/platforms/ps3/smp.c | 2 +-
> arch/powerpc/platforms/ps3/spu.c | 2 +-
> .../powerpc/platforms/pseries/event_sources.c | 2 +-
> arch/powerpc/platforms/pseries/iommu.c | 2 +-
> arch/powerpc/platforms/pseries/lpar.c | 6 +--
> arch/powerpc/platforms/pseries/pseries.h | 2 +-
> arch/powerpc/platforms/pseries/rtas-fadump.c | 6 +--
> arch/powerpc/platforms/pseries/setup.c | 4 +-
> arch/powerpc/platforms/pseries/vas.c | 2 +-
> arch/powerpc/platforms/pseries/vio.c | 6 +--
> arch/powerpc/sysdev/cpm2.c | 6 +--
> arch/powerpc/sysdev/dart_iommu.c | 2 +-
> arch/powerpc/sysdev/fsl_mpic_err.c | 4 +-
> arch/powerpc/sysdev/fsl_pci.c | 2 +-
> arch/powerpc/sysdev/fsl_pci.h | 2 +-
> arch/powerpc/sysdev/i8259.c | 2 +-
> arch/powerpc/sysdev/ipic.c | 2 +-
> arch/powerpc/sysdev/mpic.c | 2 +-
> arch/powerpc/sysdev/mpic.h | 10 ++--
> arch/powerpc/sysdev/mpic_msi.c | 6 +--
> arch/powerpc/sysdev/mpic_timer.c | 6 +--
> arch/powerpc/sysdev/mpic_u3msi.c | 2 +-
> arch/powerpc/sysdev/tsi108_pci.c | 2 +-
> arch/powerpc/sysdev/udbg_memcons.c | 2 +-
> arch/powerpc/sysdev/xics/icp-hv.c | 2 +-
> arch/powerpc/sysdev/xics/icp-opal.c | 2 +-
> arch/powerpc/sysdev/xics/xics-common.c | 2 +-
> arch/powerpc/sysdev/xive/native.c | 4 +-
> arch/powerpc/sysdev/xive/spapr.c | 6 +--
> arch/powerpc/xmon/xmon.c | 2 +-
> include/linux/cuda.h | 2 +-
> include/linux/pmu.h | 2 +-
> 150 files changed, 334 insertions(+), 334 deletions(-)
>
More information about the Linuxppc-dev
mailing list