From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-5.3 required=3.0 tests=BAYES_00,DKIM_INVALID, DKIM_SIGNED,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,SPF_HELO_NONE, SPF_PASS,URIBL_BLOCKED,USER_AGENT_SANE_1 autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 3AFC4C433E3 for ; Tue, 14 Jul 2020 10:54:15 +0000 (UTC) Received: from lists.ozlabs.org (lists.ozlabs.org [203.11.71.2]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 3F7F122200 for ; Tue, 14 Jul 2020 10:54:14 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=fail reason="signature verification failed" (2048-bit key) header.d=armlinux.org.uk header.i=@armlinux.org.uk header.b="br/lLXnO" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 3F7F122200 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=armlinux.org.uk Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=linuxppc-dev-bounces+linuxppc-dev=archiver.kernel.org@lists.ozlabs.org Received: from bilbo.ozlabs.org (lists.ozlabs.org [IPv6:2401:3900:2:1::3]) by lists.ozlabs.org (Postfix) with ESMTP id 4B5cpC6YrnzDqbp for ; Tue, 14 Jul 2020 20:54:11 +1000 (AEST) Authentication-Results: lists.ozlabs.org; spf=none (no SPF record) smtp.mailfrom=armlinux.org.uk (client-ip=2001:4d48:ad52:32c8:5054:ff:fe00:142; helo=pandora.armlinux.org.uk; envelope-from=linux+linuxppc-dev=lists.ozlabs.org@armlinux.org.uk; receiver=) Authentication-Results: lists.ozlabs.org; dmarc=pass (p=none dis=none) header.from=armlinux.org.uk Authentication-Results: lists.ozlabs.org; dkim=pass (2048-bit key; secure) header.d=armlinux.org.uk header.i=@armlinux.org.uk header.a=rsa-sha256 header.s=pandora-2019 header.b=br/lLXnO; dkim-atps=neutral Received: from pandora.armlinux.org.uk (pandora.armlinux.org.uk [IPv6:2001:4d48:ad52:32c8:5054:ff:fe00:142]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange ECDHE (P-256) server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by lists.ozlabs.org (Postfix) with ESMTPS id 4B5cNT2TZgzDqTT for ; Tue, 14 Jul 2020 20:35:21 +1000 (AEST) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=armlinux.org.uk; s=pandora-2019; h=Sender:In-Reply-To:Content-Type: MIME-Version:References:Message-ID:Subject:Cc:To:From:Date:Reply-To: Content-Transfer-Encoding:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Id: List-Help:List-Unsubscribe:List-Subscribe:List-Post:List-Owner:List-Archive; bh=HH5MekP+VtBs1CSslyllD2vFjpy9qUQDHcxcpFAt0Cg=; b=br/lLXnOwHnTXTnDX6To3QFoU 5NLL7twiv/zi55jQc2i6rniCYoe+CwuoZoXRLpMzlJ5B8LHXPP9rhENON1Sefkerm9r99IwNA6uNK z6J45dX+toA7Kn1Rx5vITCEYvxdk26KmmZpMD64niCJGWjRu8PFy0Lz3t5DGXbgBgqlFdHPsD2zxX HW5vAOevUYVjVN9dsAvNynv1uKWW2QnYNZOiZfViofIPFdlVqmbIrA7odZOJqba6UFSVQOd/Jn59v BMDnF37kNiRsSUJAso0l4OTQdduU7IjOt5FlVQik5hzAo5U5V/ukw9P0grSZdU3VN8dQynl1GYjeO 9+Jhfvk/w==; Received: from shell.armlinux.org.uk ([fd8f:7570:feb6:1:5054:ff:fe00:4ec]:39362) by pandora.armlinux.org.uk with esmtpsa (TLS1.3:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1jvIFv-00052s-E6; Tue, 14 Jul 2020 11:33:47 +0100 Received: from linux by shell.armlinux.org.uk with local (Exim 4.92) (envelope-from ) id 1jvIFh-0007E2-EL; Tue, 14 Jul 2020 11:33:33 +0100 Date: Tue, 14 Jul 2020 11:33:33 +0100 From: Russell King - ARM Linux admin To: Ard Biesheuvel Subject: Re: [PATCH 1/3] module: Rename module_alloc() to text_alloc() and move to kernel proper Message-ID: <20200714103333.GB1551@shell.armlinux.org.uk> References: <20200713182030.1418041-1-jarkko.sakkinen@linux.intel.com> <20200714095243.GB1442951@linux.intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.10.1 (2018-07-13) X-Mailman-Approved-At: Tue, 14 Jul 2020 20:36:21 +1000 X-BeenThere: linuxppc-dev@lists.ozlabs.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Linux on PowerPC Developers Mail List List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Catalin Marinas , Kefeng Wang , Paul Mackerras , Zong Li , Andi Kleen , Paul Burton , Vincent Whitchurch , Petr Mladek , Brian Gerst , Andy Lutomirski , Yonghong Song , Thomas Gleixner , Jiri Kosina , Anup Patel , Linux Kernel Mailing List , Philipp Rudo , Torsten Duwe , Masami Hiramatsu , Andrew Morton , Mark Rutland , "James E.J. Bottomley" , Vincent Chen , "open list:S390" , Joe Lawrence , Helge Deller , John Fastabend , Anil S Keshavamurthy , Andrey Ryabinin , Iurii Zaikin , Andrii Nakryiko , Vasily Gorbik , "moderated list:ARM PORT" , Daniel Axtens , Damien Le Moal , Peter Oberparleiter , Sean Christopherson , Martin KaFai Lau , Song Liu , Paul Walmsley , Heiko Carstens , Alexei Starovoitov , Jarkko Sakkinen , Atish Patra , Will Deacon , Daniel Borkmann , Masahiro Yamada , Nayna Jain , Ley Foon Tan , Christian Borntraeger , Dmitry Vyukov , Sami Tolvanen , "Naveen N. Rao" , Mao Han , Marco Elver , Steven Rostedt , Babu Moger , Borislav Petkov , Greentime Hu , Ben Dooks , Guan Xuetao , Thomas Bogendoerfer , "open list:PARISC ARCHITECTURE" , Jessica Yu , "open list:BPF JIT for MIPS \(32-BIT AND 64-BIT\)" , "David S. Miller" , Thiago Jung Bauermann , Peter Zijlstra , "open list:SPARC + UltraSPARC \(sparc/sparc64\)" , Sandipan Das , "H. Peter Anvin" , Amit Daniel Kachhap , Tiezhu Yang , Miroslav Benes , Jiri Olsa , "open list:RISC-V ARCHITECTURE" , Vincenzo Frascino , Anders Roxell , Sven Schnelle , "maintainer:X86 ARCHITECTURE \(32-BIT AND 64-BIT\)" , Mike Rapoport , Ingo Molnar , Albert Ou , "Paul E. McKenney" , Josh Poimboeuf , KP Singh , Gerald Schaefer , Nick Hu , "open list:BPF JIT for MIPS \(32-BIT AND 64-BIT\)" , "open list:MIPS" , Sergey Senozhatsky , Palmer Dabbelt , "open list:LINUX FOR POWERPC \(32-BIT AND 64-BIT\)" Errors-To: linuxppc-dev-bounces+linuxppc-dev=archiver.kernel.org@lists.ozlabs.org Sender: "Linuxppc-dev" On Tue, Jul 14, 2020 at 01:17:22PM +0300, Ard Biesheuvel wrote: > On Tue, 14 Jul 2020 at 12:53, Jarkko Sakkinen > wrote: > > > > On Mon, Jul 13, 2020 at 10:49:48PM +0300, Ard Biesheuvel wrote: > > > This patch suggests that there are other reasons why conflating > > > allocation of module space and allocating text pages for other uses > > > is a bad idea, but switching all users to text_alloc() is a step in > > > the wrong direction. It would be better to stop using module_alloc() > > > in core code except in the module loader, and have a generic > > > text_alloc() that can be overridden by the arch if necessary. Note > > > that x86 and s390 are the only architectures that use module_alloc() > > > in ftrace code. > > > > This series essentially does this: introduces text_alloc() and > > text_memfree(), which have generic implementations in kernel/text.c. > > Those can be overriddent by arch specific implementations. > > > > What you think should be done differently than in my patch set? > > > > On arm64, module_alloc is only used by the module loader, and so > pulling it out and renaming it will cause unused code to be > incorporated into the kernel when building without module support, > which is the use case you claim to be addressing. > > Module_alloc has semantics that are intimately tied to the module > loader, but over the years, it ended up being (ab)used by other > subsystems, which don't require those semantics but just need n pages > of vmalloc space with executable permissions. > > So the correct approach is to make text_alloc() implement just that, > generically, and switch bpf etc to use it. Then, only on architectures > that need it, override it with an implementation that has the required > additional semantics. > > Refactoring 10+ architectures like this without any regard for how > text_alloc() deviates from module_alloc() just creates a lot of churn > that others will have to clean up after you. For 32-bit ARM, our bpf code uses "blx/bx" (or equivalent code sequences) rather than encoding a "bl" or "b", so BPF there doesn't care where the executable memory is mapped, and doesn't need any PLTs. Given that, should bpf always allocate from the vmalloc() region to preserve the module space for modules? I'm more concerned about ftrace though, but only because I don't have the understanding of that subsystem to really say whether there are any side effects from having the allocations potentially be out of range of a "bl" or "b" instruction. If ftrace jumps both to and from the allocated page using a "load address to register, branch to register" approach like BPF does, then ftrace should be safe - and again, raises the issue that maybe it should always come from vmalloc() space. So, I think we need to keep module_alloc() for allocating memory for modules, and provide a new text_alloc() for these other cases. -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!