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 Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id 54531EE49A3 for ; Tue, 22 Aug 2023 12:13:06 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 86A1B28000E; Tue, 22 Aug 2023 08:13:05 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 7F2C990000D; Tue, 22 Aug 2023 08:13:05 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 66CA028000E; Tue, 22 Aug 2023 08:13:05 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0010.hostedemail.com [216.40.44.10]) by kanga.kvack.org (Postfix) with ESMTP id 506B890000D for ; Tue, 22 Aug 2023 08:13:05 -0400 (EDT) Received: from smtpin20.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay04.hostedemail.com (Postfix) with ESMTP id 1D73A1A0303 for ; Tue, 22 Aug 2023 12:13:05 +0000 (UTC) X-FDA: 81151629930.20.FB48672 Received: from szxga08-in.huawei.com (szxga08-in.huawei.com [45.249.212.255]) by imf28.hostedemail.com (Postfix) with ESMTP id 716E8C0002 for ; Tue, 22 Aug 2023 12:13:02 +0000 (UTC) Authentication-Results: imf28.hostedemail.com; dkim=none; dmarc=pass (policy=quarantine) header.from=huawei.com; spf=pass (imf28.hostedemail.com: domain of wangkefeng.wang@huawei.com designates 45.249.212.255 as permitted sender) smtp.mailfrom=wangkefeng.wang@huawei.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1692706383; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=msr7qS2BlK67E29Jwd//HrWpTsm5Dk3CjRrwTu2scSA=; b=xlkgI9y3Ktl+lwUsUl6lEwqg2aKJ4f9w7BvDDJ8apMWNyjEVhBRF5Iy22AO4i7WGzIEvFP 01BXCOyvVm/+zNsg9y4YyOPkIkOcsWZKtM6gQV/kXYZ1TylXstYiTqy4wvK/fLN5jVkZtn NU5TXAZTXi1i2Dis+VNW5VaBdzk1ap8= ARC-Authentication-Results: i=1; imf28.hostedemail.com; dkim=none; dmarc=pass (policy=quarantine) header.from=huawei.com; spf=pass (imf28.hostedemail.com: domain of wangkefeng.wang@huawei.com designates 45.249.212.255 as permitted sender) smtp.mailfrom=wangkefeng.wang@huawei.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1692706383; a=rsa-sha256; cv=none; b=OGxgvoh1GoH4pEzWuXDbLTXhZqaLliL3Ch/4VZEhSvsj6MguWUvHmm4cfiUO4Hx7iLmI81 0srl4HkgvQqX2YPiSVr4s0aqQxeQGrutLqi6RF7jjNjtSPSYcKXkFb7zKZQ7NtIqHTjxqF L8PQeUt+5hzTfM5xrRYNjMBdA1AjJyo= Received: from dggpemm100001.china.huawei.com (unknown [172.30.72.56]) by szxga08-in.huawei.com (SkyGuard) with ESMTP id 4RVSr13fyWz1L9DH; Tue, 22 Aug 2023 20:11:29 +0800 (CST) Received: from [10.174.177.243] (10.174.177.243) by dggpemm100001.china.huawei.com (7.185.36.93) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2507.31; Tue, 22 Aug 2023 20:12:56 +0800 Message-ID: <7fff8202-0be1-4989-959f-8c0b14ca1236@huawei.com> Date: Tue, 22 Aug 2023 20:12:55 +0800 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH rfc v2 05/10] powerpc: mm: use try_vma_locked_page_fault() Content-Language: en-US To: Christophe Leroy , Andrew Morton , "linux-mm@kvack.org" CC: "surenb@google.com" , "willy@infradead.org" , Russell King , Catalin Marinas , Will Deacon , Huacai Chen , WANG Xuerui , Michael Ellerman , Nicholas Piggin , Paul Walmsley , Palmer Dabbelt , Albert Ou , Alexander Gordeev , Gerald Schaefer , Heiko Carstens , Vasily Gorbik , Christian Borntraeger , Sven Schnelle , Dave Hansen , Andy Lutomirski , Peter Zijlstra , Thomas Gleixner , Ingo Molnar , Borislav Petkov , "x86@kernel.org" , "H . Peter Anvin" , "linux-arm-kernel@lists.infradead.org" , "linux-kernel@vger.kernel.org" , "loongarch@lists.linux.dev" , "linuxppc-dev@lists.ozlabs.org" , "linux-riscv@lists.infradead.org" , "linux-s390@vger.kernel.org" References: <20230821123056.2109942-1-wangkefeng.wang@huawei.com> <20230821123056.2109942-6-wangkefeng.wang@huawei.com> <7eeed961-c2c0-2aeb-ff8c-3717de09d605@csgroup.eu> From: Kefeng Wang In-Reply-To: <7eeed961-c2c0-2aeb-ff8c-3717de09d605@csgroup.eu> Content-Type: text/plain; charset="UTF-8"; format=flowed Content-Transfer-Encoding: 8bit X-Originating-IP: [10.174.177.243] X-ClientProxiedBy: dggems703-chm.china.huawei.com (10.3.19.180) To dggpemm100001.china.huawei.com (7.185.36.93) X-CFilter-Loop: Reflected X-Rspamd-Queue-Id: 716E8C0002 X-Rspam-User: X-Rspamd-Server: rspam05 X-Stat-Signature: d775s8t7e1kzqebqdw8ud3qj3xi5jw4f X-HE-Tag: 1692706382-872331 X-HE-Meta: U2FsdGVkX18EZK71zGqpnPY5EwKWmpmKpV7/C/4JvlAPY+pNco8leKfuuOZ3DtR0EFSj1656HrSs1uVYkHHNZDPip0SKMcBOXDV62ZO+mOkFvaZRatKX9LJvHkWKpLZygT0L1aDFYkQk1qD7XU7k4tjeyKaurH4an+X1wgEcn6zrkIzdLSV9Zr5gs9Wqrr9m22lGpzLaaKQQ32PQnspfzw9PTIQ/poHLrW/uZjL3ZsnvxZu0JGjCCrOb5BBZoi/cWU+bhCUeQhEVl3o0UaP/MJrVhuCNWXKoZbDZmgVShKwM4COyHNMJgO20frwnnSZhkintK0fLc/3HTK3/Pdaz1yxLs9sjWm3jxTwdiT82r82bVay4xNvkfWJMDGKBQEGQV/hhXDCD80ftP8tCwaNT3QohuFIOBD4BvKeu/ZXuekhRzsmGfluVvhtJA0nLqg3aR4cdonajY1tumB/d2le0FaiUGh6UjitCvSFgjYGZYu24fC0uuuJUujl6XtsT9Saja4wo1o1b7qcTZ/PqxpttsV8/CF3jmrgJxdoweGrmb+QOj6OpzQHMThL7boykJTkemUjUsUVol8H3a3GKOeMYH9eG3fJysA0VHU6ZV7Qa2OcfHJTjisTmOPf6YHObttvcAcV/zxdKaCWlQbb3lr2ec93IKeEKhbhJEYG1k3x8+qFAskk+u4PVIdy70G1k5UzP5s4R+S9oom/+mZNAwNxPliOgI20oxpVrUaOzFC6WXgKujFTUUaPENxWHFwQYnDCro5cLNe0z8EK+f+gX8BdYhdrivmJ4lOjaRAZZ+1dtgdlN11lIu9bURGzRYllKga+MB7QLfRvd3TUsEtJKGWevopmmiHK6xT/5IK7ZySYhTUacAn3tkWEvBGnXPUCOhpCnyr7cdRQigGCZh1ebWdNKnJjlJeWR60+pvXQ13twLWcl47T9La+8LOZfUuXy0vgpVZPAwsivmKvJFt8Q1KSe eN+YyrlH 7FcHINTXHSXkV+VDwpd8cpwlmxwqsT6ddK6VWeE2MeYXftuDNIc4sK53i+QNGQ0aXyJ2l01cUzvFcLn/ev0smKJh6Ncwbg2HqeiygKgJUWes0D5OVgCDZlgTo6PnOw/Y3PE9zxqltFsZZePRtdddxGtYe2yQXQePHGQLfczyyYgjTrhWLsv0LZHV/1WRpYBqWs9sjVlGVvYrEdbK6VfVtwe1Wj2CPvwcAJbZwybBRAYdu5UW5SiQiZ+8odJw+Ubjlv9EctVvZ/3GvaaYb3qx9hUUmcw== X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: On 2023/8/22 17:38, Christophe Leroy wrote: > > > Le 21/08/2023 à 14:30, Kefeng Wang a écrit : >> Use new try_vma_locked_page_fault() helper to simplify code. >> No functional change intended. > > Does it really simplifies code ? It's 32 insertions versus 34 deletions > so only removing 2 lines. Yes，it is unfriendly for powerpc as the arch's vma access check is much complex than other arch, > > I don't like the struct vm_fault you are adding because when it was four > independant variables it was handled through local registers. Now that > it is a struct it has to go via the stack, leading to unnecessary memory > read and writes. And going back and forth between architecture code and > generic code may also be counter-performant. Because different arch has different var to check vma access, so the easy way to add them into vmf, I don' find a better way. > > Did you make any performance analysis ? Page faults are really a hot > path when dealling with minor faults. no, this is only built and rfc to see the feedback about the conversion. Thanks. > > Thanks > Christophe > >> >> Signed-off-by: Kefeng Wang >> --- >> arch/powerpc/mm/fault.c | 66 ++++++++++++++++++++--------------------- >> 1 file changed, 32 insertions(+), 34 deletions(-) >> >> diff --git a/arch/powerpc/mm/fault.c b/arch/powerpc/mm/fault.c >> index b1723094d464..52f9546e020e 100644 >> --- a/arch/powerpc/mm/fault.c >> +++ b/arch/powerpc/mm/fault.c >> @@ -391,6 +391,22 @@ static int page_fault_is_bad(unsigned long err) >> #define page_fault_is_bad(__err) ((__err) & DSISR_BAD_FAULT_32S) >> #endif >> >> +#ifdef CONFIG_PER_VMA_LOCK >> +bool arch_vma_access_error(struct vm_area_struct *vma, struct vm_fault *vmf) >> +{ >> + int is_exec = TRAP(vmf->regs) == INTERRUPT_INST_STORAGE; >> + int is_write = page_fault_is_write(vmf->fault_code); >> + >> + if (unlikely(access_pkey_error(is_write, is_exec, >> + (vmf->fault_code & DSISR_KEYFAULT), vma))) >> + return true; >> + >> + if (unlikely(access_error(is_write, is_exec, vma))) >> + return true; >> + return false; >> +} >> +#endif >> + >> /* >> * For 600- and 800-family processors, the error_code parameter is DSISR >> * for a data fault, SRR1 for an instruction fault. >> @@ -407,12 +423,18 @@ static int ___do_page_fault(struct pt_regs *regs, unsigned long address, >> { >> struct vm_area_struct * vma; >> struct mm_struct *mm = current->mm; >> - unsigned int flags = FAULT_FLAG_DEFAULT; >> int is_exec = TRAP(regs) == INTERRUPT_INST_STORAGE; >> int is_user = user_mode(regs); >> int is_write = page_fault_is_write(error_code); >> vm_fault_t fault, major = 0; >> bool kprobe_fault = kprobe_page_fault(regs, 11); >> + struct vm_fault vmf = { >> + .real_address = address, >> + .fault_code = error_code, >> + .regs = regs, >> + .flags = FAULT_FLAG_DEFAULT, >> + }; >> + >> >> if (unlikely(debugger_fault_handler(regs) || kprobe_fault)) >> return 0; >> @@ -463,45 +485,21 @@ static int ___do_page_fault(struct pt_regs *regs, unsigned long address, >> * mmap_lock held >> */ >> if (is_user) >> - flags |= FAULT_FLAG_USER; >> + vmf.flags |= FAULT_FLAG_USER; >> if (is_write) >> - flags |= FAULT_FLAG_WRITE; >> + vmf.flags |= FAULT_FLAG_WRITE; >> if (is_exec) >> - flags |= FAULT_FLAG_INSTRUCTION; >> + vmf.flags |= FAULT_FLAG_INSTRUCTION; >> >> - if (!(flags & FAULT_FLAG_USER)) >> - goto lock_mmap; >> - >> - vma = lock_vma_under_rcu(mm, address); >> - if (!vma) >> - goto lock_mmap; >> - >> - if (unlikely(access_pkey_error(is_write, is_exec, >> - (error_code & DSISR_KEYFAULT), vma))) { >> - vma_end_read(vma); >> - goto lock_mmap; >> - } >> - >> - if (unlikely(access_error(is_write, is_exec, vma))) { >> - vma_end_read(vma); >> - goto lock_mmap; >> - } >> - >> - fault = handle_mm_fault(vma, address, flags | FAULT_FLAG_VMA_LOCK, regs); >> - if (!(fault & (VM_FAULT_RETRY | VM_FAULT_COMPLETED))) >> - vma_end_read(vma); >> - >> - if (!(fault & VM_FAULT_RETRY)) { >> - count_vm_vma_lock_event(VMA_LOCK_SUCCESS); >> + fault = try_vma_locked_page_fault(&vmf); >> + if (fault == VM_FAULT_NONE) >> + goto retry; >> + if (!(fault & VM_FAULT_RETRY)) >> goto done; >> - } >> - count_vm_vma_lock_event(VMA_LOCK_RETRY); >> >> if (fault_signal_pending(fault, regs)) >> return user_mode(regs) ? 0 : SIGBUS; >> >> -lock_mmap: >> - >> /* When running in the kernel we expect faults to occur only to >> * addresses in user space. All other faults represent errors in the >> * kernel and should generate an OOPS. Unfortunately, in the case of an >> @@ -528,7 +526,7 @@ static int ___do_page_fault(struct pt_regs *regs, unsigned long address, >> * make sure we exit gracefully rather than endlessly redo >> * the fault. >> */ >> - fault = handle_mm_fault(vma, address, flags, regs); >> + fault = handle_mm_fault(vma, address, vmf.flags, regs); >> >> major |= fault & VM_FAULT_MAJOR; >> >> @@ -544,7 +542,7 @@ static int ___do_page_fault(struct pt_regs *regs, unsigned long address, >> * case. >> */ >> if (unlikely(fault & VM_FAULT_RETRY)) { >> - flags |= FAULT_FLAG_TRIED; >> + vmf.flags |= FAULT_FLAG_TRIED; >> goto retry; >> } >>