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=-6.3 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,NICE_REPLY_A,SPF_HELO_NONE, SPF_PASS,UNPARSEABLE_RELAY,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 3EFCDC433DF for ; Mon, 27 Jul 2020 22:43:23 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id 09ECB20786 for ; Mon, 27 Jul 2020 22:43:22 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 09ECB20786 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=linux.alibaba.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=owner-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix) id 87DC56B0002; Mon, 27 Jul 2020 18:43:22 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 82F2B6B0005; Mon, 27 Jul 2020 18:43:22 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 747A96B0006; Mon, 27 Jul 2020 18:43:22 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0033.hostedemail.com [216.40.44.33]) by kanga.kvack.org (Postfix) with ESMTP id 60CAF6B0002 for ; Mon, 27 Jul 2020 18:43:22 -0400 (EDT) Received: from smtpin19.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay01.hostedemail.com (Postfix) with ESMTP id B0175180AD81F for ; Mon, 27 Jul 2020 22:43:21 +0000 (UTC) X-FDA: 77085333402.19.blade87_3b031b726f64 Received: from filter.hostedemail.com (10.5.16.251.rfc1918.com [10.5.16.251]) by smtpin19.hostedemail.com (Postfix) with ESMTP id 8561D1AD1B1 for ; Mon, 27 Jul 2020 22:43:21 +0000 (UTC) X-HE-Tag: blade87_3b031b726f64 X-Filterd-Recvd-Size: 5285 Received: from out30-130.freemail.mail.aliyun.com (out30-130.freemail.mail.aliyun.com [115.124.30.130]) by imf40.hostedemail.com (Postfix) with ESMTP for ; Mon, 27 Jul 2020 22:43:19 +0000 (UTC) X-Alimail-AntiSpam:AC=PASS;BC=-1|-1;BR=01201311R601e4;CH=green;DM=||false|;DS=||;FP=0|-1|-1|-1|0|-1|-1|-1;HT=e01f04427;MF=yang.shi@linux.alibaba.com;NM=1;PH=DS;RN=14;SR=0;TI=SMTPD_---0U40bDB8_1595889792; Received: from US-143344MP.local(mailfrom:yang.shi@linux.alibaba.com fp:SMTPD_---0U40bDB8_1595889792) by smtp.aliyun-inc.com(127.0.0.1); Tue, 28 Jul 2020 06:43:16 +0800 Subject: Re: [patch 01/15] mm/memory.c: avoid access flag update TLB flush for retried page fault To: Linus Torvalds Cc: linux-arch , Yu Xu , Catalin Marinas , Andrew Morton , Johannes Weiner , Hillf Danton , Hugh Dickins , Josef Bacik , "Kirill A . Shutemov" , Linux-MM , mm-commits@vger.kernel.org, Will Deacon , Matthew Wilcox References: <20200723211432.b31831a0df3bc2cbdae31b40@linux-foundation.org> <20200724041508.QlTbrHnfh%akpm@linux-foundation.org> <0323de82-cfbd-8506-fa9c-a702703dd654@linux.alibaba.com> <20200727110512.GB25400@gaia> <39560818-463f-da3a-fc9e-3a4a0a082f61@linux.alibaba.com> From: Yang Shi Message-ID: <89c6671a-39ba-d1cc-9bac-2e6717916220@linux.alibaba.com> Date: Mon, 27 Jul 2020 15:43:11 -0700 User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.12; rv:52.0) Gecko/20100101 Thunderbird/52.7.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit Content-Language: en-US X-Rspamd-Queue-Id: 8561D1AD1B1 X-Spamd-Result: default: False [0.00 / 100.00] X-Rspamd-Server: rspam04 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 Mon, Jul 27, 2020 at 11:04 AM Yang Shi wrote: >> It looks Linus's patch has better data. It seems sane to me since >> Catalin's patch still needs flush TLB in the shared domain. > Well, my patch as posted never built at all, I think. > > Looking back at that patch, I used FAULT_FLAG_RETRY. But that's not > the correct name for any of the bits. > > So you must have fixed it. Did you make it use "FAULT_FLAG_TRIED"? > Because that's the right bit - don't flush if this is actually the > second (or more) attempt. Actually I didn't have access to that test machine and I didn't try to build your patch, Yu Xu helped me test it. I will double check with him once he is back online. However that data looks sane since my patch (skip pte update) achieved the similar result. > > But I'm a bit worried that you would have used one of the other bits > (FAULT_FLAG_ALLOW_RETRY or FAULT_FLAG_RETRY_NOWAIT), and that would be > wrong. Those get set on the first attempt to say "you _may_ retry", > but they get set on the first one. > > That just shows how much I tested the patch I sent out. It was > whitespace-damaged on purpose, but I still want to check. > > The "FAULT_FLAG_TRIED" bit I believe is reasonable to test. That one > literally says "I've gone through this once already, don't bother with > spurious faults". But I don't think it triggers much in practice. We > seldom actually retry faults, it needs a page that we actually start > IO on (and dropped the mmap lock for) to happen. It wouldn't happen on > the "turn existing page dirty" case, for example. With the commit ("mm: drop mmap_sem before calling balance_dirty_pages() in write fault") the retried fault may happen much more frequently than before since it would drop mmap lock as long as dirty throttling happens. > > The "FAULT_FLAG_WRITE" bit is what we test right now. I think it's > wrong. I think it is a "this happens to work" bit, and cuts down on a > lot of common cases, by simply skipping something that might be needed > but basically never is. > > So I think a lot of this is dodgy. It doesn't matter on x86, and > nobody cared. Because x86 will always re-walk the page tables before > taking an architectural fault (the same way it walks them for > dirty/accessed bit updates - you could think of x86 as doing all the > things everybody else does in software, they just do in the hw walker > micro-fault logic instead). > > A local TLB invalidate of a single virtual address should be basically > free. We're talking single cycles kind of free. The problem here isn't > the flush_tlb_fix_spurious_fault() itself, the problem here is that > arm64 (and pretty much everybody else who uses the default fallback) > does something horribly horribly wrong, and doesn't do the free > version. Yes, I do agree global TLB flush seems overkilling for some architectures. > > Linus