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 5CD29C43334 for ; Wed, 8 Jun 2022 00:43:29 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id DF3756B0072; Tue, 7 Jun 2022 20:43:28 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id DA2196B0073; Tue, 7 Jun 2022 20:43:28 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id C42816B0074; Tue, 7 Jun 2022 20:43:28 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0012.hostedemail.com [216.40.44.12]) by kanga.kvack.org (Postfix) with ESMTP id AEAAE6B0072 for ; Tue, 7 Jun 2022 20:43:28 -0400 (EDT) Received: from smtpin08.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay09.hostedemail.com (Postfix) with ESMTP id 861DE3561B for ; Wed, 8 Jun 2022 00:43:28 +0000 (UTC) X-FDA: 79553220096.08.15535F5 Received: from mail-ed1-f46.google.com (mail-ed1-f46.google.com [209.85.208.46]) by imf26.hostedemail.com (Postfix) with ESMTP id 2F8BF14002C for ; Wed, 8 Jun 2022 00:43:28 +0000 (UTC) Received: by mail-ed1-f46.google.com with SMTP id c2so25014853edf.5 for ; Tue, 07 Jun 2022 17:43:27 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=t6G6v+U3PQZ98KAwPWp+vie7a7TsZCRfiTc7/2kHm14=; b=JNOYJH0NVg3VXaNwMMfLmSDeajuwm0jjsfsCYzz4zLbDRxZCEEO1ODq6Jv7ux/nqWG PAPM7n0lgrhyAtZ6CoqINifT9vRWp5sFFu7c2RlFxCi42xPOhVcjwt7w0znkmnIxATAl FfbKo5Rr/9zxO+Wsl0NM+JEBHJr2IZfrdyCTfqFpFb/cD03c+vXqDF+9NWOOZoldsR5w WJgvKzoPuA7i/Zwl/XjjN/8k2tQZeEbVmMDYgqBEEpOsZ9g4rAqeM7boBOkiplc09dgx tQdWtQkCfwk7cien0Ydf00bCqBxM8CpRWeaPC3/bc2NvodDtdPQ0PtvH+oaxaYLeKlu9 bBKA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=t6G6v+U3PQZ98KAwPWp+vie7a7TsZCRfiTc7/2kHm14=; b=Zx81Qu9GtwgK31fFjL8fSPMkH2jqEn8GDWQsNu67gtX+Wz9LPwvRLpxziEUVS50nrN B75DpuDuEgYqdYVw7Bfr/mC8eA3kxxgoVRir6uAmQQAoJrAmw0HS8D7yukj2jENmixa9 nZFToJ7SN8gRe4u0p7DoN+fOLCxJO5n9owBLVlq8opppDXeEDVKMqW49AFPz69l6DwpK kJWpn+IY5V06TyJg8UuZRzF0DqGOSTWludNuGHIaXpazAoXWkdznFpz/sckDxUwW9X9q E70DhuvCyMFPuEnZcGe+oDYHJvsdfD6BZWbhF+8R53bpP8R5NCgOrnzE4nZQykLCnIQU 23Gw== X-Gm-Message-State: AOAM5315cUu9w6gAoS2ODfyZXIpAUqOz4YiNxjA0xZNwP5p5Kry25VAQ /n5o3eTRVJMxaW3rFfI/4VcYtbc1yfPmj/nhO1w= X-Google-Smtp-Source: ABdhPJz+sYFrvX+1urqNbffSy0LGIhXdDvmkdAbaPCcvl4WzDlqp96l7zhactnoOnBL8HTcTkcmupOQHUjCUKLCeO2g= X-Received: by 2002:a05:6402:510d:b0:42d:e266:e02f with SMTP id m13-20020a056402510d00b0042de266e02fmr36223915edd.277.1654649006703; Tue, 07 Jun 2022 17:43:26 -0700 (PDT) MIME-Version: 1.0 References: <20220518014632.922072-1-yuzhao@google.com> <20220518014632.922072-8-yuzhao@google.com> <20220607102135.GA32448@willie-the-truck> <20220607104358.GA32583@willie-the-truck> In-Reply-To: From: Barry Song <21cnbao@gmail.com> Date: Wed, 8 Jun 2022 12:43:15 +1200 Message-ID: Subject: Re: [PATCH v11 07/14] mm: multi-gen LRU: exploit locality in rmap To: Yu Zhao Cc: Will Deacon , Andrew Morton , Linux-MM , Andi Kleen , Aneesh Kumar , Catalin Marinas , Dave Hansen , Hillf Danton , Jens Axboe , Johannes Weiner , Jonathan Corbet , Linus Torvalds , Matthew Wilcox , Mel Gorman , Michael Larabel , Michal Hocko , Mike Rapoport , Peter Zijlstra , Tejun Heo , Vlastimil Babka , LAK , Linux Doc Mailing List , LKML , x86 , Kernel Page Reclaim v2 , Brian Geffon , Jan Alexander Steffens , Oleksandr Natalenko , Steven Barrett , Suleiman Souhlal , Daniel Byrne , Donald Carr , =?UTF-8?Q?Holger_Hoffst=C3=A4tte?= , Konstantin Kharlamov , Shuang Zhai , Sofia Trinh , Vaibhav Jain , huzhanyuan@oppo.com Content-Type: text/plain; charset="UTF-8" X-Rspamd-Server: rspam07 X-Rspamd-Queue-Id: 2F8BF14002C X-Stat-Signature: 9kftiru1ioxrkbxjj4bbdprd9pbpzfjg X-Rspam-User: Authentication-Results: imf26.hostedemail.com; dkim=pass header.d=gmail.com header.s=20210112 header.b=JNOYJH0N; spf=pass (imf26.hostedemail.com: domain of 21cnbao@gmail.com designates 209.85.208.46 as permitted sender) smtp.mailfrom=21cnbao@gmail.com; dmarc=pass (policy=none) header.from=gmail.com X-HE-Tag: 1654649008-932245 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 Wed, Jun 8, 2022 at 9:07 AM Yu Zhao wrote: > > On Tue, Jun 7, 2022 at 4:44 AM Will Deacon wrote: > > > > On Tue, Jun 07, 2022 at 10:37:46AM +1200, Barry Song wrote: > > > On Tue, Jun 7, 2022 at 10:21 PM Will Deacon wrote: > > > > On Tue, Jun 07, 2022 at 07:37:10PM +1200, Barry Song wrote: > > > > > I can't really explain why we are getting a random app/java vm crash in monkey > > > > > test by using ptep_test_and_clear_young() only in lru_gen_look_around() on an > > > > > armv8-a machine without hardware PTE young support. > > > > > > > > > > Moving to ptep_clear_flush_young() in look_around can make the random > > > > > hang disappear according to zhanyuan(Cc-ed). > > > > > > > > > > On x86, ptep_clear_flush_young() is exactly ptep_test_and_clear_young() > > > > > after > > > > > 'commit b13b1d2d8692 ("x86/mm: In the PTE swapout page reclaim case clear > > > > > the accessed bit instead of flushing the TLB")' > > > > > > > > > > But on arm64, they are different. according to Will's comments in this > > > > > thread which > > > > > tried to make arm64 same with x86, > > > > > https://www.mail-archive.com/linux-kernel@vger.kernel.org/msg1793881.html > > > > > > > > > > " > > > > > This is blindly copied from x86 and isn't true for us: we don't invalidate > > > > > the TLB on context switch. That means our window for keeping the stale > > > > > entries around is potentially much bigger and might not be a great idea. > > > > > > > > > > If we roll a TLB invalidation routine without the trailing DSB, what sort of > > > > > performance does that get you? > > > > > " > > > > > We shouldn't think ptep_clear_flush_young() is safe enough in LRU to > > > > > clear PTE young? Any comments from Will? > > > > > > > > Given that this issue is specific to the multi-gen LRU work, I think Yu is > > > > the best person to comment. However, looking quickly at your analysis above, > > > > I wonder if the code is relying on this sequence: > > > > > > > > > > > > ptep_test_and_clear_young(vma, address, ptep); > > > > ptep_clear_flush_young(vma, address, ptep); > > > > > > > > > > > > to invalidate the TLB. On arm64, that won't be the case, as the invalidation > > > > in ptep_clear_flush_young() is predicated on the pte being young (and this > > > > patches the generic implementation in mm/pgtable-generic.c. In fact, that > > > > second function call is always going to be a no-op unless the pte became > > > > young again in the middle. > > > > > > thanks for your reply, sorry for failing to let you understand my question. > > > my question is actually as below, > > > right now lru_gen_look_around() is using ptep_test_and_clear_young() > > > only without flush to clear pte for a couple of pages including the specific > > > address: > > > void lru_gen_look_around(struct page_vma_mapped_walk *pvmw) > > > { > > > ... > > > > > > for (i = 0, addr = start; addr != end; i++, addr += PAGE_SIZE) { > > > ... > > > > > > if (!ptep_test_and_clear_young(pvmw->vma, addr, pte + i)) > > > continue; > > > > > > ... > > > } > > > > > > I wonder if it is safe to arm64. Do we need to move to ptep_clear_flush_young() > > > in the loop? > > > > I don't know what this code is doing, so Yu is the best person to answer > > that. There's nothing inherently dangerous about eliding the TLB > > maintenance; it really depends on the guarantees needed by the caller. > > Ack. > > > However, the snippet you posted from folio_referenced_one(): > > > > | if (pvmw.pte) { > > | + if (lru_gen_enabled() && pte_young(*pvmw.pte) && > > | + !(vma->vm_flags & (VM_SEQ_READ | VM_RAND_READ))) { > > | + lru_gen_look_around(&pvmw); > > | + referenced++; > > | + } > > | + > > | if (ptep_clear_flush_young_notify(vma, address, > > > > > > Does seem to call lru_gen_look_around() *and* > > ptep_clear_flush_young_notify(), which is what prompted my question as it > > looks pretty suspicious to me. > > The _notify varint reaches into the MMU notifier -- > lru_gen_look_around() doesn't do that because GPA space generally has > no locality. I hope this explains why both. > > As to why the code is organized this way -- it depends on the point of > view. Mine is that lru_gen_look_around() is an add-on, since its logic > is independent/separable from ptep_clear_flush_young_notify(). We can > make lru_gen_look_around() include ptep_clear_flush_young_notify(), > but that would make the code functionally interwinted, which is bad > for my taste. Given we used to have a flush for clear pte young in LRU, right now we are moving to nop in almost all cases for the flush unless the address becomes young exactly after look_around and before ptep_clear_flush_young_notify. It means we are actually dropping flush. So the question is, were we overcautious? we actually don't need the flush at all even without mglru? for arm64, without the flush, stale data might be used for a relatively long time as commented in [1], does it actually harm? [1]https://www.mail-archive.com/linux-kernel@vger.kernel.org/msg1793881.html Thanks Barry