Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752117AbbGMVfi (ORCPT ); Mon, 13 Jul 2015 17:35:38 -0400 Received: from mail-wi0-f170.google.com ([209.85.212.170]:37466 "EHLO mail-wi0-f170.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751554AbbGMVfg (ORCPT ); Mon, 13 Jul 2015 17:35:36 -0400 MIME-Version: 1.0 In-Reply-To: <55A392AB.9030702@imgtec.com> References: <20150712230402.11177.11848.stgit@bhelgaas-glaptop2.roam.corp.google.com> <20150712231120.11177.53145.stgit@bhelgaas-glaptop2.roam.corp.google.com> <55A392AB.9030702@imgtec.com> From: Bjorn Helgaas Date: Mon, 13 Jul 2015 16:35:15 -0500 Message-ID: Subject: Re: [PATCH 4/9] MIPS: MT: Remove "weak" from vpe_run() declaration To: James Hogan Cc: Ralf Baechle , Andrew Bresticker , "linux-mips@linux-mips.org" , "linux-kernel@vger.kernel.org" Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 3545 Lines: 103 On Mon, Jul 13, 2015 at 5:27 AM, James Hogan wrote: > On 13/07/15 00:11, Bjorn Helgaas wrote: >> Weak header file declarations are error-prone because they make every >> definition weak, and the linker chooses one based on link order (see >> 10629d711ed7 ("PCI: Remove __weak annotation from pcibios_get_phb_of_node >> decl")). >> >> That's not a problem for vpe_run() because Kconfig ensures there's never >> more than one definition: >> >> - vpe_run() is defined in arch/mips/kernel/vpe-mt.c if >> CONFIG_MIPS_VPE_LOADER_MT=y >> >> - vpe_run() is defined in arch/mips/mti-malta/malta-amon.c if >> CONFIG_MIPS_CMP=y >> >> - CONFIG_MIPS_VPE_LOADER_MT cannot be set if CONFIG_MIPS_CMP=y >> >> But it's simpler to verify correctness if we remove "weak" from the picture >> and test the config symbols directly. >> >> Remove "weak" from the vpe_run() declaration and use #if to test whether a >> definition should be present. >> >> Signed-off-by: Bjorn Helgaas >> --- >> arch/mips/include/asm/vpe.h | 2 +- >> arch/mips/kernel/vpe.c | 10 +++++----- >> 2 files changed, 6 insertions(+), 6 deletions(-) >> >> diff --git a/arch/mips/include/asm/vpe.h b/arch/mips/include/asm/vpe.h >> index 7849f39..80e70db 100644 >> --- a/arch/mips/include/asm/vpe.h >> +++ b/arch/mips/include/asm/vpe.h >> @@ -122,7 +122,7 @@ void release_vpe(struct vpe *v); >> void *alloc_progmem(unsigned long len); >> void release_progmem(void *ptr); >> >> -int __weak vpe_run(struct vpe *v); >> +int vpe_run(struct vpe *v); >> void cleanup_tc(struct tc *tc); >> >> int __init vpe_module_init(void); >> diff --git a/arch/mips/kernel/vpe.c b/arch/mips/kernel/vpe.c >> index 72cae9f..04539d6 100644 >> --- a/arch/mips/kernel/vpe.c >> +++ b/arch/mips/kernel/vpe.c >> @@ -817,15 +817,11 @@ static int vpe_open(struct inode *inode, struct file *filp) >> >> static int vpe_release(struct inode *inode, struct file *filp) >> { >> +#if defined(CONFIG_MIPS_VPE_LOADER_MT) || defined(CONFIG_MIPS_CMP) > > That should be CONFIG_MIPS_VPE_LOADER_CMP, in which case the error case > in the #else bit is always dead code. This file is built only if > CONFIG_MIPS_VPE_LOADER, and the other ones are defined without prompts: > > config MIPS_VPE_LOADER_CMP > bool > default "y" > depends on MIPS_VPE_LOADER && MIPS_CMP > > config MIPS_VPE_LOADER_MT > bool > default "y" > depends on MIPS_VPE_LOADER && !MIPS_CMP > > I.e. one xor the other must be "y" when MIPS_VPE_LOADER=y. > > Maybe its worth just removing the weak and the vpe_run check? Yes, thanks a lot for clearing up my Kconfig confusion! >> struct vpe *v; >> Elf_Ehdr *hdr; >> int ret = 0; >> >> - if (!vpe_run) { >> - pr_warn("VPE loader: ELF load failed.\n"); >> - return -ENOEXEC; >> - } >> - >> v = get_vpe(aprp_cpu_index()); >> if (v == NULL) >> return -ENODEV; >> @@ -855,6 +851,10 @@ static int vpe_release(struct inode *inode, struct file *filp) >> v->plen = 0; >> >> return ret; >> +#else >> + pr_warn("VPE loader: ELF load failed.\n"); >> + return -ENOEXEC; >> +#endif >> } >> >> static ssize_t vpe_write(struct file *file, const char __user *buffer, >> > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/