[PATCH] tty: serial: Fix refcount leak bug in ucc_uart.c
In soc_info(), of_find_node_by_type() will return a node pointer with refcount incremented. We should use of_node_put() when it is not used anymore. Signed-off-by: Liang He --- drivers/tty/serial/ucc_uart.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/tty/serial/ucc_uart.c b/drivers/tty/serial/ucc_uart.c index 6000853973c1..3cc9ef08455c 100644 --- a/drivers/tty/serial/ucc_uart.c +++ b/drivers/tty/serial/ucc_uart.c @@ -1137,6 +1137,8 @@ static unsigned int soc_info(unsigned int *rev_h, unsigned int *rev_l) /* No compatible property, so try the name. */ soc_string = np->name; + of_node_put(np); + /* Extract the SOC number from the "PowerPC," string */ if ((sscanf(soc_string, "PowerPC,%u", &soc) != 1) || !soc) return 0; -- 2.25.1
Re: [RFC PATCH v2 0/7] objtool: Enable and implement --mcount option on powerpc
Hi Christophe, On 15/06/22 21:33, Christophe Leroy wrote: Do you have any idea when you plan to send next revision ? I'm really looking forward to submitting the inline static calls on top of your series. I'm planning to send RFC v3 next week. - Sathvika
[PATCH v2] powerpc: embedded6xx: Fix refcount leak bugs
In xx_init_xx(), of_find_node_by_type() will return a node pointer with refcount incremented. We should use of_node_put() when it is not used anymore. Signed-off-by: Liang He --- v2: we merge all embedded6xx related bugs into one commit v1: we only report the bug in holly_init_pci() of holly.c arch/powerpc/platforms/embedded6xx/holly.c| 6 ++ arch/powerpc/platforms/embedded6xx/mpc7448_hpc2.c | 3 +++ 2 files changed, 9 insertions(+) diff --git a/arch/powerpc/platforms/embedded6xx/holly.c b/arch/powerpc/platforms/embedded6xx/holly.c index 78f2378d9223..bebc5a972694 100644 --- a/arch/powerpc/platforms/embedded6xx/holly.c +++ b/arch/powerpc/platforms/embedded6xx/holly.c @@ -123,6 +123,8 @@ static void __init holly_init_pci(void) if (np) tsi108_setup_pci(np, HOLLY_PCI_CFG_PHYS, 1); + of_node_put(np); + ppc_md.pci_exclude_device = holly_exclude_device; if (ppc_md.progress) ppc_md.progress("tsi108: resources set", 0x100); @@ -184,6 +186,9 @@ static void __init holly_init_IRQ(void) tsi108_pci_int_init(cascade_node); irq_set_handler_data(cascade_pci_irq, mpic); irq_set_chained_handler(cascade_pci_irq, tsi108_irq_cascade); + + of_node_put(tsi_pci); + of_node_put(cascade_node); #endif /* Configure MPIC outputs to CPU0 */ tsi108_write_reg(TSI108_MPIC_OFFSET + 0x30c, 0); @@ -210,6 +215,7 @@ static void __noreturn holly_restart(char *cmd) if (bridge) { prop = of_get_property(bridge, "reg", &size); addr = of_translate_address(bridge, prop); + of_node_put(bridge); } addr += (TSI108_PB_OFFSET + 0x414); diff --git a/arch/powerpc/platforms/embedded6xx/mpc7448_hpc2.c b/arch/powerpc/platforms/embedded6xx/mpc7448_hpc2.c index 8b2b42210356..ddf0c652af80 100644 --- a/arch/powerpc/platforms/embedded6xx/mpc7448_hpc2.c +++ b/arch/powerpc/platforms/embedded6xx/mpc7448_hpc2.c @@ -135,6 +135,9 @@ static void __init mpc7448_hpc2_init_IRQ(void) tsi108_pci_int_init(cascade_node); irq_set_handler_data(cascade_pci_irq, mpic); irq_set_chained_handler(cascade_pci_irq, tsi108_irq_cascade); + + of_node_put(tsi_pci); + of_node_put(cascade_node); #endif /* Configure MPIC outputs to CPU0 */ tsi108_write_reg(TSI108_MPIC_OFFSET + 0x30c, 0); -- 2.25.1
[PATCH] powerpc: embedded6xx: Fix refcount leak bug in holly.c
In holly_init_pci(), of_find_node_by_type() will return a node pointer with refcount incremented. We should use of_node_put() when it is not used anymore. Signed-off-by: Liang He --- arch/powerpc/platforms/embedded6xx/holly.c | 1 + 1 file changed, 1 insertion(+) diff --git a/arch/powerpc/platforms/embedded6xx/holly.c b/arch/powerpc/platforms/embedded6xx/holly.c index 78f2378d9223..a4938505e89a 100644 --- a/arch/powerpc/platforms/embedded6xx/holly.c +++ b/arch/powerpc/platforms/embedded6xx/holly.c @@ -122,6 +122,7 @@ static void __init holly_init_pci(void) np = of_find_node_by_type(NULL, "pci"); if (np) tsi108_setup_pci(np, HOLLY_PCI_CFG_PHYS, 1); + of_node_put(np); ppc_md.pci_exclude_device = holly_exclude_device; if (ppc_md.progress) -- 2.25.1
Re: [PATCH 1/4] hugetlb: skip to end of PT page mapping when pte not present
On 6/18/2022 1:17 AM, Mike Kravetz wrote: On 06/17/22 10:15, Peter Xu wrote: Hi, Mike, On Thu, Jun 16, 2022 at 02:05:15PM -0700, Mike Kravetz wrote: @@ -6877,6 +6896,39 @@ pte_t *huge_pte_offset(struct mm_struct *mm, return (pte_t *)pmd; } +/* + * Return a mask that can be used to update an address to the last huge + * page in a page table page mapping size. Used to skip non-present + * page table entries when linearly scanning address ranges. Architectures + * with unique huge page to page table relationships can define their own + * version of this routine. + */ +unsigned long hugetlb_mask_last_page(struct hstate *h) +{ + unsigned long hp_size = huge_page_size(h); + + switch (hp_size) { + case P4D_SIZE: + return PGDIR_SIZE - P4D_SIZE; + case PUD_SIZE: + return P4D_SIZE - PUD_SIZE; + case PMD_SIZE: + return PUD_SIZE - PMD_SIZE; + default: Should we add a WARN_ON_ONCE() if it should never trigger? Sure. I will add this. + break; /* Should never happen */ + } + + return ~(0UL); +} + +#else + +/* See description above. Architectures can provide their own version. */ +__weak unsigned long hugetlb_mask_last_page(struct hstate *h) +{ + return ~(0UL); I'm wondering whether it's better to return 0 rather than ~0 by default. Could an arch with !CONFIG_ARCH_WANT_GENERAL_HUGETLB wrongly skip some valid address ranges with ~0, or perhaps I misread? Thank you, thank you, thank you Peter! Yes, the 'default' return for hugetlb_mask_last_page() should be 0. If there is no 'optimization', we do not want to modify the address so we want to OR with 0 not ~0. My bad, I must have been thinking AND instead of OR. I will change here as well as in Baolin's patch. Ah, I also overlooked this. Thanks Peter, and thanks Mike for updating.
[PATCH] powerpc: sysdev: Fix refcount leak bug in fsl_pci.c
In is_kdump(), we need a of_node_put() to dec the refcount which is incremented by of_find_node_by_type(). Signed-off-by: Liang He --- arch/powerpc/sysdev/fsl_pci.c | 6 +- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/arch/powerpc/sysdev/fsl_pci.c b/arch/powerpc/sysdev/fsl_pci.c index 1011cfea2e32..4c986c955951 100644 --- a/arch/powerpc/sysdev/fsl_pci.c +++ b/arch/powerpc/sysdev/fsl_pci.c @@ -180,6 +180,7 @@ static int setup_one_atmu(struct ccsr_pci __iomem *pci, static bool is_kdump(void) { struct device_node *node; + bool ret; node = of_find_node_by_type(NULL, "memory"); if (!node) { @@ -187,7 +188,10 @@ static bool is_kdump(void) return false; } - return of_property_read_bool(node, "linux,usable-memory"); + ret = of_property_read_bool(node, "linux,usable-memory"); + of_node_put(node); + + return ret; } /* atmu setup for fsl pci/pcie controller */ -- 2.25.1
[PATCH] powerpc: 8xx: Fix refcount leak bug in tqm8xx_setup
In init_ioports(), of_find_node_by_name() will return a node pointer with refcount incremented. We should use of_node_put() when it is not used anymore. Signed-off-by: Liang He --- arch/powerpc/platforms/8xx/tqm8xx_setup.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/arch/powerpc/platforms/8xx/tqm8xx_setup.c b/arch/powerpc/platforms/8xx/tqm8xx_setup.c index 3725d51248df..ffcfd17a5fa3 100644 --- a/arch/powerpc/platforms/8xx/tqm8xx_setup.c +++ b/arch/powerpc/platforms/8xx/tqm8xx_setup.c @@ -105,6 +105,9 @@ static void __init init_ioports(void) if (dnode == NULL) return; prop = of_find_property(dnode, "ethernet1", &len); + + of_node_put(dnode); + if (prop == NULL) return; -- 2.25.1
[PATCH] cpufreq: pmac32-cpufreq: Fix refcount leak bug
In pmac_cpufreq_init_MacRISC3(), we need to add corresponding of_node_put() for the three node pointers whose refcount have been incremented by of_find_node_by_name(). Signed-off-by: Liang He --- drivers/cpufreq/pmac32-cpufreq.c | 4 1 file changed, 4 insertions(+) diff --git a/drivers/cpufreq/pmac32-cpufreq.c b/drivers/cpufreq/pmac32-cpufreq.c index 20f64a8b0a35..4b8ee2014da6 100644 --- a/drivers/cpufreq/pmac32-cpufreq.c +++ b/drivers/cpufreq/pmac32-cpufreq.c @@ -470,6 +470,10 @@ static int pmac_cpufreq_init_MacRISC3(struct device_node *cpunode) if (slew_done_gpio_np) slew_done_gpio = read_gpio(slew_done_gpio_np); + of_node_put(volt_gpio_np); + of_node_put(freq_gpio_np); + of_node_put(slew_done_gpio_np); + /* If we use the frequency GPIOs, calculate the min/max speeds based * on the bus frequencies */ -- 2.25.1
[PATCH] powerpc: powernv: opal-core: Fix refcount leak bug
In create_opalcore(), of_find_node_by_name() will return a node pointer with refcount incremented. We should use of_node_put() when it is not used anymore. Signed-off-by: Liang He --- arch/powerpc/platforms/powernv/opal-core.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/arch/powerpc/platforms/powernv/opal-core.c b/arch/powerpc/platforms/powernv/opal-core.c index adcb1a1a2bfe..bb7657115f1d 100644 --- a/arch/powerpc/platforms/powernv/opal-core.c +++ b/arch/powerpc/platforms/powernv/opal-core.c @@ -348,6 +348,8 @@ static int __init create_opalcore(void) if (!dn || ret) pr_warn("WARNING: Failed to read OPAL base & entry values\n"); + of_node_put(dn); + /* Use count to keep track of the program headers */ count = 0; -- 2.25.1
Re: [PATCH v2 3/3] arch/*/: remove CONFIG_VIRT_TO_BUS
Arnd, Am 18.06.2022 um 00:57 schrieb Arnd Bergmann: From: Arnd Bergmann All architecture-independent users of virt_to_bus() and bus_to_virt() have been fixed to use the dma mapping interfaces or have been removed now. This means the definitions on most architectures, and the CONFIG_VIRT_TO_BUS symbol are now obsolete and can be removed. The only exceptions to this are a few network and scsi drivers for m68k Amiga and VME machines and ppc32 Macintosh. These drivers work correctly with the old interfaces and are probably not worth changing. The Amiga SCSI drivers are all old WD33C93 ones, and replacing virt_to_bus by virt_to_phys in the dma_setup() function there would cause no functional change at all. drivers/vme/bridges/vme_ca91cx42.c hasn't been used at all on m68k (it is a PCI-to-VME bridge chipset driver that would be needed on architectures that natively use a PCI bus). I haven't found anything that selects that driver, so not sure it is even still in use?? That would allow you to drop the remaining virt_to_bus define from arch/m68k/include/asm/virtconvert.h. I could submit a patch to convert the Amiga SCSI drivers to use virt_to_phys if Geert and the SCSI maintainers think it's worth the churn. 32bit powerpc is a different matter though. Cheers, Michael
Re: [PATCH 1/4] hugetlb: skip to end of PT page mapping when pte not present
On 06/17/22 19:26, kernel test robot wrote: > Hi Mike, > > I love your patch! Yet something to improve: > > [auto build test ERROR on soc/for-next] > [also build test ERROR on linus/master v5.19-rc2 next-20220617] > [cannot apply to arm64/for-next/core arm/for-next kvmarm/next > xilinx-xlnx/master] > [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/intel-lab-lkp/linux/commits/Mike-Kravetz/hugetlb-speed-up-linear-address-scanning/20220617-050726 > base: https://git.kernel.org/pub/scm/linux/kernel/git/soc/soc.git for-next > config: i386-randconfig-a002 > (https://download.01.org/0day-ci/archive/20220617/202206171929.ziurng6p-...@intel.com/config) > compiler: clang version 15.0.0 (https://github.com/llvm/llvm-project > f0e608de27b3d56846eebf3712ab542979d6) > 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 > # > https://github.com/intel-lab-lkp/linux/commit/4c647687607f10fece04967b8180c0dadaf765e6 > git remote add linux-review https://github.com/intel-lab-lkp/linux > git fetch --no-tags linux-review > Mike-Kravetz/hugetlb-speed-up-linear-address-scanning/20220617-050726 > git checkout 4c647687607f10fece04967b8180c0dadaf765e6 > # save the config file > mkdir build_dir && cp config build_dir/.config > COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 > O=build_dir ARCH=i386 SHELL=/bin/bash > > If you fix the issue, kindly add following tag where applicable > Reported-by: kernel test robot > > All errors (new ones prefixed by >>): A couple of things here, > > >> mm/hugetlb.c:6901:7: error: duplicate case value '4194304' >case PUD_SIZE: > ^ >include/asm-generic/pgtable-nopud.h:20:20: note: expanded from macro > 'PUD_SIZE' >#define PUD_SIZE(1UL << PUD_SHIFT) >^ >mm/hugetlb.c:6899:7: note: previous case defined here >case P4D_SIZE: > ^ >include/asm-generic/pgtable-nop4d.h:13:19: note: expanded from macro > 'P4D_SIZE' >#define P4D_SIZE(1UL << P4D_SHIFT) In the CONFIG_ARCH_WANT_GENERAL_HUGETLB case covered by this version of hugetlb_mask_last_page, huge pages can only be PMD_SIZE or PUD_SIZE. So, the 'case P4D_SIZE:' should not exist and can be removed. >^ >mm/hugetlb.c:6903:7: error: duplicate case value '4194304' >case PMD_SIZE: > ^ >include/asm-generic/pgtable-nopmd.h:22:20: note: expanded from macro > 'PMD_SIZE' >#define PMD_SIZE(1UL << PMD_SHIFT) >^ On i386 with CONFIG_PGTABLE_LEVELS=2, PUD_SIZE == PMD_SIZE. Originally, I coded this as a if .. else if ... statement instead of a switch. If coded this way, the compiler does not complain about the duplicate values. The only other alternative I can think of is something like '#if CONFIG_PGTABLE_LEVELS > 2' around the PUD_SIZE case. I would prefer the if else if, unless someone can suggest something else? -- Mike Kravetz
Re: [PATCH 1/4] hugetlb: skip to end of PT page mapping when pte not present
Hi Mike, I love your patch! Yet something to improve: [auto build test ERROR on soc/for-next] [also build test ERROR on linus/master v5.19-rc2 next-20220617] [cannot apply to arm64/for-next/core arm/for-next kvmarm/next xilinx-xlnx/master] [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/intel-lab-lkp/linux/commits/Mike-Kravetz/hugetlb-speed-up-linear-address-scanning/20220617-050726 base: https://git.kernel.org/pub/scm/linux/kernel/git/soc/soc.git for-next config: i386-debian-10.3 (https://download.01.org/0day-ci/archive/20220618/202206180021.rcc4b1by-...@intel.com/config) compiler: gcc-11 (Debian 11.3.0-3) 11.3.0 reproduce (this is a W=1 build): # https://github.com/intel-lab-lkp/linux/commit/4c647687607f10fece04967b8180c0dadaf765e6 git remote add linux-review https://github.com/intel-lab-lkp/linux git fetch --no-tags linux-review Mike-Kravetz/hugetlb-speed-up-linear-address-scanning/20220617-050726 git checkout 4c647687607f10fece04967b8180c0dadaf765e6 # save the config file mkdir build_dir && cp config build_dir/.config make W=1 O=build_dir ARCH=i386 SHELL=/bin/bash If you fix the issue, kindly add following tag where applicable Reported-by: kernel test robot All errors (new ones prefixed by >>): mm/hugetlb.c: In function 'hugetlb_mask_last_page': >> mm/hugetlb.c:6901:9: error: duplicate case value 6901 | case PUD_SIZE: | ^~~~ mm/hugetlb.c:6899:9: note: previously used here 6899 | case P4D_SIZE: | ^~~~ vim +6901 mm/hugetlb.c 6886 6887 /* 6888 * Return a mask that can be used to update an address to the last huge 6889 * page in a page table page mapping size. Used to skip non-present 6890 * page table entries when linearly scanning address ranges. Architectures 6891 * with unique huge page to page table relationships can define their own 6892 * version of this routine. 6893 */ 6894 unsigned long hugetlb_mask_last_page(struct hstate *h) 6895 { 6896 unsigned long hp_size = huge_page_size(h); 6897 6898 switch (hp_size) { 6899 case P4D_SIZE: 6900 return PGDIR_SIZE - P4D_SIZE; > 6901 case PUD_SIZE: 6902 return P4D_SIZE - PUD_SIZE; 6903 case PMD_SIZE: 6904 return PUD_SIZE - PMD_SIZE; 6905 default: 6906 break; /* Should never happen */ 6907 } 6908 6909 return ~(0UL); 6910 } 6911 -- 0-DAY CI Kernel Test Service https://01.org/lkp
Re: [PATCH 1/4] hugetlb: skip to end of PT page mapping when pte not present
On 06/17/22 10:15, Peter Xu wrote: > Hi, Mike, > > On Thu, Jun 16, 2022 at 02:05:15PM -0700, Mike Kravetz wrote: > > @@ -6877,6 +6896,39 @@ pte_t *huge_pte_offset(struct mm_struct *mm, > > return (pte_t *)pmd; > > } > > > > +/* > > + * Return a mask that can be used to update an address to the last huge > > + * page in a page table page mapping size. Used to skip non-present > > + * page table entries when linearly scanning address ranges. Architectures > > + * with unique huge page to page table relationships can define their own > > + * version of this routine. > > + */ > > +unsigned long hugetlb_mask_last_page(struct hstate *h) > > +{ > > + unsigned long hp_size = huge_page_size(h); > > + > > + switch (hp_size) { > > + case P4D_SIZE: > > + return PGDIR_SIZE - P4D_SIZE; > > + case PUD_SIZE: > > + return P4D_SIZE - PUD_SIZE; > > + case PMD_SIZE: > > + return PUD_SIZE - PMD_SIZE; > > + default: > > Should we add a WARN_ON_ONCE() if it should never trigger? > Sure. I will add this. > > + break; /* Should never happen */ > > + } > > + > > + return ~(0UL); > > +} > > + > > +#else > > + > > +/* See description above. Architectures can provide their own version. */ > > +__weak unsigned long hugetlb_mask_last_page(struct hstate *h) > > +{ > > + return ~(0UL); > > I'm wondering whether it's better to return 0 rather than ~0 by default. > Could an arch with !CONFIG_ARCH_WANT_GENERAL_HUGETLB wrongly skip some > valid address ranges with ~0, or perhaps I misread? Thank you, thank you, thank you Peter! Yes, the 'default' return for hugetlb_mask_last_page() should be 0. If there is no 'optimization', we do not want to modify the address so we want to OR with 0 not ~0. My bad, I must have been thinking AND instead of OR. I will change here as well as in Baolin's patch. -- Mike Kravetz
Re: [PATCH 1/4] hugetlb: skip to end of PT page mapping when pte not present
Hi Mike, I love your patch! Yet something to improve: [auto build test ERROR on soc/for-next] [also build test ERROR on linus/master v5.19-rc2 next-20220617] [cannot apply to arm64/for-next/core arm/for-next kvmarm/next xilinx-xlnx/master] [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/intel-lab-lkp/linux/commits/Mike-Kravetz/hugetlb-speed-up-linear-address-scanning/20220617-050726 base: https://git.kernel.org/pub/scm/linux/kernel/git/soc/soc.git for-next config: i386-randconfig-a002 (https://download.01.org/0day-ci/archive/20220617/202206171929.ziurng6p-...@intel.com/config) compiler: clang version 15.0.0 (https://github.com/llvm/llvm-project f0e608de27b3d56846eebf3712ab542979d6) 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 # https://github.com/intel-lab-lkp/linux/commit/4c647687607f10fece04967b8180c0dadaf765e6 git remote add linux-review https://github.com/intel-lab-lkp/linux git fetch --no-tags linux-review Mike-Kravetz/hugetlb-speed-up-linear-address-scanning/20220617-050726 git checkout 4c647687607f10fece04967b8180c0dadaf765e6 # save the config file mkdir build_dir && cp config build_dir/.config COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=i386 SHELL=/bin/bash If you fix the issue, kindly add following tag where applicable Reported-by: kernel test robot All errors (new ones prefixed by >>): >> mm/hugetlb.c:6901:7: error: duplicate case value '4194304' case PUD_SIZE: ^ include/asm-generic/pgtable-nopud.h:20:20: note: expanded from macro 'PUD_SIZE' #define PUD_SIZE(1UL << PUD_SHIFT) ^ mm/hugetlb.c:6899:7: note: previous case defined here case P4D_SIZE: ^ include/asm-generic/pgtable-nop4d.h:13:19: note: expanded from macro 'P4D_SIZE' #define P4D_SIZE(1UL << P4D_SHIFT) ^ mm/hugetlb.c:6903:7: error: duplicate case value '4194304' case PMD_SIZE: ^ include/asm-generic/pgtable-nopmd.h:22:20: note: expanded from macro 'PMD_SIZE' #define PMD_SIZE(1UL << PMD_SHIFT) ^ mm/hugetlb.c:6901:7: note: previous case defined here case PUD_SIZE: ^ include/asm-generic/pgtable-nopud.h:20:20: note: expanded from macro 'PUD_SIZE' #define PUD_SIZE(1UL << PUD_SHIFT) ^ 2 errors generated. vim +/4194304 +6901 mm/hugetlb.c 6886 6887 /* 6888 * Return a mask that can be used to update an address to the last huge 6889 * page in a page table page mapping size. Used to skip non-present 6890 * page table entries when linearly scanning address ranges. Architectures 6891 * with unique huge page to page table relationships can define their own 6892 * version of this routine. 6893 */ 6894 unsigned long hugetlb_mask_last_page(struct hstate *h) 6895 { 6896 unsigned long hp_size = huge_page_size(h); 6897 6898 switch (hp_size) { 6899 case P4D_SIZE: 6900 return PGDIR_SIZE - P4D_SIZE; > 6901 case PUD_SIZE: 6902 return P4D_SIZE - PUD_SIZE; 6903 case PMD_SIZE: 6904 return PUD_SIZE - PMD_SIZE; 6905 default: 6906 break; /* Should never happen */ 6907 } 6908 6909 return ~(0UL); 6910 } 6911 -- 0-DAY CI Kernel Test Service https://01.org/lkp
[PATCH] KVM: PPC: use __func__ to get funcion's name in an output message
Prefer using '"%s...", __func__' to get current function's name in an output message. Signed-off-by: XueBing Chen --- arch/powerpc/kvm/book3s_64_mmu_hv.c | 7 +++ 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/arch/powerpc/kvm/book3s_64_mmu_hv.c b/arch/powerpc/kvm/book3s_64_mmu_hv.c index 0aeb51738ca9..e795ed3d1346 100644 --- a/arch/powerpc/kvm/book3s_64_mmu_hv.c +++ b/arch/powerpc/kvm/book3s_64_mmu_hv.c @@ -1194,7 +1194,7 @@ static int resize_hpt_allocate(struct kvm_resize_hpt *resize) if (rc < 0) return rc; - resize_hpt_debug(resize, "resize_hpt_allocate(): HPT @ 0x%lx\n", + resize_hpt_debug(resize, "%s(): HPT @ 0x%lx\n", __func__, resize->hpt.virt); return 0; @@ -1435,7 +1435,7 @@ static void resize_hpt_prepare_work(struct work_struct *work) */ mutex_unlock(&kvm->arch.mmu_setup_lock); - resize_hpt_debug(resize, "resize_hpt_prepare_work(): order = %d\n", + resize_hpt_debug(resize, "%s(): order = %d\n", __func__, resize->order); err = resize_hpt_allocate(resize); @@ -1879,8 +1879,7 @@ static ssize_t kvm_htab_write(struct file *file, const char __user *buf, ret = kvmppc_virtmode_do_h_enter(kvm, H_EXACT, i, v, r, tmp); if (ret != H_SUCCESS) { - pr_err("kvm_htab_write ret %ld i=%ld v=%lx " - "r=%lx\n", ret, i, v, r); + pr_err("%s ret %ld i=%ld v=%lx r=%lx\n", __func__, ret, i, v, r); goto out; } if (!mmu_ready && is_vrma_hpte(v)) { -- 2.36.1
Re: [PATCH 1/4] hugetlb: skip to end of PT page mapping when pte not present
Hi Peter, On Fri, Jun 17, 2022 at 4:22 PM Peter Xu wrote: > On Thu, Jun 16, 2022 at 02:05:15PM -0700, Mike Kravetz wrote: > > @@ -6877,6 +6896,39 @@ pte_t *huge_pte_offset(struct mm_struct *mm, > > return (pte_t *)pmd; > > } > > > > +/* > > + * Return a mask that can be used to update an address to the last huge > > + * page in a page table page mapping size. Used to skip non-present > > + * page table entries when linearly scanning address ranges. Architectures > > + * with unique huge page to page table relationships can define their own > > + * version of this routine. > > + */ > > +unsigned long hugetlb_mask_last_page(struct hstate *h) > > +{ > > + unsigned long hp_size = huge_page_size(h); > > + > > + switch (hp_size) { > > + case P4D_SIZE: > > + return PGDIR_SIZE - P4D_SIZE; > > + case PUD_SIZE: > > + return P4D_SIZE - PUD_SIZE; > > + case PMD_SIZE: > > + return PUD_SIZE - PMD_SIZE; > > + default: > > Should we add a WARN_ON_ONCE() if it should never trigger? And with panic_on_warn, it'll panic only once ;-) Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org 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
Re: [PATCH] KVM: PPC: Align pt_regs in kvm_vcpu_arch structure
Hi, Fabiano. On 5/25/22 09:49, Fabiano Rosas wrote: The H_ENTER_NESTED hypercall receives as second parameter the address of a region of memory containing the values for the nested guest privileged registers. We currently use the pt_regs structure contained within kvm_vcpu_arch for that end. Most hypercalls that receive a memory address expect that region to not cross a 4k page boundary. We would want H_ENTER_NESTED to follow the same pattern so this patch ensures the pt_regs structure sits within a page. Signed-off-by: Fabiano Rosas Is it necessary to explain in the commit message that even though the second parameter needs to be 4k-aligned, we're aligning pt_regs to 512 bytes so it can be placed within a 4k boundary because its size is below 512 bytes? The natural thinking would be aligning it to 4k bytes, which would punch a huge hole in kvm_vcpu_arch. I think having the explanation of why 512 vs. 4k is worthwhile mentioning. --- arch/powerpc/include/asm/kvm_host.h | 6 +- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/arch/powerpc/include/asm/kvm_host.h b/arch/powerpc/include/asm/kvm_host.h index faf301d0dec0..87eba60f2920 100644 --- a/arch/powerpc/include/asm/kvm_host.h +++ b/arch/powerpc/include/asm/kvm_host.h @@ -519,7 +519,11 @@ struct kvm_vcpu_arch { struct kvmppc_book3s_shadow_vcpu *shadow_vcpu; #endif - struct pt_regs regs; + /* +* This is passed along to the HV via H_ENTER_NESTED. Align to +* prevent it crossing a real 4K page. +*/ + struct pt_regs regs __aligned(512); struct thread_fp_state fp; -- Murilo
[PATCH] powerpc: sysdev: Fix refcount leak bug in mpic_msgr
In mpic_msgr_number_of_blocks() and mpic_msgr_block_number(), of_find_node_by_name() will return a node pointer with refcount incremented. We should use of_node_put() when it is not used anymore. Signed-off-by: Liang He --- arch/powerpc/sysdev/mpic_msgr.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/arch/powerpc/sysdev/mpic_msgr.c b/arch/powerpc/sysdev/mpic_msgr.c index 698fefaaa6dd..c2a235d9eb09 100644 --- a/arch/powerpc/sysdev/mpic_msgr.c +++ b/arch/powerpc/sysdev/mpic_msgr.c @@ -122,6 +122,7 @@ static unsigned int mpic_msgr_number_of_blocks(void) count += 1; } } + of_node_put(aliases); return count; } @@ -150,6 +151,7 @@ static int mpic_msgr_block_number(struct device_node *node) if (node == of_find_node_by_path(prop->value)) break; } + of_node_put(aliases); return index == number_of_blocks ? -1 : index; } -- 2.25.1
[PATCH powerpc] powerpc/bpf: Fix use of user_pt_regs in uapi
This is a partial revert of commit a6460b03f945ee ("powerpc/bpf: Fix broken uapi for BPF_PROG_TYPE_PERF_EVENT"). Unlike x86, powerpc has both pt_regs and user_pt_regs structures. As such, we still need to override perf_arch_bpf_user_pt_regs() so that the correct user_regs structure is used. However, unlike arm64 and s390, we expose user_pt_regs to userspace as just 'pt_regs'. Due to this, trying to #include throws the below error: /usr/include/linux/bpf_perf_event.h:14:28: error: field ‘regs’ has incomplete type 14 | bpf_user_pt_regs_t regs; |^~~~ Note that this was not showing up with the bpf selftest build since tools/include/uapi/asm/bpf_perf_event.h didn't include the powerpc variant. Fix this by removing arch/powerpc/include/uapi/asm/bpf_perf_event.h, allowing fallback to the asm-generic version. Fixes: a6460b03f945ee ("powerpc/bpf: Fix broken uapi for BPF_PROG_TYPE_PERF_EVENT") Cc: sta...@vger.kernel.org # v4.20+ Signed-off-by: Naveen N. Rao --- arch/powerpc/include/uapi/asm/bpf_perf_event.h | 9 - 1 file changed, 9 deletions(-) delete mode 100644 arch/powerpc/include/uapi/asm/bpf_perf_event.h diff --git a/arch/powerpc/include/uapi/asm/bpf_perf_event.h b/arch/powerpc/include/uapi/asm/bpf_perf_event.h deleted file mode 100644 index 5e1e648aeec4c8..00 --- a/arch/powerpc/include/uapi/asm/bpf_perf_event.h +++ /dev/null @@ -1,9 +0,0 @@ -/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */ -#ifndef _UAPI__ASM_BPF_PERF_EVENT_H__ -#define _UAPI__ASM_BPF_PERF_EVENT_H__ - -#include - -typedef struct user_pt_regs bpf_user_pt_regs_t; - -#endif /* _UAPI__ASM_BPF_PERF_EVENT_H__ */ base-commit: bcd1c02813b8ab4ae019c65ffb716c9f579868e7 -- 2.36.1
Re:Re: [PATCH] powerpc: powernv: Fix refcount leak bug in opal-powercap
At 2022-06-17 13:01:27, "Christophe JAILLET" wrote: >Le 17/06/2022 à 06:20, Liang He a écrit : >> In opal_powercap_init(), of_find_compatible_node() will return >> a node pointer with refcount incremented. We should use of_node_put() >> in fail path or when it is not used anymore. >> >> Besides, for_each_child_of_node() will automatically *inc* and *dec* >> refcount during iteration. However, we should add the of_node_put() >> if there is a break. > >Hi, > >I'm not sure that your patch is right here. Because of this *inc* and >*dec* things, do we still need to of_node_put(powercap) once we have >entered for_each_child_of_node? > >I think that this reference will be released on the first iteration of >the loop. > > >Maybe of_node_put(powercap) should be duplicated everywhere it is >relevant and removed from the error handling path? >Or an additional reference should be taken before the loop? >Or adding a new label with "powercap = NULL" and branching there when >needed? > >CJ > >> >> Signed-off-by: Liang He >> --- >> arch/powerpc/platforms/powernv/opal-powercap.c | 5 - >> 1 file changed, 4 insertions(+), 1 deletion(-) >> >> diff --git a/arch/powerpc/platforms/powernv/opal-powercap.c >> b/arch/powerpc/platforms/powernv/opal-powercap.c >> index 64506b46e77b..b102477d3f95 100644 >> --- a/arch/powerpc/platforms/powernv/opal-powercap.c >> +++ b/arch/powerpc/platforms/powernv/opal-powercap.c >> @@ -153,7 +153,7 @@ void __init opal_powercap_init(void) >> pcaps = kcalloc(of_get_child_count(powercap), sizeof(*pcaps), >> GFP_KERNEL); >> if (!pcaps) >> -return; >> +goto out_powercap; >> >> powercap_kobj = kobject_create_and_add("powercap", opal_kobj); >> if (!powercap_kobj) { >> @@ -236,6 +236,9 @@ void __init opal_powercap_init(void) >> kfree(pcaps[i].pg.name); >> } >> kobject_put(powercap_kobj); >> +of_node_put(node); >> out_pcaps: >> kfree(pcaps); >> +out_powercap: >> +of_node_put(powercap); >> } Hi, CJ. I think my patch is correct based on the old commit: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?h=v5.19-rc2&id=09700c504d8e63faffd2a2235074e8c5d130cb8f Bugs and fix solutions in this 09700c504d8e63-commit are very similar with mine. Besides, I also find similar new bugs in other two files in the same directory 'powernv', so I have merged all three files' patches into one commit. '[PATCH v2] powerpc: powernv: Fix refcount leak bug'. Thanks. Liang
[PATCH v2] powerpc: powernv: Fix refcount leak bug
In these driver init functions, there are two kinds of errors: (1) missing of_put_node() for of_find_compatible_node()'s returned pointer (refcount incremented) in fail path or when it is not used anymore. (2) missing of_put_node() for 'for_each_xxx' loop's break These bugs are similar with the ones reported by commit-09700c504d. Signed-off-by: Liang He --- changelog: v2: merge all powernv related bugs into one commit v1: only fix bugs in opal-powercap.c arch/powerpc/platforms/powernv/opal-powercap.c | 6 +- arch/powerpc/platforms/powernv/opal-psr.c | 6 +- arch/powerpc/platforms/powernv/opal-sensor-groups.c | 6 +- 3 files changed, 15 insertions(+), 3 deletions(-) diff --git a/arch/powerpc/platforms/powernv/opal-powercap.c b/arch/powerpc/platforms/powernv/opal-powercap.c index 64506b46e77b..78c359c90093 100644 --- a/arch/powerpc/platforms/powernv/opal-powercap.c +++ b/arch/powerpc/platforms/powernv/opal-powercap.c @@ -153,7 +153,7 @@ void __init opal_powercap_init(void) pcaps = kcalloc(of_get_child_count(powercap), sizeof(*pcaps), GFP_KERNEL); if (!pcaps) - return; + goto out_powercap; powercap_kobj = kobject_create_and_add("powercap", opal_kobj); if (!powercap_kobj) { @@ -226,6 +226,7 @@ void __init opal_powercap_init(void) } i++; } + of_node_put(powercap); return; @@ -236,6 +237,9 @@ void __init opal_powercap_init(void) kfree(pcaps[i].pg.name); } kobject_put(powercap_kobj); + of_node_put(node); out_pcaps: kfree(pcaps); +out_powercap: + of_node_put(powercap); } diff --git a/arch/powerpc/platforms/powernv/opal-psr.c b/arch/powerpc/platforms/powernv/opal-psr.c index 69d7e75950d1..ec32e0a93f08 100644 --- a/arch/powerpc/platforms/powernv/opal-psr.c +++ b/arch/powerpc/platforms/powernv/opal-psr.c @@ -135,7 +135,7 @@ void __init opal_psr_init(void) psr_attrs = kcalloc(of_get_child_count(psr), sizeof(*psr_attrs), GFP_KERNEL); if (!psr_attrs) - return; + goto out_psr; psr_kobj = kobject_create_and_add("psr", opal_kobj); if (!psr_kobj) { @@ -162,10 +162,14 @@ void __init opal_psr_init(void) } i++; } + of_node_put(psr); return; out_kobj: + of_node_put(node); kobject_put(psr_kobj); out: kfree(psr_attrs); +out_psr: + of_node_put(psr); } diff --git a/arch/powerpc/platforms/powernv/opal-sensor-groups.c b/arch/powerpc/platforms/powernv/opal-sensor-groups.c index 8fba7d25ae56..9944376b115c 100644 --- a/arch/powerpc/platforms/powernv/opal-sensor-groups.c +++ b/arch/powerpc/platforms/powernv/opal-sensor-groups.c @@ -170,7 +170,7 @@ void __init opal_sensor_groups_init(void) sgs = kcalloc(of_get_child_count(sg), sizeof(*sgs), GFP_KERNEL); if (!sgs) - return; + goto out_sg_put; sg_kobj = kobject_create_and_add("sensor_groups", opal_kobj); if (!sg_kobj) { @@ -222,6 +222,7 @@ void __init opal_sensor_groups_init(void) } i++; } + of_node_put(sg); return; @@ -231,6 +232,9 @@ void __init opal_sensor_groups_init(void) kfree(sgs[i].sg.attrs); } kobject_put(sg_kobj); + of_node_put(node); out_sgs: kfree(sgs); +out_sg_put: + of_node_put(sg); } -- 2.25.1
Re: [PATCH 1/4] hugetlb: skip to end of PT page mapping when pte not present
Hi, Mike, On Thu, Jun 16, 2022 at 02:05:15PM -0700, Mike Kravetz wrote: > @@ -6877,6 +6896,39 @@ pte_t *huge_pte_offset(struct mm_struct *mm, > return (pte_t *)pmd; > } > > +/* > + * Return a mask that can be used to update an address to the last huge > + * page in a page table page mapping size. Used to skip non-present > + * page table entries when linearly scanning address ranges. Architectures > + * with unique huge page to page table relationships can define their own > + * version of this routine. > + */ > +unsigned long hugetlb_mask_last_page(struct hstate *h) > +{ > + unsigned long hp_size = huge_page_size(h); > + > + switch (hp_size) { > + case P4D_SIZE: > + return PGDIR_SIZE - P4D_SIZE; > + case PUD_SIZE: > + return P4D_SIZE - PUD_SIZE; > + case PMD_SIZE: > + return PUD_SIZE - PMD_SIZE; > + default: Should we add a WARN_ON_ONCE() if it should never trigger? > + break; /* Should never happen */ > + } > + > + return ~(0UL); > +} > + > +#else > + > +/* See description above. Architectures can provide their own version. */ > +__weak unsigned long hugetlb_mask_last_page(struct hstate *h) > +{ > + return ~(0UL); I'm wondering whether it's better to return 0 rather than ~0 by default. Could an arch with !CONFIG_ARCH_WANT_GENERAL_HUGETLB wrongly skip some valid address ranges with ~0, or perhaps I misread? Thanks, -- Peter Xu
Re: [RFC PATCH 4/4] objtool/powerpc: Add --mcount specific implementation
Christophe Leroy wrote: Le 16/06/2022 à 15:57, Peter Zijlstra a écrit : On Thu, Jun 16, 2022 at 01:40:34PM +, Christophe Leroy wrote: sizeof(u64) is always 8 by definition. So if size is 8 we are working on a binary file for a 64 bits target, if not it means we are working for a 32 bits target. Cross-builds invalidate this I think. Best to look at something like: elf->ehdr.e_ident[EI_CLASS] == ELFCLASS32 Yes that's what it does indirectly: int size = elf_class_size(elf); With static inline int elf_class_size(struct elf *elf) { if (elf->ehdr.e_ident[EI_CLASS] == ELFCLASS32) return sizeof(u32); else return sizeof(u64); } Ok, those come from the below patch: https://lore.kernel.org/all/c4b06b5b314183d85615765a5ce421a057674bd8.1653398233.git.christophe.le...@csgroup.eu/T/#u I guess it would have been clearer if 'size' was named differently: 'addr_size' perhaps? - Naveen
Re: [PATCH v2 2/3] scsi: BusLogic remove bus_to_virt
On 2022-06-17 13:57, Arnd Bergmann wrote: From: Arnd Bergmann The BusLogic driver is the last remaining driver that relies on the deprecated bus_to_virt() function, which in turn only works on a few architectures, and is incompatible with both swiotlb and iommu support. Before commit 391e2f25601e ("[SCSI] BusLogic: Port driver to 64-bit."), the driver had a dependency on x86-32, presumably because of this problem. However, the change introduced another bug that made it still impossible to use the driver on any 64-bit machine. This was in turn fixed in commit 56f396146af2 ("scsi: BusLogic: Fix 64-bit system enumeration error for Buslogic"), 8 years later, which shows that there are not a lot of users. Maciej is still using the driver on 32-bit hardware, and Khalid mentioned that the driver works with the device emulation used in VirtualBox and VMware. Both of those only emulate it for Windows 2000 and older operating systems that did not ship with the better LSI logic driver. Do a minimum fix that searches through the list of descriptors to find one that matches the bus address. This is clearly as inefficient as was indicated in the code comment about the lack of a bus_to_virt() replacement. A better fix would likely involve changing out the entire descriptor allocation for a simpler one, but that would be much more invasive. FWIW, if efficiency *is* a practical concern, even under the current allocation scheme it looks like there are only 4 actual DMA allocations to search through. From a quick scan (since it's too hot here not to get distracted...), if I wanted to optimise this in future I'd probably remove the semi-redundant allocgrp_* fields from struct blogic_ccb and hang a dedicated list of the block allocations off the adapter - at that point the lookup could likely already be more efficient than a theoretical dma_to_virt() interface would be if it had to go off and walk an IOMMU pagetable. Then the next question would be whether it's viable to make a single 32KB allocation rather 4*8KB, so it's no longer even a list. For now, though, I agree with the simple change that's clear and easy to reason about: Reviewed-by: Robin Murphy Thanks, Robin. Cc: Maciej W. Rozycki Cc: Matt Wang Cc: Khalid Aziz Signed-off-by: Arnd Bergmann --- drivers/scsi/BusLogic.c | 27 --- drivers/scsi/Kconfig| 2 +- 2 files changed, 17 insertions(+), 12 deletions(-) diff --git a/drivers/scsi/BusLogic.c b/drivers/scsi/BusLogic.c index a897c8f914cf..d057abfcdd5c 100644 --- a/drivers/scsi/BusLogic.c +++ b/drivers/scsi/BusLogic.c @@ -2515,12 +2515,26 @@ static int blogic_resultcode(struct blogic_adapter *adapter, return (hoststatus << 16) | tgt_status; } +/* + * turn the dma address from an inbox into a ccb pointer + * This is rather inefficient. + */ +static struct blogic_ccb * +blogic_inbox_to_ccb(struct blogic_adapter *adapter, struct blogic_inbox *inbox) +{ + struct blogic_ccb *ccb; + + for (ccb = adapter->all_ccbs; ccb; ccb = ccb->next_all) + if (inbox->ccb == ccb->dma_handle) + break; + + return ccb; +} /* blogic_scan_inbox scans the Incoming Mailboxes saving any Incoming Mailbox entries for completion processing. */ - static void blogic_scan_inbox(struct blogic_adapter *adapter) { /* @@ -2540,16 +2554,7 @@ static void blogic_scan_inbox(struct blogic_adapter *adapter) enum blogic_cmplt_code comp_code; while ((comp_code = next_inbox->comp_code) != BLOGIC_INBOX_FREE) { - /* - We are only allowed to do this because we limit our - architectures we run on to machines where bus_to_virt( - actually works. There *needs* to be a dma_addr_to_virt() - in the new PCI DMA mapping interface to replace - bus_to_virt() or else this code is going to become very - innefficient. -*/ - struct blogic_ccb *ccb = - (struct blogic_ccb *) bus_to_virt(next_inbox->ccb); + struct blogic_ccb *ccb = blogic_inbox_to_ccb(adapter, adapter->next_inbox); if (comp_code != BLOGIC_CMD_NOTFOUND) { if (ccb->status == BLOGIC_CCB_ACTIVE || ccb->status == BLOGIC_CCB_RESET) { diff --git a/drivers/scsi/Kconfig b/drivers/scsi/Kconfig index cf75588a2587..56bdc08d0b77 100644 --- a/drivers/scsi/Kconfig +++ b/drivers/scsi/Kconfig @@ -513,7 +513,7 @@ config SCSI_HPTIOP config SCSI_BUSLOGIC tristate "BusLogic SCSI support" - depends on PCI && SCSI && VIRT_TO_BUS + depends on PCI && SCSI help This is support for BusLogic MultiMaster and FlashPoint SCSI Host Adapters. Consult the SCSI-HOWTO, available from
Re: [PATCH 04/36] cpuidle,intel_idle: Fix CPUIDLE_FLAG_IRQ_ENABLE
, Arnd Bergmann , ulli.kr...@googlemail.com, vgu...@kernel.org, linux-...@vger.kernel.org, j...@joshtriplett.org, rost...@goodmis.org, r...@vger.kernel.org, b...@alien8.de, bc...@quicinc.com, tsbog...@alpha.franken.de, linux-par...@vger.kernel.org, sudeep.ho...@arm.com, shawn...@kernel.org, da...@davemloft.net, dal...@libc.org, t...@atomide.com, amakha...@vmware.com, bjorn.anders...@linaro.org, h...@zytor.com, sparcli...@vger.kernel.org, linux-hexa...@vger.kernel.org, linux-ri...@lists.infradead.org, anton.iva...@cambridgegreys.com, jo...@southpole.se, yury.no...@gmail.com, rich...@nod.at, x...@kernel.org, li...@armlinux.org.uk, mi...@redhat.com, a...@eecs.berkeley.edu, paul...@kernel.org, h...@linux.ibm.com, stefan.kristians...@saunalahti.fi, openr...@lists.librecores.org, paul.walms...@sifive.com, linux-te...@vger.kernel.org, namhy...@kernel.org, andriy.shevche...@linux.intel.com, jpoim...@kernel.org, jgr...@suse.com, mon...@monstr.eu, linux-m...@vger.kernel.org, palmer@dab belt.com, a...@brainfault.org, i...@jurassic.park.msu.ru, johan...@sipsolutions.net, linuxppc-dev@lists.ozlabs.org Errors-To: linuxppc-dev-bounces+archive=mail-archive@lists.ozlabs.org Sender: "Linuxppc-dev" Hi Peter, On Mon, 13 Jun 2022 10:44:22 +0200, Peter Zijlstra wrote: > On Thu, Jun 09, 2022 at 04:49:21PM -0700, Jacob Pan wrote: > > Hi Peter, > > > > On Wed, 08 Jun 2022 16:27:27 +0200, Peter Zijlstra > > wrote: > > > > > Commit c227233ad64c ("intel_idle: enable interrupts before C1 on > > > Xeons") wrecked intel_idle in two ways: > > > > > > - must not have tracing in idle functions > > > - must return with IRQs disabled > > > > > > Additionally, it added a branch for no good reason. > > > > > > Fixes: c227233ad64c ("intel_idle: enable interrupts before C1 on > > > Xeons") Signed-off-by: Peter Zijlstra (Intel) > > > --- > > > drivers/idle/intel_idle.c | 48 > > > +++--- 1 file changed, 37 > > > insertions(+), 11 deletions(-) > > > > > > --- a/drivers/idle/intel_idle.c > > > +++ b/drivers/idle/intel_idle.c > > > @@ -129,21 +137,37 @@ static unsigned int mwait_substates __in > > > * > > > * Must be called under local_irq_disable(). > > > */ > > nit: this comment is no long true, right? > > It still is, all the idle routines are called with interrupts disabled, > but must also exit with interrupts disabled. > > If the idle method requires interrupts to be enabled, it must be sure to > disable them again before returning. Given all the RCU/tracing concerns > it must use raw_local_irq_*() for this though. Makes sense, it is just little confusing when the immediate caller does raw_local_irq_enable() which does not cancel out local_irq_disable(). Thanks, Jacob
[PATCH v2 3/3] arch/*/: remove CONFIG_VIRT_TO_BUS
From: Arnd Bergmann All architecture-independent users of virt_to_bus() and bus_to_virt() have been fixed to use the dma mapping interfaces or have been removed now. This means the definitions on most architectures, and the CONFIG_VIRT_TO_BUS symbol are now obsolete and can be removed. The only exceptions to this are a few network and scsi drivers for m68k Amiga and VME machines and ppc32 Macintosh. These drivers work correctly with the old interfaces and are probably not worth changing. On alpha and parisc, virt_to_bus() were still used in asm/floppy.h. alpha can use isa_virt_to_bus() like x86 does, and parisc can just open-code the virt_to_phys() here, as this is architecture specific code. I tried updating the bus-virt-phys-mapping.rst documentation, which started as an email from Linus to explain some details of the Linux-2.0 driver interfaces. The bits about virt_to_bus() were declared obsolete backin 2000, and the rest is not all that relevant any more, so in the end I just decided to remove the file completely. Reviewed-by: Geert Uytterhoeven Acked-by: Geert Uytterhoeven Acked-by: Michael Ellerman (powerpc) Signed-off-by: Arnd Bergmann --- .../core-api/bus-virt-phys-mapping.rst| 220 -- Documentation/core-api/dma-api-howto.rst | 14 -- Documentation/core-api/index.rst | 1 - .../translations/zh_CN/core-api/index.rst | 1 - arch/alpha/Kconfig| 1 - arch/alpha/include/asm/floppy.h | 2 +- arch/alpha/include/asm/io.h | 8 +- arch/ia64/Kconfig | 1 - arch/ia64/include/asm/io.h| 8 - arch/m68k/Kconfig | 1 - arch/m68k/include/asm/virtconvert.h | 4 +- arch/microblaze/Kconfig | 1 - arch/microblaze/include/asm/io.h | 2 - arch/mips/Kconfig | 1 - arch/mips/include/asm/io.h| 9 - arch/parisc/Kconfig | 1 - arch/parisc/include/asm/floppy.h | 4 +- arch/parisc/include/asm/io.h | 2 - arch/powerpc/Kconfig | 1 - arch/powerpc/include/asm/io.h | 2 - arch/riscv/include/asm/page.h | 1 - arch/x86/Kconfig | 1 - arch/x86/include/asm/io.h | 9 - arch/xtensa/Kconfig | 1 - arch/xtensa/include/asm/io.h | 3 - include/asm-generic/io.h | 14 -- mm/Kconfig| 8 - 27 files changed, 10 insertions(+), 311 deletions(-) delete mode 100644 Documentation/core-api/bus-virt-phys-mapping.rst diff --git a/Documentation/core-api/bus-virt-phys-mapping.rst b/Documentation/core-api/bus-virt-phys-mapping.rst deleted file mode 100644 index c72b24a7d52c.. --- a/Documentation/core-api/bus-virt-phys-mapping.rst +++ /dev/null @@ -1,220 +0,0 @@ -== -How to access I/O mapped memory from within device drivers -== - -:Author: Linus - -.. warning:: - - The virt_to_bus() and bus_to_virt() functions have been - superseded by the functionality provided by the PCI DMA interface - (see Documentation/core-api/dma-api-howto.rst). They continue - to be documented below for historical purposes, but new code - must not use them. --davidm 00/12/12 - -:: - - [ This is a mail message in response to a query on IO mapping, thus the -strange format for a "document" ] - -The AHA-1542 is a bus-master device, and your patch makes the driver give the -controller the physical address of the buffers, which is correct on x86 -(because all bus master devices see the physical memory mappings directly). - -However, on many setups, there are actually **three** different ways of looking -at memory addresses, and in this case we actually want the third, the -so-called "bus address". - -Essentially, the three ways of addressing memory are (this is "real memory", -that is, normal RAM--see later about other details): - - - CPU untranslated. This is the "physical" address. Physical address - 0 is what the CPU sees when it drives zeroes on the memory bus. - - - CPU translated address. This is the "virtual" address, and is - completely internal to the CPU itself with the CPU doing the appropriate - translations into "CPU untranslated". - - - bus address. This is the address of memory as seen by OTHER devices, - not the CPU. Now, in theory there could be many different bus - addresses, with each device seeing memory in some device-specific way, but - happily most hardware designers aren't actually actively trying to make - things any more complex than necessary, so you can assume that all - external hardwar
[PATCH v2 2/3] scsi: BusLogic remove bus_to_virt
From: Arnd Bergmann The BusLogic driver is the last remaining driver that relies on the deprecated bus_to_virt() function, which in turn only works on a few architectures, and is incompatible with both swiotlb and iommu support. Before commit 391e2f25601e ("[SCSI] BusLogic: Port driver to 64-bit."), the driver had a dependency on x86-32, presumably because of this problem. However, the change introduced another bug that made it still impossible to use the driver on any 64-bit machine. This was in turn fixed in commit 56f396146af2 ("scsi: BusLogic: Fix 64-bit system enumeration error for Buslogic"), 8 years later, which shows that there are not a lot of users. Maciej is still using the driver on 32-bit hardware, and Khalid mentioned that the driver works with the device emulation used in VirtualBox and VMware. Both of those only emulate it for Windows 2000 and older operating systems that did not ship with the better LSI logic driver. Do a minimum fix that searches through the list of descriptors to find one that matches the bus address. This is clearly as inefficient as was indicated in the code comment about the lack of a bus_to_virt() replacement. A better fix would likely involve changing out the entire descriptor allocation for a simpler one, but that would be much more invasive. Cc: Maciej W. Rozycki Cc: Matt Wang Cc: Khalid Aziz Signed-off-by: Arnd Bergmann --- drivers/scsi/BusLogic.c | 27 --- drivers/scsi/Kconfig| 2 +- 2 files changed, 17 insertions(+), 12 deletions(-) diff --git a/drivers/scsi/BusLogic.c b/drivers/scsi/BusLogic.c index a897c8f914cf..d057abfcdd5c 100644 --- a/drivers/scsi/BusLogic.c +++ b/drivers/scsi/BusLogic.c @@ -2515,12 +2515,26 @@ static int blogic_resultcode(struct blogic_adapter *adapter, return (hoststatus << 16) | tgt_status; } +/* + * turn the dma address from an inbox into a ccb pointer + * This is rather inefficient. + */ +static struct blogic_ccb * +blogic_inbox_to_ccb(struct blogic_adapter *adapter, struct blogic_inbox *inbox) +{ + struct blogic_ccb *ccb; + + for (ccb = adapter->all_ccbs; ccb; ccb = ccb->next_all) + if (inbox->ccb == ccb->dma_handle) + break; + + return ccb; +} /* blogic_scan_inbox scans the Incoming Mailboxes saving any Incoming Mailbox entries for completion processing. */ - static void blogic_scan_inbox(struct blogic_adapter *adapter) { /* @@ -2540,16 +2554,7 @@ static void blogic_scan_inbox(struct blogic_adapter *adapter) enum blogic_cmplt_code comp_code; while ((comp_code = next_inbox->comp_code) != BLOGIC_INBOX_FREE) { - /* - We are only allowed to do this because we limit our - architectures we run on to machines where bus_to_virt( - actually works. There *needs* to be a dma_addr_to_virt() - in the new PCI DMA mapping interface to replace - bus_to_virt() or else this code is going to become very - innefficient. -*/ - struct blogic_ccb *ccb = - (struct blogic_ccb *) bus_to_virt(next_inbox->ccb); + struct blogic_ccb *ccb = blogic_inbox_to_ccb(adapter, adapter->next_inbox); if (comp_code != BLOGIC_CMD_NOTFOUND) { if (ccb->status == BLOGIC_CCB_ACTIVE || ccb->status == BLOGIC_CCB_RESET) { diff --git a/drivers/scsi/Kconfig b/drivers/scsi/Kconfig index cf75588a2587..56bdc08d0b77 100644 --- a/drivers/scsi/Kconfig +++ b/drivers/scsi/Kconfig @@ -513,7 +513,7 @@ config SCSI_HPTIOP config SCSI_BUSLOGIC tristate "BusLogic SCSI support" - depends on PCI && SCSI && VIRT_TO_BUS + depends on PCI && SCSI help This is support for BusLogic MultiMaster and FlashPoint SCSI Host Adapters. Consult the SCSI-HOWTO, available from -- 2.29.2
[PATCH v2 1/3] scsi: dpt_i2o: drop stale VIRT_TO_BUS dependency
From: Arnd Bergmann The dpt_i2o driver was fixed to stop using virt_to_bus() in 2008, but it still has a stale reference in an error handling code path that could never work. Fix it up to build without VIRT_TO_BUS and remove the Kconfig dependency. The alternative to this would be to just remove the driver, as it is clearly obsolete. The i2o driver layer was removed in 2015 with commit 4a72a7af462d ("staging: remove i2o subsystem"), but the even older dpt_i2o scsi driver stayed around. The last non-cleanup patches I could find were from Miquel van Smoorenburg and Mark Salyzyn back in 2008, they might know if there is any chance of the hardware still being used anywhere. Fixes: 67af2b060e02 ("[SCSI] dpt_i2o: move from virt_to_bus/bus_to_virt to dma_alloc_coherent") Cc: Miquel van Smoorenburg Cc: Mark Salyzyn Signed-off-by: Arnd Bergmann --- drivers/scsi/Kconfig | 2 +- drivers/scsi/dpt_i2o.c | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/scsi/Kconfig b/drivers/scsi/Kconfig index a9fe5152addd..cf75588a2587 100644 --- a/drivers/scsi/Kconfig +++ b/drivers/scsi/Kconfig @@ -460,7 +460,7 @@ config SCSI_MVUMI config SCSI_DPT_I2O tristate "Adaptec I2O RAID support " - depends on SCSI && PCI && VIRT_TO_BUS + depends on SCSI && PCI help This driver supports all of Adaptec's I2O based RAID controllers as well as the DPT SmartRaid V cards. This is an Adaptec maintained diff --git a/drivers/scsi/dpt_i2o.c b/drivers/scsi/dpt_i2o.c index 2e9155ba7408..55dfe7011912 100644 --- a/drivers/scsi/dpt_i2o.c +++ b/drivers/scsi/dpt_i2o.c @@ -52,11 +52,11 @@ MODULE_DESCRIPTION("Adaptec I2O RAID Driver"); #include #include +#include #include #include #include /* for boot_cpu_data */ -#include /* for virt_to_bus, etc. */ #include #include @@ -2112,7 +2112,7 @@ static irqreturn_t adpt_isr(int irq, void *dev_id) } else { /* Ick, we should *never* be here */ printk(KERN_ERR "dpti: reply frame not from pool\n"); - reply = (u8 *)bus_to_virt(m); + goto out; } if (readl(reply) & MSG_FAIL) { -- 2.29.2
[PATCH v2 0/3] phase out CONFIG_VIRT_TO_BUS
From: Arnd Bergmann The virt_to_bus/bus_to_virt interface has been deprecated for decades. After Jakub Kicinski put a lot of work into cleaning out the network drivers using them, there are only a couple of other drivers left, which can all be removed or otherwise cleaned up, to remove the old interface for good. Any out of tree drivers using virt_to_bus() should be converted to using the dma-mapping interfaces, typically dma_alloc_coherent() or dma_map_single()). There are a few m68k and ppc32 specific drivers that keep using the interfaces, but these are all guarded with architecture-specific Kconfig dependencies, and are not actually broken. There are still a number of drivers that are using virt_to_phys() and phys_to_virt() in place of dma-mapping operations, and these are often broken, but they are out of scope for this series. I would like the first two patches to either get merged through the SCSI tree, or get an Ack from the SCSI maintainers so I can merge them through the asm-generic tree Arnd --- Changes since v1: - dropped VME patches that are already in staging-next - dropped media patch that gets merged independently - added a networking patch and dropped it again after it got merged - replace BusLogic removal with a workaround Cc: Jakub Kicinski Cc: Christoph Hellwig # dma-mapping Cc: Marek Szyprowski # dma-mapping Cc: Robin Murphy # dma-mapping Cc: io...@lists.linux-foundation.org Cc: Khalid Aziz # buslogic Cc: Maciej W. Rozycki # buslogic Cc: Matt Wang # buslogic Cc: Miquel van Smoorenburg # dpt_i2o Cc: Mark Salyzyn # dpt_i2o Cc: linux-s...@vger.kernel.org Cc: linuxppc-dev@lists.ozlabs.org Cc: linux-a...@vger.kernel.org Cc: linux-al...@vger.kernel.org Cc: linux-m...@lists.linux-m68k.org Cc: linux-par...@vger.kernel.org Cc: Denis Efremov # floppy Arnd Bergmann (3): scsi: dpt_i2o: drop stale VIRT_TO_BUS dependency scsi: BusLogic remove bus_to_virt arch/*/: remove CONFIG_VIRT_TO_BUS .../core-api/bus-virt-phys-mapping.rst| 220 -- Documentation/core-api/dma-api-howto.rst | 14 -- Documentation/core-api/index.rst | 1 - .../translations/zh_CN/core-api/index.rst | 1 - arch/alpha/Kconfig| 1 - arch/alpha/include/asm/floppy.h | 2 +- arch/alpha/include/asm/io.h | 8 +- arch/ia64/Kconfig | 1 - arch/ia64/include/asm/io.h| 8 - arch/m68k/Kconfig | 1 - arch/m68k/include/asm/virtconvert.h | 4 +- arch/microblaze/Kconfig | 1 - arch/microblaze/include/asm/io.h | 2 - arch/mips/Kconfig | 1 - arch/mips/include/asm/io.h| 9 - arch/parisc/Kconfig | 1 - arch/parisc/include/asm/floppy.h | 4 +- arch/parisc/include/asm/io.h | 2 - arch/powerpc/Kconfig | 1 - arch/powerpc/include/asm/io.h | 2 - arch/riscv/include/asm/page.h | 1 - arch/x86/Kconfig | 1 - arch/x86/include/asm/io.h | 9 - arch/xtensa/Kconfig | 1 - arch/xtensa/include/asm/io.h | 3 - drivers/scsi/BusLogic.c | 27 ++- drivers/scsi/Kconfig | 4 +- drivers/scsi/dpt_i2o.c| 4 +- include/asm-generic/io.h | 14 -- mm/Kconfig| 8 - 30 files changed, 30 insertions(+), 326 deletions(-) delete mode 100644 Documentation/core-api/bus-virt-phys-mapping.rst -- 2.29.2
Re: [PATCH v2 0/4] pseries-wdt: initial support for H_WATCHDOG-based watchdog timers
On 6/2/22 14:53, Scott Cheloha wrote: PAPR v2.12 defines a new hypercall, H_WATCHDOG. This patch series adds support for this hypercall to powerpc/pseries kernels and introduces a new watchdog driver, "pseries-wdt", for the virtual timers exposed by the hypercall. This series is preceded by the following: RFC v1: https://lore.kernel.org/linux-watchdog/20220413165104.179144-1-chel...@linux.ibm.com/ RFC v2: https://lore.kernel.org/linux-watchdog/20220509174357.5448-1-chel...@linux.ibm.com/ PATCH v1: https://lore.kernel.org/linux-watchdog/20220520183552.33426-1-chel...@linux.ibm.com/ Tested successfully with mainline QEMU plus Alexey's new h_watchdog patches in https://patchwork.ozlabs.org/project/qemu-ppc/list/?series=305226. All patches of this series: Tested-by: Daniel Henrique Barboza Thanks, Daniel Changes of note from PATCH v1: - Trim down the large comment documenting the H_WATCHDOG hypercall. The comment is likely to rot, so remove anything we aren't using and anything overly obvious. - Remove any preprocessor definitions not actually used in the module right now. If we want to use other features offered by the hypercall we can add them in later. They're just clutter until then. - Simplify the "action" module parameter. The value is now an index into an array of possible timeoutAction values. This design removes the need for the custom get/set methods used in PATCH v1. Now we merely need to check that the "action" value is a valid index during pseries_wdt_probe(). Easy. - Make the timeoutAction a member of pseries_wdt, "action". This eliminates the use of a global variable during pseries_wdt_start(). - Use watchdog_init_timeout() idiomatically. Check its return value and error out of pseries_wdt_probe() if it fails.
[PATCH] powerpc: maple: Fix refcount leak bug in time.c
In maple_get_boot_time(), of_find_compatible_node() will return a node pointer with refcount incremented. We should use of_node_put() in fail path or when it is not used anymore. Signed-off-by: Liang He --- arch/powerpc/platforms/maple/time.c | 1 + 1 file changed, 1 insertion(+) diff --git a/arch/powerpc/platforms/maple/time.c b/arch/powerpc/platforms/maple/time.c index 823e219ef8ee..91606411d2e0 100644 --- a/arch/powerpc/platforms/maple/time.c +++ b/arch/powerpc/platforms/maple/time.c @@ -153,6 +153,7 @@ time64_t __init maple_get_boot_time(void) maple_rtc_addr); } bail: + of_node_put(rtcs); if (maple_rtc_addr == 0) { maple_rtc_addr = RTC_PORT(0); /* legacy address */ printk(KERN_INFO "Maple: No device node for RTC, assuming " -- 2.25.1
Re:Re: [PATCH v4] powerpc:85xx: Add missing of_node_put() in sgy_cst1000
在 2022-06-17 18:57:36,"Christophe Leroy" 写道: > > >Le 17/06/2022 à 12:47, Liang He a écrit : >> >> >> >> At 2022-06-17 16:27:03, conor.doo...@microchip.com wrote: >>> On 17/06/2022 09:17, Liang He wrote: At 2022-06-17 14:53:13, "Christophe Leroy" wrote: > > > Le 17/06/2022 à 08:45, Liang He a écrit : >> >> >> >> At 2022-06-17 14:28:56, "Christophe Leroy" >> wrote: >>> >>> >>> Le 17/06/2022 à 08:08, Liang He a écrit : In gpio_halt_probe(), of_find_matching_node() will return a node pointer with refcount incremented. We should use of_node_put() in fail path or when it is not used anymore. Signed-off-by: Liang He --- changelog: v4: reuse exist 'err' and use a simple code style, advised by CJ v3: use local 'child_node' advised by Michael. v2: use goto-label patch style advised by Christophe Leroy. v1: add of_node_put() before each exit. arch/powerpc/platforms/85xx/sgy_cts1000.c | 35 ++- 1 file changed, 22 insertions(+), 13 deletions(-) diff --git a/arch/powerpc/platforms/85xx/sgy_cts1000.c b/arch/powerpc/platforms/85xx/sgy_cts1000.c index 98ae64075193..e4588943fe7e 100644 --- a/arch/powerpc/platforms/85xx/sgy_cts1000.c +++ b/arch/powerpc/platforms/85xx/sgy_cts1000.c @@ -71,6 +71,7 @@ static int gpio_halt_probe(struct platform_device *pdev) { enum of_gpio_flags flags; struct device_node *node = pdev->dev.of_node; + struct device_node *child_node; int gpio, err, irq; int trigger; @@ -78,26 +79,29 @@ static int gpio_halt_probe(struct platform_device *pdev) return -ENODEV; /* If there's no matching child, this isn't really an error */ - halt_node = of_find_matching_node(node, child_match); - if (!halt_node) + child_node = of_find_matching_node(node, child_match); + if (!child_node) return 0; /* Technically we could just read the first one, but punish * DT writers for invalid form. */ - if (of_gpio_count(halt_node) != 1) - return -EINVAL; + if (of_gpio_count(child_node) != 1) { + err = -EINVAL; + goto err_put; + } /* Get the gpio number relative to the dynamic base. */ - gpio = of_get_gpio_flags(halt_node, 0, &flags); - if (!gpio_is_valid(gpio)) - return -EINVAL; + gpio = of_get_gpio_flags(child_node, 0, &flags); + if (!gpio_is_valid(gpio)) { + err = -EINVAL; + gotot err_put; >>> >>> Did you test the build ? >> >> Sorry for this fault. >> >> In fact, I am still finding an efficient way to building different arch >> source code as I only have x86-64. >> >> Now I am try using QEMU. >> >> Anyway, sorry for this fault. > > You can find cross compilers for most architectures for x86-64 here : > https://mirrors.edge.kernel.org/pub/tools/crosstool/ > > Christophe Hi, Christophe and Conor. Sorry to trouble you again. Now I only know how to quickly identify the refcounting bugs, but I cannot efficiently give a build test. For example, I use the cross compilers 'powerpc-linux-gnu-gcc' to compile 'arch/powerpc/platforms/85xx/sgy_cts1000.c' with -fsyntax-only flag. But I meet too many header file missing errors. Even if I add some 'include' pathes, e.g., ./arch/powerpc/include, ./include, there are still too many other errors. So if there is any efficient way to check my patch code to avoid 'gotot' error again. >>> >>> idk anything about powerpc, but what I find is a nice way to get a compiler >>> for an arch I don't use is to search on lore.kernel.org for a 0day robot >>> build error since it gives instructions for building on that arch. >>> For example: >>> https://lore.kernel.org/linuxppc-dev/202206060910.ryntfqdi-...@intel.com/ >>> >>> >>> In this case, your bug seems obvious? You typed "gotot" instead of "goto". >>> >>> Hope that helps, >>> Conor. >>> Thanks again, Christophe and Conor. Liang >> >> Thanks, Conor and Christophe. >> >> I finally figure out an efficient way in which I can use cross-compiler to >> check my single patched file as follow: >> >> powerpc64le-linux-gnu-gcc -Wp,-MMD,arch/powerpc/kernel/.io.o.d -nostdinc >> -I./arch/powerpc/includ
[PATCH] powerpc: kernel: Change the order of of_node_put()
In add_pcspkr(), it is better to call of_node_put() after the 'if(!np)' check. Signed-off-by: Liang He --- arch/powerpc/kernel/setup-common.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arch/powerpc/kernel/setup-common.c b/arch/powerpc/kernel/setup-common.c index eb0077b302e2..761817d1f4db 100644 --- a/arch/powerpc/kernel/setup-common.c +++ b/arch/powerpc/kernel/setup-common.c @@ -563,9 +563,9 @@ static __init int add_pcspkr(void) int ret; np = of_find_compatible_node(NULL, NULL, "pnpPNP,100"); - of_node_put(np); if (!np) return -ENODEV; + of_node_put(np); pd = platform_device_alloc("pcspkr", -1); if (!pd) -- 2.25.1
Re: [PATCH v4] powerpc:85xx: Add missing of_node_put() in sgy_cst1000
Le 17/06/2022 à 12:47, Liang He a écrit : > > > > At 2022-06-17 16:27:03, conor.doo...@microchip.com wrote: >> On 17/06/2022 09:17, Liang He wrote: >>> >>> >>> >>> At 2022-06-17 14:53:13, "Christophe Leroy" >>> wrote: Le 17/06/2022 à 08:45, Liang He a écrit : > > > > At 2022-06-17 14:28:56, "Christophe Leroy" > wrote: >> >> >> Le 17/06/2022 à 08:08, Liang He a écrit : >>> In gpio_halt_probe(), of_find_matching_node() will return a node >>> pointer with refcount incremented. We should use of_node_put() in >>> fail path or when it is not used anymore. >>> >>> Signed-off-by: Liang He >>> --- >>> changelog: >>> v4: reuse exist 'err' and use a simple code style, advised by CJ >>> v3: use local 'child_node' advised by Michael. >>> v2: use goto-label patch style advised by Christophe Leroy. >>> v1: add of_node_put() before each exit. >>> >>> arch/powerpc/platforms/85xx/sgy_cts1000.c | 35 >>> ++- >>> 1 file changed, 22 insertions(+), 13 deletions(-) >>> >>> diff --git a/arch/powerpc/platforms/85xx/sgy_cts1000.c >>> b/arch/powerpc/platforms/85xx/sgy_cts1000.c >>> index 98ae64075193..e4588943fe7e 100644 >>> --- a/arch/powerpc/platforms/85xx/sgy_cts1000.c >>> +++ b/arch/powerpc/platforms/85xx/sgy_cts1000.c >>> @@ -71,6 +71,7 @@ static int gpio_halt_probe(struct platform_device >>> *pdev) >>> { >>> enum of_gpio_flags flags; >>> struct device_node *node = pdev->dev.of_node; >>> + struct device_node *child_node; >>> int gpio, err, irq; >>> int trigger; >>> >>> @@ -78,26 +79,29 @@ static int gpio_halt_probe(struct platform_device >>> *pdev) >>> return -ENODEV; >>> >>> /* If there's no matching child, this isn't really an error */ >>> - halt_node = of_find_matching_node(node, child_match); >>> - if (!halt_node) >>> + child_node = of_find_matching_node(node, child_match); >>> + if (!child_node) >>> return 0; >>> >>> /* Technically we could just read the first one, but punish >>> * DT writers for invalid form. */ >>> - if (of_gpio_count(halt_node) != 1) >>> - return -EINVAL; >>> + if (of_gpio_count(child_node) != 1) { >>> + err = -EINVAL; >>> + goto err_put; >>> + } >>> >>> /* Get the gpio number relative to the dynamic base. */ >>> - gpio = of_get_gpio_flags(halt_node, 0, &flags); >>> - if (!gpio_is_valid(gpio)) >>> - return -EINVAL; >>> + gpio = of_get_gpio_flags(child_node, 0, &flags); >>> + if (!gpio_is_valid(gpio)) { >>> + err = -EINVAL; >>> + gotot err_put; >> >> Did you test the build ? > > Sorry for this fault. > > In fact, I am still finding an efficient way to building different arch > source code as I only have x86-64. > > Now I am try using QEMU. > > Anyway, sorry for this fault. You can find cross compilers for most architectures for x86-64 here : https://mirrors.edge.kernel.org/pub/tools/crosstool/ Christophe >>> >>> Hi, Christophe and Conor. >>> >>> Sorry to trouble you again. >>> >>> Now I only know how to quickly identify the refcounting bugs, but I cannot >>> efficiently give a build test. >>> >>> For example, I use the cross compilers 'powerpc-linux-gnu-gcc' to compile >>> 'arch/powerpc/platforms/85xx/sgy_cts1000.c' with -fsyntax-only flag. >>> But I meet too many header file missing errors. Even if I add some >>> 'include' pathes, e.g., ./arch/powerpc/include, ./include, >>> there are still too many other errors. >>> >>> So if there is any efficient way to check my patch code to avoid 'gotot' >>> error again. >> >> idk anything about powerpc, but what I find is a nice way to get a compiler >> for an arch I don't use is to search on lore.kernel.org for a 0day robot >> build error since it gives instructions for building on that arch. >> For example: >> https://lore.kernel.org/linuxppc-dev/202206060910.ryntfqdi-...@intel.com/ >> >> >> In this case, your bug seems obvious? You typed "gotot" instead of "goto". >> >> Hope that helps, >> Conor. >> >>> >>> Thanks again, Christophe and Conor. >>> >>> Liang > > Thanks, Conor and Christophe. > > I finally figure out an efficient way in which I can use cross-compiler to > check my single patched file as follow: > > powerpc64le-linux-gnu-gcc -Wp,-MMD,arch/powerpc/kernel/.io.o.d -nostdinc > -I./arch/powerpc/include -I./arch/powerpc/include/generated -I./include > -I./arch/powerpc/include/uapi -I./arch/powerpc/include/generated/uapi > -I./include/uapi -I./include/gener
[PATCH v5] powerpc:85xx: Add missing of_node_put() in sgy_cst1000
In gpio_halt_probe(), of_find_matching_node() will return a node pointer with refcount incremented. We should use of_node_put() in fail path or when it is not used anymore. Signed-off-by: Liang He --- changelog: v5: fix 'gotot' error introduced by v4 and use cross-compiler to test v4: reuse exist 'err' and use a simple code style, advised by CJ v3: use local 'child_node' advised by Michael. v2: use goto-label patch style advised by Christophe Leroy. v1: add of_node_put() before each exit. arch/powerpc/platforms/85xx/sgy_cts1000.c | 35 ++- 1 file changed, 22 insertions(+), 13 deletions(-) diff --git a/arch/powerpc/platforms/85xx/sgy_cts1000.c b/arch/powerpc/platforms/85xx/sgy_cts1000.c index 98ae64075193..e14d1b74d4e4 100644 --- a/arch/powerpc/platforms/85xx/sgy_cts1000.c +++ b/arch/powerpc/platforms/85xx/sgy_cts1000.c @@ -71,6 +71,7 @@ static int gpio_halt_probe(struct platform_device *pdev) { enum of_gpio_flags flags; struct device_node *node = pdev->dev.of_node; + struct device_node *child_node; int gpio, err, irq; int trigger; @@ -78,26 +79,29 @@ static int gpio_halt_probe(struct platform_device *pdev) return -ENODEV; /* If there's no matching child, this isn't really an error */ - halt_node = of_find_matching_node(node, child_match); - if (!halt_node) + child_node = of_find_matching_node(node, child_match); + if (!child_node) return 0; /* Technically we could just read the first one, but punish * DT writers for invalid form. */ - if (of_gpio_count(halt_node) != 1) - return -EINVAL; + if (of_gpio_count(child_node) != 1) { + err = -EINVAL; + goto err_put; + } /* Get the gpio number relative to the dynamic base. */ - gpio = of_get_gpio_flags(halt_node, 0, &flags); - if (!gpio_is_valid(gpio)) - return -EINVAL; + gpio = of_get_gpio_flags(child_node, 0, &flags); + if (!gpio_is_valid(gpio)) { + err = -EINVAL; + goto err_put; + } err = gpio_request(gpio, "gpio-halt"); if (err) { printk(KERN_ERR "gpio-halt: error requesting GPIO %d.\n", gpio); - halt_node = NULL; - return err; + goto err_put; } trigger = (flags == OF_GPIO_ACTIVE_LOW); @@ -105,15 +109,14 @@ static int gpio_halt_probe(struct platform_device *pdev) gpio_direction_output(gpio, !trigger); /* Now get the IRQ which tells us when the power button is hit */ - irq = irq_of_parse_and_map(halt_node, 0); + irq = irq_of_parse_and_map(child_node, 0); err = request_irq(irq, gpio_halt_irq, IRQF_TRIGGER_RISING | - IRQF_TRIGGER_FALLING, "gpio-halt", halt_node); + IRQF_TRIGGER_FALLING, "gpio-halt", child_node); if (err) { printk(KERN_ERR "gpio-halt: error requesting IRQ %d for " "GPIO %d.\n", irq, gpio); gpio_free(gpio); - halt_node = NULL; - return err; + goto err_put; } /* Register our halt function */ @@ -123,7 +126,12 @@ static int gpio_halt_probe(struct platform_device *pdev) printk(KERN_INFO "gpio-halt: registered GPIO %d (%d trigger, %d" " irq).\n", gpio, trigger, irq); + halt_node = child_node; return 0; + +err_put: + of_node_put(child_node); + return err; } static int gpio_halt_remove(struct platform_device *pdev) @@ -139,6 +147,7 @@ static int gpio_halt_remove(struct platform_device *pdev) gpio_free(gpio); + of_node_put(halt_node); halt_node = NULL; } -- 2.25.1
Re:Re: [PATCH v4] powerpc:85xx: Add missing of_node_put() in sgy_cst1000
At 2022-06-17 16:27:03, conor.doo...@microchip.com wrote: >On 17/06/2022 09:17, Liang He wrote: >> >> >> >> At 2022-06-17 14:53:13, "Christophe Leroy" >> wrote: >>> >>> >>> Le 17/06/2022 à 08:45, Liang He a écrit : At 2022-06-17 14:28:56, "Christophe Leroy" wrote: > > > Le 17/06/2022 à 08:08, Liang He a écrit : >> In gpio_halt_probe(), of_find_matching_node() will return a node >> pointer with refcount incremented. We should use of_node_put() in >> fail path or when it is not used anymore. >> >> Signed-off-by: Liang He >> --- >> changelog: >> v4: reuse exist 'err' and use a simple code style, advised by CJ >> v3: use local 'child_node' advised by Michael. >> v2: use goto-label patch style advised by Christophe Leroy. >> v1: add of_node_put() before each exit. >> >> arch/powerpc/platforms/85xx/sgy_cts1000.c | 35 >> ++- >> 1 file changed, 22 insertions(+), 13 deletions(-) >> >> diff --git a/arch/powerpc/platforms/85xx/sgy_cts1000.c >> b/arch/powerpc/platforms/85xx/sgy_cts1000.c >> index 98ae64075193..e4588943fe7e 100644 >> --- a/arch/powerpc/platforms/85xx/sgy_cts1000.c >> +++ b/arch/powerpc/platforms/85xx/sgy_cts1000.c >> @@ -71,6 +71,7 @@ static int gpio_halt_probe(struct platform_device >> *pdev) >> { >> enum of_gpio_flags flags; >> struct device_node *node = pdev->dev.of_node; >> +struct device_node *child_node; >> int gpio, err, irq; >> int trigger; >> >> @@ -78,26 +79,29 @@ static int gpio_halt_probe(struct platform_device >> *pdev) >> return -ENODEV; >> >> /* If there's no matching child, this isn't really an error */ >> -halt_node = of_find_matching_node(node, child_match); >> -if (!halt_node) >> +child_node = of_find_matching_node(node, child_match); >> +if (!child_node) >> return 0; >> >> /* Technically we could just read the first one, but punish >> * DT writers for invalid form. */ >> -if (of_gpio_count(halt_node) != 1) >> -return -EINVAL; >> +if (of_gpio_count(child_node) != 1) { >> +err = -EINVAL; >> +goto err_put; >> +} >> >> /* Get the gpio number relative to the dynamic base. */ >> -gpio = of_get_gpio_flags(halt_node, 0, &flags); >> -if (!gpio_is_valid(gpio)) >> -return -EINVAL; >> +gpio = of_get_gpio_flags(child_node, 0, &flags); >> +if (!gpio_is_valid(gpio)) { >> +err = -EINVAL; >> +gotot err_put; > > Did you test the build ? Sorry for this fault. In fact, I am still finding an efficient way to building different arch source code as I only have x86-64. Now I am try using QEMU. Anyway, sorry for this fault. >>> >>> You can find cross compilers for most architectures for x86-64 here : >>> https://mirrors.edge.kernel.org/pub/tools/crosstool/ >>> >>> Christophe >> >> Hi, Christophe and Conor. >> >> Sorry to trouble you again. >> >> Now I only know how to quickly identify the refcounting bugs, but I cannot >> efficiently give a build test. >> >> For example, I use the cross compilers 'powerpc-linux-gnu-gcc' to compile >> 'arch/powerpc/platforms/85xx/sgy_cts1000.c' with -fsyntax-only flag. >> But I meet too many header file missing errors. Even if I add some 'include' >> pathes, e.g., ./arch/powerpc/include, ./include, >> there are still too many other errors. >> >> So if there is any efficient way to check my patch code to avoid 'gotot' >> error again. > >idk anything about powerpc, but what I find is a nice way to get a compiler >for an arch I don't use is to search on lore.kernel.org for a 0day robot >build error since it gives instructions for building on that arch. >For example: >https://lore.kernel.org/linuxppc-dev/202206060910.ryntfqdi-...@intel.com/ > > >In this case, your bug seems obvious? You typed "gotot" instead of "goto". > >Hope that helps, >Conor. > >> >> Thanks again, Christophe and Conor. >> >> Liang Thanks, Conor and Christophe. I finally figure out an efficient way in which I can use cross-compiler to check my single patched file as follow: powerpc64le-linux-gnu-gcc -Wp,-MMD,arch/powerpc/kernel/.io.o.d -nostdinc -I./arch/powerpc/include -I./arch/powerpc/include/generated -I./include -I./arch/powerpc/include/uapi -I./arch/powerpc/include/generated/uapi -I./include/uapi -I./include/generated/uapi -include ./include/linux/compiler-version.h -include ./include/linux/kconfig.h -include ./include/linux/compiler_types.h -D__KERNEL__ -I ./arch/powerpc -fmacro-prefix
Re: [PATCH] arch/*: Disable softirq stacks on PREEMPT_RT.
On 2022-06-15 17:41:45 [+0200], Arnd Bergmann wrote: > Applied to the asm-generic tree with the above fixup, thanks! Thank you Arnd. > Arnd Sebastian
Re: [PATCH -next v5 7/8] arm64: add uaccess to machine check safe
On Sat, May 28, 2022 at 06:50:55AM +, Tong Tiangen wrote: > If user access fail due to hardware memory error, only the relevant > processes are affected, so killing the user process and isolate the > error page with hardware memory errors is a more reasonable choice > than kernel panic. > > Signed-off-by: Tong Tiangen > --- > arch/arm64/lib/copy_from_user.S | 8 > arch/arm64/lib/copy_to_user.S | 8 All of these changes are to the *kernel* accesses performed as part of copy to/from user, and have nothing to do with userspace, so it does not make sense to mark these as UACCESS. Do we *actually* need to recover from failues on these accesses? Looking at _copy_from_user(), the kernel will immediately follow this up with a memset() to the same address which will be fatal anyway, so this is only punting the failure for a few instructions. If we really need to recover from certain accesses to kernel memory we should add a new EX_TYPE_KACCESS_ERR_ZERO_MC or similar, but we need a strong rationale as to why that's useful. As things stand I do not beleive it makes sense for copy to/from user specifically. > arch/arm64/mm/extable.c | 8 > 3 files changed, 12 insertions(+), 12 deletions(-) > > diff --git a/arch/arm64/lib/copy_from_user.S b/arch/arm64/lib/copy_from_user.S > index 34e317907524..402dd48a4f93 100644 > --- a/arch/arm64/lib/copy_from_user.S > +++ b/arch/arm64/lib/copy_from_user.S > @@ -25,7 +25,7 @@ > .endm > > .macro strb1 reg, ptr, val > - strb \reg, [\ptr], \val > + USER(9998f, strb \reg, [\ptr], \val) > .endm > > .macro ldrh1 reg, ptr, val > @@ -33,7 +33,7 @@ > .endm > > .macro strh1 reg, ptr, val > - strh \reg, [\ptr], \val > + USER(9998f, strh \reg, [\ptr], \val) > .endm > > .macro ldr1 reg, ptr, val > @@ -41,7 +41,7 @@ > .endm > > .macro str1 reg, ptr, val > - str \reg, [\ptr], \val > + USER(9998f, str \reg, [\ptr], \val) > .endm > > .macro ldp1 reg1, reg2, ptr, val > @@ -49,7 +49,7 @@ > .endm > > .macro stp1 reg1, reg2, ptr, val > - stp \reg1, \reg2, [\ptr], \val > + USER(9998f, stp \reg1, \reg2, [\ptr], \val) > .endm > > end .reqx5 > diff --git a/arch/arm64/lib/copy_to_user.S b/arch/arm64/lib/copy_to_user.S > index 802231772608..4134bdb3a8b0 100644 > --- a/arch/arm64/lib/copy_to_user.S > +++ b/arch/arm64/lib/copy_to_user.S > @@ -20,7 +20,7 @@ > * x0 - bytes not copied > */ > .macro ldrb1 reg, ptr, val > - ldrb \reg, [\ptr], \val > + USER(9998f, ldrb \reg, [\ptr], \val) > .endm > > .macro strb1 reg, ptr, val > @@ -28,7 +28,7 @@ > .endm > > .macro ldrh1 reg, ptr, val > - ldrh \reg, [\ptr], \val > + USER(9998f, ldrh \reg, [\ptr], \val) > .endm > > .macro strh1 reg, ptr, val > @@ -36,7 +36,7 @@ > .endm > > .macro ldr1 reg, ptr, val > - ldr \reg, [\ptr], \val > + USER(9998f, ldr \reg, [\ptr], \val) > .endm > > .macro str1 reg, ptr, val > @@ -44,7 +44,7 @@ > .endm > > .macro ldp1 reg1, reg2, ptr, val > - ldp \reg1, \reg2, [\ptr], \val > + USER(9998f, ldp \reg1, \reg2, [\ptr], \val) > .endm > > .macro stp1 reg1, reg2, ptr, val > diff --git a/arch/arm64/mm/extable.c b/arch/arm64/mm/extable.c > index c301dcf6335f..8ca8d9639f9f 100644 > --- a/arch/arm64/mm/extable.c > +++ b/arch/arm64/mm/extable.c > @@ -86,10 +86,10 @@ bool fixup_exception_mc(struct pt_regs *regs) > if (!ex) > return false; > > - /* > - * This is not complete, More Machine check safe extable type can > - * be processed here. > - */ > + switch (ex->type) { > + case EX_TYPE_UACCESS_ERR_ZERO: > + return ex_handler_uaccess_err_zero(ex, regs); > + } This addition specifically makes sense to me, so can you split this into a separate patch? Thanks, Mark.
Re: [PATCH -next v5 6/8] arm64: add support for machine check error safe
On Sat, May 28, 2022 at 06:50:54AM +, Tong Tiangen wrote: > During the processing of arm64 kernel hardware memory errors(do_sea()), if > the errors is consumed in the kernel, the current processing is panic. > However, it is not optimal. > > Take uaccess for example, if the uaccess operation fails due to memory > error, only the user process will be affected, kill the user process > and isolate the user page with hardware memory errors is a better choice. > > This patch only enable machine error check framework, it add exception > fixup before kernel panic in do_sea() and only limit the consumption of > hardware memory errors in kernel mode triggered by user mode processes. > If fixup successful, panic can be avoided. > > Signed-off-by: Tong Tiangen > --- > arch/arm64/Kconfig | 1 + > arch/arm64/include/asm/extable.h | 1 + > arch/arm64/mm/extable.c | 17 + > arch/arm64/mm/fault.c| 27 ++- > 4 files changed, 45 insertions(+), 1 deletion(-) > > diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig > index aaeb70358979..a3b12ff0cd7f 100644 > --- a/arch/arm64/Kconfig > +++ b/arch/arm64/Kconfig > @@ -19,6 +19,7 @@ config ARM64 > select ARCH_ENABLE_SPLIT_PMD_PTLOCK if PGTABLE_LEVELS > 2 > select ARCH_ENABLE_THP_MIGRATION if TRANSPARENT_HUGEPAGE > select ARCH_HAS_CACHE_LINE_SIZE > + select ARCH_HAS_COPY_MC if ACPI_APEI_GHES > select ARCH_HAS_CURRENT_STACK_POINTER > select ARCH_HAS_DEBUG_VIRTUAL > select ARCH_HAS_DEBUG_VM_PGTABLE > diff --git a/arch/arm64/include/asm/extable.h > b/arch/arm64/include/asm/extable.h > index 72b0e71cc3de..f80ebd0addfd 100644 > --- a/arch/arm64/include/asm/extable.h > +++ b/arch/arm64/include/asm/extable.h > @@ -46,4 +46,5 @@ bool ex_handler_bpf(const struct exception_table_entry *ex, > #endif /* !CONFIG_BPF_JIT */ > > bool fixup_exception(struct pt_regs *regs); > +bool fixup_exception_mc(struct pt_regs *regs); > #endif > diff --git a/arch/arm64/mm/extable.c b/arch/arm64/mm/extable.c > index 228d681a8715..c301dcf6335f 100644 > --- a/arch/arm64/mm/extable.c > +++ b/arch/arm64/mm/extable.c > @@ -9,6 +9,7 @@ > > #include > #include > +#include > > static inline unsigned long > get_ex_fixup(const struct exception_table_entry *ex) > @@ -76,3 +77,19 @@ bool fixup_exception(struct pt_regs *regs) > > BUG(); > } > + > +bool fixup_exception_mc(struct pt_regs *regs) > +{ > + const struct exception_table_entry *ex; > + > + ex = search_exception_tables(instruction_pointer(regs)); > + if (!ex) > + return false; > + > + /* > + * This is not complete, More Machine check safe extable type can > + * be processed here. > + */ > + > + return false; > +} > diff --git a/arch/arm64/mm/fault.c b/arch/arm64/mm/fault.c > index c5e11768e5c1..b262bd282a89 100644 > --- a/arch/arm64/mm/fault.c > +++ b/arch/arm64/mm/fault.c > @@ -696,6 +696,29 @@ static int do_bad(unsigned long far, unsigned long esr, > struct pt_regs *regs) > return 1; /* "fault" */ > } > > +static bool arm64_do_kernel_sea(unsigned long addr, unsigned int esr, > + struct pt_regs *regs, int sig, int code) > +{ > + if (!IS_ENABLED(CONFIG_ARCH_HAS_COPY_MC)) > + return false; > + > + if (user_mode(regs) || !current->mm) > + return false; What's the `!current->mm` check for? > + > + if (apei_claim_sea(regs) < 0) > + return false; > + > + if (!fixup_exception_mc(regs)) > + return false; I thought we still wanted to signal the task in this case? Or do you expect to add that into `fixup_exception_mc()` ? > + > + set_thread_esr(0, esr); Why are we not setting the address? Is that deliberate, or an oversight? > + > + arm64_force_sig_fault(sig, code, addr, > + "Uncorrected hardware memory error in kernel-access\n"); I think the wording here is misleading since we don't expect to recover from accesses to kernel memory, and would be better as something like: "Uncorrected memory error on access to user memory\n" Thanks, Mark. > + > + return true; > +} > + > static int do_sea(unsigned long far, unsigned long esr, struct pt_regs *regs) > { > const struct fault_info *inf; > @@ -721,7 +744,9 @@ static int do_sea(unsigned long far, unsigned long esr, > struct pt_regs *regs) >*/ > siaddr = untagged_addr(far); > } > - arm64_notify_die(inf->name, regs, inf->sig, inf->code, siaddr, esr); > + > + if (!arm64_do_kernel_sea(siaddr, esr, regs, inf->sig, inf->code)) > + arm64_notify_die(inf->name, regs, inf->sig, inf->code, siaddr, > esr); > > return 0; > } > -- > 2.25.1 >
Re: [PATCH -next v5 4/8] arm64: extable: cleanup redundant extable type EX_TYPE_FIXUP
On Sat, May 28, 2022 at 06:50:52AM +, Tong Tiangen wrote: > Currently, extable type EX_TYPE_FIXUP is no place to use, We can safely > remove it. > > Suggested-by: Mark Rutland > Signed-off-by: Tong Tiangen Acked-by: Mark Rutland Mark. > --- > arch/arm64/include/asm/asm-extable.h | 20 > arch/arm64/mm/extable.c | 9 - > 2 files changed, 4 insertions(+), 25 deletions(-) > > diff --git a/arch/arm64/include/asm/asm-extable.h > b/arch/arm64/include/asm/asm-extable.h > index d01bd94cc4c2..1f2974467273 100644 > --- a/arch/arm64/include/asm/asm-extable.h > +++ b/arch/arm64/include/asm/asm-extable.h > @@ -3,11 +3,10 @@ > #define __ASM_ASM_EXTABLE_H > > #define EX_TYPE_NONE 0 > -#define EX_TYPE_FIXUP1 > -#define EX_TYPE_BPF 2 > -#define EX_TYPE_UACCESS_ERR_ZERO 3 > -#define EX_TYPE_LOAD_UNALIGNED_ZEROPAD 4 > -#define EX_TYPE_KACCESS_ERR_ZERO 5 > +#define EX_TYPE_BPF 1 > +#define EX_TYPE_UACCESS_ERR_ZERO 2 > +#define EX_TYPE_LOAD_UNALIGNED_ZEROPAD 3 > +#define EX_TYPE_KACCESS_ERR_ZERO 4 > > #ifdef __ASSEMBLY__ > > @@ -20,14 +19,6 @@ > .short (data); \ > .popsection; > > -/* > - * Create an exception table entry for `insn`, which will branch to `fixup` > - * when an unhandled fault is taken. > - */ > - .macro _asm_extable, insn, fixup > - __ASM_EXTABLE_RAW(\insn, \fixup, EX_TYPE_FIXUP, 0) > - .endm > - > /* > * Create an exception table entry for uaccess `insn`, which will branch to > `fixup` > * when an unhandled fault is taken. > @@ -62,9 +53,6 @@ > ".short (" data ")\n" \ > ".popsection\n" > > -#define _ASM_EXTABLE(insn, fixup) \ > - __ASM_EXTABLE_RAW(#insn, #fixup, __stringify(EX_TYPE_FIXUP), "0") > - > #define EX_DATA_REG_ERR_SHIFT0 > #define EX_DATA_REG_ERR GENMASK(4, 0) > #define EX_DATA_REG_ZERO_SHIFT 5 > diff --git a/arch/arm64/mm/extable.c b/arch/arm64/mm/extable.c > index 056591e5ca80..228d681a8715 100644 > --- a/arch/arm64/mm/extable.c > +++ b/arch/arm64/mm/extable.c > @@ -16,13 +16,6 @@ get_ex_fixup(const struct exception_table_entry *ex) > return ((unsigned long)&ex->fixup + ex->fixup); > } > > -static bool ex_handler_fixup(const struct exception_table_entry *ex, > - struct pt_regs *regs) > -{ > - regs->pc = get_ex_fixup(ex); > - return true; > -} > - > static bool ex_handler_uaccess_err_zero(const struct exception_table_entry > *ex, > struct pt_regs *regs) > { > @@ -72,8 +65,6 @@ bool fixup_exception(struct pt_regs *regs) > return false; > > switch (ex->type) { > - case EX_TYPE_FIXUP: > - return ex_handler_fixup(ex, regs); > case EX_TYPE_BPF: > return ex_handler_bpf(ex, regs); > case EX_TYPE_UACCESS_ERR_ZERO: > -- > 2.25.1 >
Re:Re: [PATCH v4] powerpc:85xx: Add missing of_node_put() in sgy_cst1000
At 2022-06-17 16:27:03, conor.doo...@microchip.com wrote: >On 17/06/2022 09:17, Liang He wrote: >> >> >> >> At 2022-06-17 14:53:13, "Christophe Leroy" >> wrote: >>> >>> >>> Le 17/06/2022 à 08:45, Liang He a écrit : At 2022-06-17 14:28:56, "Christophe Leroy" wrote: > > > Le 17/06/2022 à 08:08, Liang He a écrit : >> In gpio_halt_probe(), of_find_matching_node() will return a node >> pointer with refcount incremented. We should use of_node_put() in >> fail path or when it is not used anymore. >> >> Signed-off-by: Liang He >> --- >> changelog: >> v4: reuse exist 'err' and use a simple code style, advised by CJ >> v3: use local 'child_node' advised by Michael. >> v2: use goto-label patch style advised by Christophe Leroy. >> v1: add of_node_put() before each exit. >> >> arch/powerpc/platforms/85xx/sgy_cts1000.c | 35 >> ++- >> 1 file changed, 22 insertions(+), 13 deletions(-) >> >> diff --git a/arch/powerpc/platforms/85xx/sgy_cts1000.c >> b/arch/powerpc/platforms/85xx/sgy_cts1000.c >> index 98ae64075193..e4588943fe7e 100644 >> --- a/arch/powerpc/platforms/85xx/sgy_cts1000.c >> +++ b/arch/powerpc/platforms/85xx/sgy_cts1000.c >> @@ -71,6 +71,7 @@ static int gpio_halt_probe(struct platform_device >> *pdev) >> { >> enum of_gpio_flags flags; >> struct device_node *node = pdev->dev.of_node; >> +struct device_node *child_node; >> int gpio, err, irq; >> int trigger; >> >> @@ -78,26 +79,29 @@ static int gpio_halt_probe(struct platform_device >> *pdev) >> return -ENODEV; >> >> /* If there's no matching child, this isn't really an error */ >> -halt_node = of_find_matching_node(node, child_match); >> -if (!halt_node) >> +child_node = of_find_matching_node(node, child_match); >> +if (!child_node) >> return 0; >> >> /* Technically we could just read the first one, but punish >> * DT writers for invalid form. */ >> -if (of_gpio_count(halt_node) != 1) >> -return -EINVAL; >> +if (of_gpio_count(child_node) != 1) { >> +err = -EINVAL; >> +goto err_put; >> +} >> >> /* Get the gpio number relative to the dynamic base. */ >> -gpio = of_get_gpio_flags(halt_node, 0, &flags); >> -if (!gpio_is_valid(gpio)) >> -return -EINVAL; >> +gpio = of_get_gpio_flags(child_node, 0, &flags); >> +if (!gpio_is_valid(gpio)) { >> +err = -EINVAL; >> +gotot err_put; > > Did you test the build ? Sorry for this fault. In fact, I am still finding an efficient way to building different arch source code as I only have x86-64. Now I am try using QEMU. Anyway, sorry for this fault. >>> >>> You can find cross compilers for most architectures for x86-64 here : >>> https://mirrors.edge.kernel.org/pub/tools/crosstool/ >>> >>> Christophe >> >> Hi, Christophe and Conor. >> >> Sorry to trouble you again. >> >> Now I only know how to quickly identify the refcounting bugs, but I cannot >> efficiently give a build test. >> >> For example, I use the cross compilers 'powerpc-linux-gnu-gcc' to compile >> 'arch/powerpc/platforms/85xx/sgy_cts1000.c' with -fsyntax-only flag. >> But I meet too many header file missing errors. Even if I add some 'include' >> pathes, e.g., ./arch/powerpc/include, ./include, >> there are still too many other errors. >> >> So if there is any efficient way to check my patch code to avoid 'gotot' >> error again. > >idk anything about powerpc, but what I find is a nice way to get a compiler >for an arch I don't use is to search on lore.kernel.org for a 0day robot >build error since it gives instructions for building on that arch. >For example: >https://lore.kernel.org/linuxppc-dev/202206060910.ryntfqdi-...@intel.com/ > > >In this case, your bug seems obvious? You typed "gotot" instead of "goto". > >Hope that helps, >Conor. > >> >> Thanks again, Christophe and Conor. >> >> Liang Thanks so much, I will try it.
Re: [PATCH -next v5 3/8] arm64: extable: move _cond_extable to _cond_uaccess_extable
On Sat, May 28, 2022 at 06:50:51AM +, Tong Tiangen wrote: > Currently, We use _cond_extable for cache maintenance uaccess helper > caches_clean_inval_user_pou(), so this should be moved over to > EX_TYPE_UACCESS_ERR_ZERO and rename _cond_extable to _cond_uaccess_extable > for clarity. > > Suggested-by: Mark Rutland > Signed-off-by: Tong Tiangen Acked-by: Mark Rutland Mark. > --- > arch/arm64/include/asm/asm-extable.h | 6 +++--- > arch/arm64/include/asm/assembler.h | 4 ++-- > 2 files changed, 5 insertions(+), 5 deletions(-) > > diff --git a/arch/arm64/include/asm/asm-extable.h > b/arch/arm64/include/asm/asm-extable.h > index 9c94ac1f082c..d01bd94cc4c2 100644 > --- a/arch/arm64/include/asm/asm-extable.h > +++ b/arch/arm64/include/asm/asm-extable.h > @@ -40,9 +40,9 @@ > * Create an exception table entry for `insn` if `fixup` is provided. > Otherwise > * do nothing. > */ > - .macro _cond_extable, insn, fixup > - .ifnc \fixup, > - _asm_extable\insn, \fixup > + .macro _cond_uaccess_extable, insn, fixup > + .ifnc \fixup, > + _asm_extable_uaccess\insn, \fixup > .endif > .endm > > diff --git a/arch/arm64/include/asm/assembler.h > b/arch/arm64/include/asm/assembler.h > index 8c5a61aeaf8e..dc422fa437c2 100644 > --- a/arch/arm64/include/asm/assembler.h > +++ b/arch/arm64/include/asm/assembler.h > @@ -423,7 +423,7 @@ alternative_endif > b.lo.Ldcache_op\@ > dsb \domain > > - _cond_extable .Ldcache_op\@, \fixup > + _cond_uaccess_extable .Ldcache_op\@, \fixup > .endm > > /* > @@ -462,7 +462,7 @@ alternative_endif > dsb ish > isb > > - _cond_extable .Licache_op\@, \fixup > + _cond_uaccess_extable .Licache_op\@, \fixup > .endm > > /* > -- > 2.25.1 >
Re: [PATCH 2/4] arm64/hugetlb: Implement arm64 specific hugetlb_mask_last_page
On Thu, Jun 16, 2022 at 02:05:16PM -0700, Mike Kravetz wrote: > From: Baolin Wang > > The HugeTLB address ranges are linearly scanned during fork, unmap and > remap operations, and the linear scan can skip to the end of range mapped > by the page table page if hitting a non-present entry, which can help > to speed linear scanning of the HugeTLB address ranges. > > So hugetlb_mask_last_page() is introduced to help to update the address in > the loop of HugeTLB linear scanning with getting the last huge page mapped > by the associated page table page[1], when a non-present entry is encountered. > > Considering ARM64 specific cont-pte/pmd size HugeTLB, this patch implemented > an ARM64 specific hugetlb_mask_last_page() to help this case. > > [1] > https://lore.kernel.org/linux-mm/20220527225849.284839-1-mike.krav...@oracle.com/ > > Signed-off-by: Baolin Wang > Signed-off-by: Mike Kravetz Acked-by: Muchun Song Thanks.
Re: [PATCH -next v5 2/8] arm64: extable: make uaaccess helper use extable type EX_TYPE_UACCESS_ERR_ZERO
On Sat, May 28, 2022 at 06:50:50AM +, Tong Tiangen wrote: > Currnetly, the extable type used by __arch_copy_from/to_user() is > EX_TYPE_FIXUP. In fact, It is more clearly to use meaningful > EX_TYPE_UACCESS_*. > > Suggested-by: Mark Rutland > Signed-off-by: Tong Tiangen > --- > arch/arm64/include/asm/asm-extable.h | 8 > arch/arm64/include/asm/asm-uaccess.h | 12 ++-- > 2 files changed, 14 insertions(+), 6 deletions(-) > > diff --git a/arch/arm64/include/asm/asm-extable.h > b/arch/arm64/include/asm/asm-extable.h > index 56ebe183e78b..9c94ac1f082c 100644 > --- a/arch/arm64/include/asm/asm-extable.h > +++ b/arch/arm64/include/asm/asm-extable.h > @@ -28,6 +28,14 @@ > __ASM_EXTABLE_RAW(\insn, \fixup, EX_TYPE_FIXUP, 0) > .endm > > +/* > + * Create an exception table entry for uaccess `insn`, which will branch to > `fixup` > + * when an unhandled fault is taken. > + * ex->data = ~0 means both reg_err and reg_zero is set to wzr(x31). > + */ > + .macro _asm_extable_uaccess, insn, fixup > + __ASM_EXTABLE_RAW(\insn, \fixup, EX_TYPE_UACCESS_ERR_ZERO, ~0) > + .endm I'm not too keen on using `~0` here, since that also sets other bits in the data field, and its somewhat opaque. How painful is it to generate the data fields as with the C version of this macro, so that we can pass in wzr explciitly for the two sub-fields? Other than that, this looks good to me. Thanks, Mark. > /* > * Create an exception table entry for `insn` if `fixup` is provided. > Otherwise > * do nothing. > diff --git a/arch/arm64/include/asm/asm-uaccess.h > b/arch/arm64/include/asm/asm-uaccess.h > index 0557af834e03..75b211c98dea 100644 > --- a/arch/arm64/include/asm/asm-uaccess.h > +++ b/arch/arm64/include/asm/asm-uaccess.h > @@ -61,7 +61,7 @@ alternative_else_nop_endif > > #define USER(l, x...)\ > :x; \ > - _asm_extableb, l > + _asm_extable_uaccessb, l > > /* > * Generate the assembly for LDTR/STTR with exception table entries. > @@ -73,8 +73,8 @@ alternative_else_nop_endif > 8889:ldtr\reg2, [\addr, #8]; > add \addr, \addr, \post_inc; > > - _asm_extableb,\l; > - _asm_extable8889b,\l; > + _asm_extable_uaccessb, \l; > + _asm_extable_uaccess8889b, \l; > .endm > > .macro user_stp l, reg1, reg2, addr, post_inc > @@ -82,14 +82,14 @@ alternative_else_nop_endif > 8889:sttr\reg2, [\addr, #8]; > add \addr, \addr, \post_inc; > > - _asm_extableb,\l; > - _asm_extable8889b,\l; > + _asm_extable_uaccessb,\l; > + _asm_extable_uaccess8889b,\l; > .endm > > .macro user_ldst l, inst, reg, addr, post_inc > :\inst \reg, [\addr]; > add \addr, \addr, \post_inc; > > - _asm_extableb,\l; > + _asm_extable_uaccessb, \l; > .endm > #endif > -- > 2.25.1 >
Re: [PATCH -next v5 1/8] arm64: extable: add new extable type EX_TYPE_KACCESS_ERR_ZERO support
On Sat, May 28, 2022 at 06:50:49AM +, Tong Tiangen wrote: > Currently, The extable type EX_TYPE_UACCESS_ERR_ZERO is used by > __get/put_kernel_nofault(), but those helpers are not uaccess type, so we > add a new extable type EX_TYPE_KACCESS_ERR_ZERO which can be used by > __get/put_kernel_no_fault(). > > This is also to prepare for distinguishing the two types in machine check > safe process. > > Suggested-by: Mark Rutland > Signed-off-by: Tong Tiangen This looks good to me, so modulo one nit below: Acked-by: Mark Rutland > --- > arch/arm64/include/asm/asm-extable.h | 13 > arch/arm64/include/asm/uaccess.h | 94 ++-- > arch/arm64/mm/extable.c | 1 + > 3 files changed, 61 insertions(+), 47 deletions(-) > > diff --git a/arch/arm64/include/asm/asm-extable.h > b/arch/arm64/include/asm/asm-extable.h > index c39f2437e08e..56ebe183e78b 100644 > --- a/arch/arm64/include/asm/asm-extable.h > +++ b/arch/arm64/include/asm/asm-extable.h > @@ -7,6 +7,7 @@ > #define EX_TYPE_BPF 2 > #define EX_TYPE_UACCESS_ERR_ZERO 3 > #define EX_TYPE_LOAD_UNALIGNED_ZEROPAD 4 > +#define EX_TYPE_KACCESS_ERR_ZERO 5 Could we please renumber this so the UACCESS and KACCESS definitions are next to one another, i.e. #define EX_TYPE_BPF 2 #define EX_TYPE_UACCESS_ERR_ZERO3 #define EX_TYPE_KACCESS_ERR_ZERO4 #define EX_TYPE_LOAD_UNALIGNED_ZEROPAD 5 Thanks, Mark. > > #ifdef __ASSEMBLY__ > > @@ -73,9 +74,21 @@ > EX_DATA_REG(ZERO, zero) \ > ")") > > +#define _ASM_EXTABLE_KACCESS_ERR_ZERO(insn, fixup, err, zero) > \ > + __DEFINE_ASM_GPR_NUMS \ > + __ASM_EXTABLE_RAW(#insn, #fixup,\ > + __stringify(EX_TYPE_KACCESS_ERR_ZERO),\ > + "(" \ > + EX_DATA_REG(ERR, err) " | " \ > + EX_DATA_REG(ZERO, zero) \ > + ")") > + > #define _ASM_EXTABLE_UACCESS_ERR(insn, fixup, err) \ > _ASM_EXTABLE_UACCESS_ERR_ZERO(insn, fixup, err, wzr) > > +#define _ASM_EXTABLE_KACCESS_ERR(insn, fixup, err) \ > + _ASM_EXTABLE_KACCESS_ERR_ZERO(insn, fixup, err, wzr) > + > #define EX_DATA_REG_DATA_SHIFT 0 > #define EX_DATA_REG_DATA GENMASK(4, 0) > #define EX_DATA_REG_ADDR_SHIFT 5 > diff --git a/arch/arm64/include/asm/uaccess.h > b/arch/arm64/include/asm/uaccess.h > index 63f9c828f1a7..2fc9f0861769 100644 > --- a/arch/arm64/include/asm/uaccess.h > +++ b/arch/arm64/include/asm/uaccess.h > @@ -232,34 +232,34 @@ static inline void __user *__uaccess_mask_ptr(const > void __user *ptr) > * The "__xxx_error" versions set the third argument to -EFAULT if an error > * occurs, and leave it unchanged on success. > */ > -#define __get_mem_asm(load, reg, x, addr, err) > \ > +#define __get_mem_asm(load, reg, x, addr, err, type) \ > asm volatile( \ > "1: " load "" reg "1, [%2]\n" \ > "2:\n" \ > - _ASM_EXTABLE_UACCESS_ERR_ZERO(1b, 2b, %w0, %w1) \ > + _ASM_EXTABLE_##type##ACCESS_ERR_ZERO(1b, 2b, %w0, %w1) \ > : "+r" (err), "=&r" (x) \ > : "r" (addr)) > > -#define __raw_get_mem(ldr, x, ptr, err) > \ > -do { \ > - unsigned long __gu_val; \ > - switch (sizeof(*(ptr))) { \ > - case 1: \ > - __get_mem_asm(ldr "b", "%w", __gu_val, (ptr), (err)); \ > - break; \ > - case 2: \ > - __get_mem_asm(ldr "h", "%w", __gu_val, (ptr), (err)); \ > - break; \ > - case 4: \ > - __get_mem_asm(ldr, "%w", __gu_val, (ptr), (err)); \ > - break; \ > - case 8: \ > - __get_mem_asm(ldr, "%x", __gu_val, (ptr), (err)); \ > - break; \ > - default:\ > - BUILD_BUG();
Re:Re: [PATCH v4] powerpc:85xx: Add missing of_node_put() in sgy_cst1000
At 2022-06-17 14:53:13, "Christophe Leroy" wrote: > > >Le 17/06/2022 à 08:45, Liang He a écrit : >> >> >> >> At 2022-06-17 14:28:56, "Christophe Leroy" >> wrote: >>> >>> >>> Le 17/06/2022 à 08:08, Liang He a écrit : In gpio_halt_probe(), of_find_matching_node() will return a node pointer with refcount incremented. We should use of_node_put() in fail path or when it is not used anymore. Signed-off-by: Liang He --- changelog: v4: reuse exist 'err' and use a simple code style, advised by CJ v3: use local 'child_node' advised by Michael. v2: use goto-label patch style advised by Christophe Leroy. v1: add of_node_put() before each exit. arch/powerpc/platforms/85xx/sgy_cts1000.c | 35 ++- 1 file changed, 22 insertions(+), 13 deletions(-) diff --git a/arch/powerpc/platforms/85xx/sgy_cts1000.c b/arch/powerpc/platforms/85xx/sgy_cts1000.c index 98ae64075193..e4588943fe7e 100644 --- a/arch/powerpc/platforms/85xx/sgy_cts1000.c +++ b/arch/powerpc/platforms/85xx/sgy_cts1000.c @@ -71,6 +71,7 @@ static int gpio_halt_probe(struct platform_device *pdev) { enum of_gpio_flags flags; struct device_node *node = pdev->dev.of_node; + struct device_node *child_node; int gpio, err, irq; int trigger; @@ -78,26 +79,29 @@ static int gpio_halt_probe(struct platform_device *pdev) return -ENODEV; /* If there's no matching child, this isn't really an error */ - halt_node = of_find_matching_node(node, child_match); - if (!halt_node) + child_node = of_find_matching_node(node, child_match); + if (!child_node) return 0; /* Technically we could just read the first one, but punish * DT writers for invalid form. */ - if (of_gpio_count(halt_node) != 1) - return -EINVAL; + if (of_gpio_count(child_node) != 1) { + err = -EINVAL; + goto err_put; + } /* Get the gpio number relative to the dynamic base. */ - gpio = of_get_gpio_flags(halt_node, 0, &flags); - if (!gpio_is_valid(gpio)) - return -EINVAL; + gpio = of_get_gpio_flags(child_node, 0, &flags); + if (!gpio_is_valid(gpio)) { + err = -EINVAL; + gotot err_put; >>> >>> Did you test the build ? >> >> Sorry for this fault. >> >> In fact, I am still finding an efficient way to building different arch >> source code as I only have x86-64. >> >> Now I am try using QEMU. >> >> Anyway, sorry for this fault. > >You can find cross compilers for most architectures for x86-64 here : >https://mirrors.edge.kernel.org/pub/tools/crosstool/ > >Christophe Hi, Christophe and Conor. Sorry to trouble you again. Now I only know how to quickly identify the refcounting bugs, but I cannot efficiently give a build test. For example, I use the cross compilers 'powerpc-linux-gnu-gcc' to compile 'arch/powerpc/platforms/85xx/sgy_cts1000.c' with -fsyntax-only flag. But I meet too many header file missing errors. Even if I add some 'include' pathes, e.g., ./arch/powerpc/include, ./include, there are still too many other errors. So if there is any efficient way to check my patch code to avoid 'gotot' error again. Thanks again, Christophe and Conor. Liang
Re: [PATCH 1/4] hugetlb: skip to end of PT page mapping when pte not present
On Thu, Jun 16, 2022 at 02:05:15PM -0700, Mike Kravetz wrote: > HugeTLB address ranges are linearly scanned during fork, unmap and > remap operations. If a non-present entry is encountered, the code > currently continues to the next huge page aligned address. However, > a non-present entry implies that the page table page for that entry > is not present. Therefore, the linear scan can skip to the end of > range mapped by the page table page. This can speed operations on > large sparsely populated hugetlb mappings. > > Create a new routine hugetlb_mask_last_page() that will return an > address mask. When the mask is ORed with an address, the result > will be the address of the last huge page mapped by the associated > page table page. Use this mask to update addresses in routines which > linearly scan hugetlb address ranges when a non-present pte is > encountered. > > hugetlb_mask_last_page is related to the implementation of > huge_pte_offset as hugetlb_mask_last_page is called when huge_pte_offset > returns NULL. This patch only provides a complete hugetlb_mask_last_page > implementation when CONFIG_ARCH_WANT_GENERAL_HUGETLB is defined. > Architectures which provide their own versions of huge_pte_offset can also > provide their own version of hugetlb_mask_last_page. > > Signed-off-by: Mike Kravetz > Tested-by: Baolin Wang > Reviewed-by: Baolin Wang It'll be more efficient, Thanks. Acked-by: Muchun Song
[PATCH] powerpc: powernv: Fix refcount leak in opal
In opal_imc_init_dev(), of_find_compatible_node() will return a node pointer with refcount incremented. We should use of_node_put() when it is not used anymore. Signed-off-by: Liang He --- arch/powerpc/platforms/powernv/opal.c | 1 + 1 file changed, 1 insertion(+) diff --git a/arch/powerpc/platforms/powernv/opal.c b/arch/powerpc/platforms/powernv/opal.c index 55a8fbfdb5b2..d86cc48a10aa 100644 --- a/arch/powerpc/platforms/powernv/opal.c +++ b/arch/powerpc/platforms/powernv/opal.c @@ -952,6 +952,7 @@ static void __init opal_imc_init_dev(void) np = of_find_compatible_node(NULL, NULL, IMC_DTB_COMPAT); if (np) of_platform_device_create(np, NULL, NULL); + of_node_put(np); } static int kopald(void *unused) -- 2.25.1
[PATCH] powerc: Update asm-prototypes.h comment
This header was recently cleaned up in commit 76222808fc25 ("powerpc: Move C prototypes out of asm-prototypes.h"), update the comment to reflect it's proper purpose. Signed-off-by: Michael Ellerman --- arch/powerpc/include/asm/asm-prototypes.h | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/arch/powerpc/include/asm/asm-prototypes.h b/arch/powerpc/include/asm/asm-prototypes.h index d995c65d18ab..eda6dba9378f 100644 --- a/arch/powerpc/include/asm/asm-prototypes.h +++ b/arch/powerpc/include/asm/asm-prototypes.h @@ -2,8 +2,9 @@ #ifndef _ASM_POWERPC_ASM_PROTOTYPES_H #define _ASM_POWERPC_ASM_PROTOTYPES_H /* - * This file is for prototypes of C functions that are only called - * from asm, and any associated variables. + * This file is for C prototypes of asm symbols that are EXPORTed. + * It allows the modversions logic to see their prototype and + * generate proper CRCs for them. * * Copyright 2016, Daniel Axtens, IBM Corporation. */ -- 2.35.3