| Message ID | 20210122123244.34033-2-ganeshgr@linux.ibm.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v4,1/2] powerpc/mce: Reduce the size of event arrays | expand |
| Context | Check | Description |
|---|---|---|
| snowpatch_ozlabs/apply_patch | success | Successfully applied on branch powerpc/merge (41d8cb7ece7c81e4eb897ed7ec7d3c3d72fd0af4) |
| snowpatch_ozlabs/build-ppc64le | success | Build succeeded |
| snowpatch_ozlabs/build-ppc64be | success | Build succeeded |
| snowpatch_ozlabs/build-ppc64e | success | Build succeeded |
| snowpatch_ozlabs/build-pmac32 | fail | Build failed! |
| snowpatch_ozlabs/checkpatch | success | total: 0 errors, 0 warnings, 0 checks, 226 lines checked |
| snowpatch_ozlabs/needsstable | success | Patch has no Fixes tags |
Hi Ganesh,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on powerpc/next]
[also build test ERROR on v5.11-rc4 next-20210122]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Ganesh-Goudar/powerpc-mce-Reduce-the-size-of-event-arrays/20210124-191230
base: https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git next
config: powerpc-randconfig-r005-20210124 (attached as .config)
compiler: clang version 12.0.0 (https://github.com/llvm/llvm-project bd3a387ee76f58caa0d7901f3f84e9bb3d006f27)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# install powerpc cross compiling tool for clang build
# apt-get install binutils-powerpc-linux-gnu
# https://github.com/0day-ci/linux/commit/fab6401db419da33d1757ebf519f030ab758ae7a
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Ganesh-Goudar/powerpc-mce-Reduce-the-size-of-event-arrays/20210124-191230
git checkout fab6401db419da33d1757ebf519f030ab758ae7a
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=powerpc
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All errors (new ones prefixed by >>):
>> arch/powerpc/kernel/setup-common.c:940:2: error: implicit declaration of function 'mce_init' [-Werror,-Wimplicit-function-declaration]
mce_init();
^
1 error generated.
vim +/mce_init +940 arch/powerpc/kernel/setup-common.c
847
848 /*
849 * Called into from start_kernel this initializes memblock, which is used
850 * to manage page allocation until mem_init is called.
851 */
852 void __init setup_arch(char **cmdline_p)
853 {
854 kasan_init();
855
856 *cmdline_p = boot_command_line;
857
858 /* Set a half-reasonable default so udelay does something sensible */
859 loops_per_jiffy = 500000000 / HZ;
860
861 /* Unflatten the device-tree passed by prom_init or kexec */
862 unflatten_device_tree();
863
864 /*
865 * Initialize cache line/block info from device-tree (on ppc64) or
866 * just cputable (on ppc32).
867 */
868 initialize_cache_info();
869
870 /* Initialize RTAS if available. */
871 rtas_initialize();
872
873 /* Check if we have an initrd provided via the device-tree. */
874 check_for_initrd();
875
876 /* Probe the machine type, establish ppc_md. */
877 probe_machine();
878
879 /* Setup panic notifier if requested by the platform. */
880 setup_panic();
881
882 /*
883 * Configure ppc_md.power_save (ppc32 only, 64-bit machines do
884 * it from their respective probe() function.
885 */
886 setup_power_save();
887
888 /* Discover standard serial ports. */
889 find_legacy_serial_ports();
890
891 /* Register early console with the printk subsystem. */
892 register_early_udbg_console();
893
894 /* Setup the various CPU maps based on the device-tree. */
895 smp_setup_cpu_maps();
896
897 /* Initialize xmon. */
898 xmon_setup();
899
900 /* Check the SMT related command line arguments (ppc64). */
901 check_smt_enabled();
902
903 /* Parse memory topology */
904 mem_topology_setup();
905
906 /*
907 * Release secondary cpus out of their spinloops at 0x60 now that
908 * we can map physical -> logical CPU ids.
909 *
910 * Freescale Book3e parts spin in a loop provided by firmware,
911 * so smp_release_cpus() does nothing for them.
912 */
913 #ifdef CONFIG_SMP
914 smp_setup_pacas();
915
916 /* On BookE, setup per-core TLB data structures. */
917 setup_tlb_core_data();
918 #endif
919 /* Print various info about the machine that has been gathered so far. */
920 print_system_info();
921
922 /* Reserve large chunks of memory for use by CMA for KVM. */
923 kvm_cma_reserve();
924
925 /* Reserve large chunks of memory for us by CMA for hugetlb */
926 gigantic_hugetlb_cma_reserve();
927
928 klp_init_thread_info(&init_task);
929
930 init_mm.start_code = (unsigned long)_stext;
931 init_mm.end_code = (unsigned long) _etext;
932 init_mm.end_data = (unsigned long) _edata;
933 init_mm.brk = klimit;
934
935 mm_iommu_init(&init_mm);
936 irqstack_early_init();
937 exc_lvl_early_init();
938 emergency_stack_init();
939
> 940 mce_init();
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
Le 22/01/2021 à 13:32, Ganesh Goudar a écrit : > Access to per-cpu variables requires translation to be enabled on > pseries machine running in hash mmu mode, Since part of MCE handler > runs in realmode and part of MCE handling code is shared between ppc > architectures pseries and powernv, it becomes difficult to manage > these variables differently on different architectures, So have > these variables in paca instead of having them as per-cpu variables > to avoid complications. > > Signed-off-by: Ganesh Goudar <ganeshgr@linux.ibm.com> > --- > v2: Dynamically allocate memory for machine check event info > > v3: Remove check for hash mmu lpar, use memblock_alloc_try_nid > to allocate memory. > > v4: Spliting the patch into two. > --- > arch/powerpc/include/asm/mce.h | 18 +++++++ > arch/powerpc/include/asm/paca.h | 4 ++ > arch/powerpc/kernel/mce.c | 79 ++++++++++++++++++------------ > arch/powerpc/kernel/setup-common.c | 2 +- > 4 files changed, 70 insertions(+), 33 deletions(-) > > diff --git a/arch/powerpc/kernel/setup-common.c b/arch/powerpc/kernel/setup-common.c > index 71f38e9248be..17dc451f0e45 100644 > --- a/arch/powerpc/kernel/setup-common.c > +++ b/arch/powerpc/kernel/setup-common.c > @@ -916,7 +916,6 @@ void __init setup_arch(char **cmdline_p) > /* On BookE, setup per-core TLB data structures. */ > setup_tlb_core_data(); > #endif > - This line removal is really required for this patch ? > /* Print various info about the machine that has been gathered so far. */ > print_system_info(); > > @@ -938,6 +937,7 @@ void __init setup_arch(char **cmdline_p) > exc_lvl_early_init(); > emergency_stack_init(); > > + mce_init(); You have to include mce.h to avoid build failure on PPC32. > smp_release_cpus(); > > initmem_init(); >
On 1/25/21 2:54 PM, Christophe Leroy wrote: > > > Le 22/01/2021 à 13:32, Ganesh Goudar a écrit : >> Access to per-cpu variables requires translation to be enabled on >> pseries machine running in hash mmu mode, Since part of MCE handler >> runs in realmode and part of MCE handling code is shared between ppc >> architectures pseries and powernv, it becomes difficult to manage >> these variables differently on different architectures, So have >> these variables in paca instead of having them as per-cpu variables >> to avoid complications. >> >> Signed-off-by: Ganesh Goudar <ganeshgr@linux.ibm.com> >> --- >> v2: Dynamically allocate memory for machine check event info >> >> v3: Remove check for hash mmu lpar, use memblock_alloc_try_nid >> to allocate memory. >> >> v4: Spliting the patch into two. >> --- >> arch/powerpc/include/asm/mce.h | 18 +++++++ >> arch/powerpc/include/asm/paca.h | 4 ++ >> arch/powerpc/kernel/mce.c | 79 ++++++++++++++++++------------ >> arch/powerpc/kernel/setup-common.c | 2 +- >> 4 files changed, 70 insertions(+), 33 deletions(-) >> >> diff --git a/arch/powerpc/kernel/setup-common.c >> b/arch/powerpc/kernel/setup-common.c >> index 71f38e9248be..17dc451f0e45 100644 >> --- a/arch/powerpc/kernel/setup-common.c >> +++ b/arch/powerpc/kernel/setup-common.c >> @@ -916,7 +916,6 @@ void __init setup_arch(char **cmdline_p) >> /* On BookE, setup per-core TLB data structures. */ >> setup_tlb_core_data(); >> #endif >> - > > This line removal is really required for this patch ? I will correct it, Thanks for catching. > >> /* Print various info about the machine that has been gathered >> so far. */ >> print_system_info(); >> @@ -938,6 +937,7 @@ void __init setup_arch(char **cmdline_p) >> exc_lvl_early_init(); >> emergency_stack_init(); >> + mce_init(); > > You have to include mce.h to avoid build failure on PPC32. Sure, thanks >> smp_release_cpus(); >> initmem_init(); >>
diff --git a/arch/powerpc/include/asm/mce.h b/arch/powerpc/include/asm/mce.h index 7d8b6679ec68..331d944280b8 100644 --- a/arch/powerpc/include/asm/mce.h +++ b/arch/powerpc/include/asm/mce.h @@ -206,6 +206,17 @@ struct mce_error_info { #define MAX_MC_EVT 10 +struct mce_info { + int mce_nest_count; + struct machine_check_event mce_event[MAX_MC_EVT]; + /* Queue for delayed MCE events. */ + int mce_queue_count; + struct machine_check_event mce_event_queue[MAX_MC_EVT]; + /* Queue for delayed MCE UE events. */ + int mce_ue_count; + struct machine_check_event mce_ue_event_queue[MAX_MC_EVT]; +}; + /* Release flags for get_mce_event() */ #define MCE_EVENT_RELEASE true #define MCE_EVENT_DONTRELEASE false @@ -234,4 +245,11 @@ long __machine_check_early_realmode_p8(struct pt_regs *regs); long __machine_check_early_realmode_p9(struct pt_regs *regs); long __machine_check_early_realmode_p10(struct pt_regs *regs); #endif /* CONFIG_PPC_BOOK3S_64 */ + +#ifdef CONFIG_PPC_BOOK3S_64 +void mce_init(void); +#else +static inline void mce_init(void) { }; +#endif /* CONFIG_PPC_BOOK3S_64 */ + #endif /* __ASM_PPC64_MCE_H__ */ diff --git a/arch/powerpc/include/asm/paca.h b/arch/powerpc/include/asm/paca.h index 9454d29ff4b4..38e0c55e845d 100644 --- a/arch/powerpc/include/asm/paca.h +++ b/arch/powerpc/include/asm/paca.h @@ -29,6 +29,7 @@ #include <asm/hmi.h> #include <asm/cpuidle.h> #include <asm/atomic.h> +#include <asm/mce.h> #include <asm-generic/mmiowb_types.h> @@ -273,6 +274,9 @@ struct paca_struct { #ifdef CONFIG_MMIOWB struct mmiowb_state mmiowb_state; #endif +#ifdef CONFIG_PPC_BOOK3S_64 + struct mce_info *mce_info; +#endif /* CONFIG_PPC_BOOK3S_64 */ } ____cacheline_aligned; extern void copy_mm_to_paca(struct mm_struct *mm); diff --git a/arch/powerpc/kernel/mce.c b/arch/powerpc/kernel/mce.c index 9f3e133b57b7..6ec5c68997ed 100644 --- a/arch/powerpc/kernel/mce.c +++ b/arch/powerpc/kernel/mce.c @@ -17,22 +17,13 @@ #include <linux/irq_work.h> #include <linux/extable.h> #include <linux/ftrace.h> +#include <linux/memblock.h> #include <asm/machdep.h> #include <asm/mce.h> #include <asm/nmi.h> -static DEFINE_PER_CPU(int, mce_nest_count); -static DEFINE_PER_CPU(struct machine_check_event[MAX_MC_EVT], mce_event); - -/* Queue for delayed MCE events. */ -static DEFINE_PER_CPU(int, mce_queue_count); -static DEFINE_PER_CPU(struct machine_check_event[MAX_MC_EVT], mce_event_queue); - -/* Queue for delayed MCE UE events. */ -static DEFINE_PER_CPU(int, mce_ue_count); -static DEFINE_PER_CPU(struct machine_check_event[MAX_MC_EVT], - mce_ue_event_queue); +#include "setup.h" static void machine_check_process_queued_event(struct irq_work *work); static void machine_check_ue_irq_work(struct irq_work *work); @@ -103,9 +94,10 @@ void save_mce_event(struct pt_regs *regs, long handled, struct mce_error_info *mce_err, uint64_t nip, uint64_t addr, uint64_t phys_addr) { - int index = __this_cpu_inc_return(mce_nest_count) - 1; - struct machine_check_event *mce = this_cpu_ptr(&mce_event[index]); + int index = local_paca->mce_info->mce_nest_count++; + struct machine_check_event *mce; + mce = &local_paca->mce_info->mce_event[index]; /* * Return if we don't have enough space to log mce event. * mce_nest_count may go beyond MAX_MC_EVT but that's ok, @@ -191,7 +183,7 @@ void save_mce_event(struct pt_regs *regs, long handled, */ int get_mce_event(struct machine_check_event *mce, bool release) { - int index = __this_cpu_read(mce_nest_count) - 1; + int index = local_paca->mce_info->mce_nest_count - 1; struct machine_check_event *mc_evt; int ret = 0; @@ -201,7 +193,7 @@ int get_mce_event(struct machine_check_event *mce, bool release) /* Check if we have MCE info to process. */ if (index < MAX_MC_EVT) { - mc_evt = this_cpu_ptr(&mce_event[index]); + mc_evt = &local_paca->mce_info->mce_event[index]; /* Copy the event structure and release the original */ if (mce) *mce = *mc_evt; @@ -211,7 +203,7 @@ int get_mce_event(struct machine_check_event *mce, bool release) } /* Decrement the count to free the slot. */ if (release) - __this_cpu_dec(mce_nest_count); + local_paca->mce_info->mce_nest_count--; return ret; } @@ -233,13 +225,14 @@ static void machine_check_ue_event(struct machine_check_event *evt) { int index; - index = __this_cpu_inc_return(mce_ue_count) - 1; + index = local_paca->mce_info->mce_ue_count++; /* If queue is full, just return for now. */ if (index >= MAX_MC_EVT) { - __this_cpu_dec(mce_ue_count); + local_paca->mce_info->mce_ue_count--; return; } - memcpy(this_cpu_ptr(&mce_ue_event_queue[index]), evt, sizeof(*evt)); + memcpy(&local_paca->mce_info->mce_ue_event_queue[index], + evt, sizeof(*evt)); /* Queue work to process this event later. */ irq_work_queue(&mce_ue_event_irq_work); @@ -256,13 +249,14 @@ void machine_check_queue_event(void) if (!get_mce_event(&evt, MCE_EVENT_RELEASE)) return; - index = __this_cpu_inc_return(mce_queue_count) - 1; + index = local_paca->mce_info->mce_queue_count++; /* If queue is full, just return for now. */ if (index >= MAX_MC_EVT) { - __this_cpu_dec(mce_queue_count); + local_paca->mce_info->mce_queue_count--; return; } - memcpy(this_cpu_ptr(&mce_event_queue[index]), &evt, sizeof(evt)); + memcpy(&local_paca->mce_info->mce_event_queue[index], + &evt, sizeof(evt)); /* Queue irq work to process this event later. */ irq_work_queue(&mce_event_process_work); @@ -289,9 +283,9 @@ static void machine_process_ue_event(struct work_struct *work) int index; struct machine_check_event *evt; - while (__this_cpu_read(mce_ue_count) > 0) { - index = __this_cpu_read(mce_ue_count) - 1; - evt = this_cpu_ptr(&mce_ue_event_queue[index]); + while (local_paca->mce_info->mce_ue_count > 0) { + index = local_paca->mce_info->mce_ue_count - 1; + evt = &local_paca->mce_info->mce_ue_event_queue[index]; blocking_notifier_call_chain(&mce_notifier_list, 0, evt); #ifdef CONFIG_MEMORY_FAILURE /* @@ -304,7 +298,7 @@ static void machine_process_ue_event(struct work_struct *work) */ if (evt->error_type == MCE_ERROR_TYPE_UE) { if (evt->u.ue_error.ignore_event) { - __this_cpu_dec(mce_ue_count); + local_paca->mce_info->mce_ue_count--; continue; } @@ -320,7 +314,7 @@ static void machine_process_ue_event(struct work_struct *work) "was generated\n"); } #endif - __this_cpu_dec(mce_ue_count); + local_paca->mce_info->mce_ue_count--; } } /* @@ -338,17 +332,17 @@ static void machine_check_process_queued_event(struct irq_work *work) * For now just print it to console. * TODO: log this error event to FSP or nvram. */ - while (__this_cpu_read(mce_queue_count) > 0) { - index = __this_cpu_read(mce_queue_count) - 1; - evt = this_cpu_ptr(&mce_event_queue[index]); + while (local_paca->mce_info->mce_queue_count > 0) { + index = local_paca->mce_info->mce_queue_count - 1; + evt = &local_paca->mce_info->mce_event_queue[index]; if (evt->error_type == MCE_ERROR_TYPE_UE && evt->u.ue_error.ignore_event) { - __this_cpu_dec(mce_queue_count); + local_paca->mce_info->mce_queue_count--; continue; } machine_check_print_event_info(evt, false, false); - __this_cpu_dec(mce_queue_count); + local_paca->mce_info->mce_queue_count--; } } @@ -741,3 +735,24 @@ long hmi_exception_realmode(struct pt_regs *regs) return 1; } + +void __init mce_init(void) +{ + struct mce_info *mce_info; + u64 limit; + int i; + + limit = min(ppc64_bolted_size(), ppc64_rma_size); + for_each_possible_cpu(i) { + mce_info = memblock_alloc_try_nid(sizeof(*mce_info), + __alignof__(*mce_info), + MEMBLOCK_LOW_LIMIT, + limit, cpu_to_node(i)); + if (!mce_info) + goto err; + paca_ptrs[i]->mce_info = mce_info; + } + return; +err: + panic("Failed to allocate memory for MCE event data\n"); +} diff --git a/arch/powerpc/kernel/setup-common.c b/arch/powerpc/kernel/setup-common.c index 71f38e9248be..17dc451f0e45 100644 --- a/arch/powerpc/kernel/setup-common.c +++ b/arch/powerpc/kernel/setup-common.c @@ -916,7 +916,6 @@ void __init setup_arch(char **cmdline_p) /* On BookE, setup per-core TLB data structures. */ setup_tlb_core_data(); #endif - /* Print various info about the machine that has been gathered so far. */ print_system_info(); @@ -938,6 +937,7 @@ void __init setup_arch(char **cmdline_p) exc_lvl_early_init(); emergency_stack_init(); + mce_init(); smp_release_cpus(); initmem_init();
Access to per-cpu variables requires translation to be enabled on pseries machine running in hash mmu mode, Since part of MCE handler runs in realmode and part of MCE handling code is shared between ppc architectures pseries and powernv, it becomes difficult to manage these variables differently on different architectures, So have these variables in paca instead of having them as per-cpu variables to avoid complications. Signed-off-by: Ganesh Goudar <ganeshgr@linux.ibm.com> --- v2: Dynamically allocate memory for machine check event info v3: Remove check for hash mmu lpar, use memblock_alloc_try_nid to allocate memory. v4: Spliting the patch into two. --- arch/powerpc/include/asm/mce.h | 18 +++++++ arch/powerpc/include/asm/paca.h | 4 ++ arch/powerpc/kernel/mce.c | 79 ++++++++++++++++++------------ arch/powerpc/kernel/setup-common.c | 2 +- 4 files changed, 70 insertions(+), 33 deletions(-)