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=-2.5 required=3.0 tests=DKIM_SIGNED,DKIM_VALID, HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS, T_DKIMWL_WL_HIGH,URIBL_BLOCKED,USER_AGENT_MUTT autolearn=ham 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 E0D7AC2BCA1 for ; Fri, 7 Jun 2019 20:12:06 +0000 (UTC) Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (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 B25A0208C0 for ; Fri, 7 Jun 2019 20:12:06 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=lists.infradead.org header.i=@lists.infradead.org header.b="ntLycrrt" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org B25A0208C0 Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=infradead.org Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-arm-kernel-bounces+infradead-linux-arm-kernel=archiver.kernel.org@lists.infradead.org DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20170209; h=Sender: Content-Transfer-Encoding:Content-Type:Cc:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:References: Message-ID:Subject:To:From:Date:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=c0t1dQPOVwEUzjMwm51euD9/6LmQh63+doUp/dJEztg=; b=ntLycrrtqglPxi F7rlcfOAJKDckCyjYLRPJATvDPnjnF7DEUrY8l+lxGzZuMpPSIehRVpXw+H0890LkC8fG6LZB/lAC qeGgeTCH2HnAWygE51gPpiSqtjTFWUT+iVch3nVvwBYGTi5JmST0i+u46lLKMxtwvF1/Cp6+es7/9 ZoOL9dzgsAkoB/NVhbQaCWvNyAOh2+W0p2KSgpKCo5iVFsNdpbj9Et+pDfjmAqxzp9LNP1Lg/0iBx 1+V/HivU3ALikA1LCgDUYUHjGuFQwlAibWBhdYeX4ZxJzbyoC7ivQfHfY+2XNjlwD8jFbfI6becce bcbTZhzzgZ650/x1mmhQ==; Received: from localhost ([127.0.0.1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.92 #3 (Red Hat Linux)) id 1hZLDa-0002mn-94; Fri, 07 Jun 2019 20:12:06 +0000 Received: from willy by bombadil.infradead.org with local (Exim 4.92 #3 (Red Hat Linux)) id 1hZLDX-0002mb-5b; Fri, 07 Jun 2019 20:12:03 +0000 Date: Fri, 7 Jun 2019 13:12:03 -0700 From: Matthew Wilcox To: Anshuman Khandual Subject: Re: [RFC V3] mm: Generalize and rename notify_page_fault() as kprobe_page_fault() Message-ID: <20190607201202.GA32656@bombadil.infradead.org> References: <1559903655-5609-1-git-send-email-anshuman.khandual@arm.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <1559903655-5609-1-git-send-email-anshuman.khandual@arm.com> User-Agent: Mutt/1.9.2 (2017-12-15) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Mark Rutland , Michal Hocko , linux-ia64@vger.kernel.org, linux-sh@vger.kernel.org, Peter Zijlstra , Catalin Marinas , Dave Hansen , Will Deacon , linux-mm@kvack.org, Paul Mackerras , sparclinux@vger.kernel.org, linux-s390@vger.kernel.org, Yoshinori Sato , Michael Ellerman , x86@kernel.org, Russell King , Ingo Molnar , Fenghua Yu , Stephen Rothwell , Andrey Konovalov , Andy Lutomirski , Thomas Gleixner , linux-arm-kernel@lists.infradead.org, Christophe Leroy , Tony Luck , Heiko Carstens , linux-kernel@vger.kernel.org, Martin Schwidefsky , Andrew Morton , linuxppc-dev@lists.ozlabs.org, "David S. Miller" Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+infradead-linux-arm-kernel=archiver.kernel.org@lists.infradead.org Before: > @@ -46,23 +46,6 @@ kmmio_fault(struct pt_regs *regs, unsigned long addr) > return 0; > } > > -static nokprobe_inline int kprobes_fault(struct pt_regs *regs) > -{ > - if (!kprobes_built_in()) > - return 0; > - if (user_mode(regs)) > - return 0; > - /* > - * To be potentially processing a kprobe fault and to be allowed to call > - * kprobe_running(), we have to be non-preemptible. > - */ > - if (preemptible()) > - return 0; > - if (!kprobe_running()) > - return 0; > - return kprobe_fault_handler(regs, X86_TRAP_PF); > -} After: > +++ b/include/linux/kprobes.h > @@ -458,4 +458,20 @@ static inline bool is_kprobe_optinsn_slot(unsigned long addr) > } > #endif > > +static nokprobe_inline bool kprobe_page_fault(struct pt_regs *regs, > + unsigned int trap) > +{ > + int ret = 0; > + > + /* > + * To be potentially processing a kprobe fault and to be allowed > + * to call kprobe_running(), we have to be non-preemptible. > + */ > + if (kprobes_built_in() && !preemptible() && !user_mode(regs)) { > + if (kprobe_running() && kprobe_fault_handler(regs, trap)) > + ret = 1; > + } > + return ret; > +} Do you really think this is easier to read? Why not just move the x86 version to include/linux/kprobes.h, and replace the int with bool? On Fri, Jun 07, 2019 at 04:04:15PM +0530, Anshuman Khandual wrote: > Very similar definitions for notify_page_fault() are being used by multiple > architectures duplicating much of the same code. This attempts to unify all > of them into a generic implementation, rename it as kprobe_page_fault() and > then move it to a common header. I think this description suffers from having been written for v1 of this patch. It describes what you _did_, but it's not what this patch currently _is_. Why not something like: Architectures which support kprobes have very similar boilerplate around calling kprobe_fault_handler(). Use a helper function in kprobes.h to unify them, based on the x86 code. This changes the behaviour for other architectures when preemption is enabled. Previously, they would have disabled preemption while calling the kprobe handler. However, preemption would be disabled if this fault was due to a kprobe, so we know the fault was not due to a kprobe handler and can simply return failure. This behaviour was introduced in commit a980c0ef9f6d ("x86/kprobes: Refactor kprobes_fault() like kprobe_exceptions_notify()") > arch/arm/mm/fault.c | 24 +----------------------- > arch/arm64/mm/fault.c | 24 +----------------------- > arch/ia64/mm/fault.c | 24 +----------------------- > arch/powerpc/mm/fault.c | 23 ++--------------------- > arch/s390/mm/fault.c | 16 +--------------- > arch/sh/mm/fault.c | 18 ++---------------- > arch/sparc/mm/fault_64.c | 16 +--------------- > arch/x86/mm/fault.c | 21 ++------------------- > include/linux/kprobes.h | 16 ++++++++++++++++ What about arc and mips? _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel