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 30263C433EF for ; Tue, 14 Jun 2022 07:57:46 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id A4D598D0228; Tue, 14 Jun 2022 03:57:45 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 9FD356B00EE; Tue, 14 Jun 2022 03:57:45 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 89E718D0228; Tue, 14 Jun 2022 03:57:45 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0014.hostedemail.com [216.40.44.14]) by kanga.kvack.org (Postfix) with ESMTP id 7ACE76B00ED for ; Tue, 14 Jun 2022 03:57:45 -0400 (EDT) Received: from smtpin18.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay09.hostedemail.com (Postfix) with ESMTP id 92A7B35AD4 for ; Tue, 14 Jun 2022 07:57:44 +0000 (UTC) X-FDA: 79576087248.18.8BEB1BB Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by imf15.hostedemail.com (Postfix) with ESMTP id 55D0FA00F8 for ; Tue, 14 Jun 2022 07:51:04 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1655193064; h=from:from: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=P5vldjIUqidCO6vlFIaqoxzvdgolTmKJY9z8eS7S7dk=; b=MGs5+Li+k3tVweU88CMbMGqhPgpGPbXqo3dq+Psp6BUIOTO5U6brTp/QO1QZytEG3mW4GN 4n4eTZve+BXFtsoSUGBiwGu99xBnnYOlNMXW2hW1Y21MjLnzkW5hQPkk2QO6hHp5PCDHWV Pfn+GgygEsWgovBZCDZ5LQWpMJQRbR0= Received: from mail-wm1-f69.google.com (mail-wm1-f69.google.com [209.85.128.69]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-5-M2SyCHAiM9KnX8JMK6JeLw-1; Tue, 14 Jun 2022 03:51:01 -0400 X-MC-Unique: M2SyCHAiM9KnX8JMK6JeLw-1 Received: by mail-wm1-f69.google.com with SMTP id ay28-20020a05600c1e1c00b0039c5cbe76c1so7203748wmb.1 for ; Tue, 14 Jun 2022 00:51:00 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:message-id:date:mime-version:user-agent :content-language:to:cc:references:from:organization:subject :in-reply-to:content-transfer-encoding; bh=P5vldjIUqidCO6vlFIaqoxzvdgolTmKJY9z8eS7S7dk=; b=3V+9ySOIN8TzRS+Rj/C5X7lSAaXxVjdoBh3H74vj/v1IbVV4QjdiN7kj5BhlsPqe5p fau0bSGgqPii6ZueXhFG5Ce9woWZgt1830WjwoVxi5ri6F+DDlEGvcGUqVdNTNwVIkXZ h/QIebE/HsSqVDuCkEkMI7kKbs9Cs9yKGqp9i0mZCjW0Fzn67/xEyYMfcz7OJ7LTDCkG KiovDfvOAemoOCuCGi167HXeHmdnyp4POKmHP61Riq03hHIfqRgiF+d6soHu1JVHKYNY Gfl9m2SMRcsROM6qqfNi9Kj8ACZJThU+KJwZ7sbYVBl45W71TutOcUbqDVCslPhqGV5K Ui3w== X-Gm-Message-State: AJIora+ttYnqBRcioFo52P1gb33o06K+ZipIHaFeYjIuWBTDH9L1Giap hUK21xLXk/tdbrc4DBksD0vqvsW2xcAMiHkcmMI7PeMXM66qqqBvi78si3pMasYO9EDChgZYmPD V3n9FE/jChMU= X-Received: by 2002:a5d:4650:0:b0:213:ba65:73fd with SMTP id j16-20020a5d4650000000b00213ba6573fdmr3467658wrs.61.1655193059914; Tue, 14 Jun 2022 00:50:59 -0700 (PDT) X-Google-Smtp-Source: AGRyM1uuuaA6MjIUnj4gRWDfzmfKbYDTX3HpjKjHiI2FtFYEffsdxTUBszXdyoJ0Ax/8lNLp8uGq/g== X-Received: by 2002:a5d:4650:0:b0:213:ba65:73fd with SMTP id j16-20020a5d4650000000b00213ba6573fdmr3467614wrs.61.1655193059541; Tue, 14 Jun 2022 00:50:59 -0700 (PDT) Received: from ?IPV6:2003:cb:c70b:cf00:aace:de16:d459:d411? (p200300cbc70bcf00aacede16d459d411.dip0.t-ipconnect.de. [2003:cb:c70b:cf00:aace:de16:d459:d411]) by smtp.gmail.com with ESMTPSA id j18-20020a05600c191200b003974a00697esm20199504wmq.38.2022.06.14.00.50.58 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Tue, 14 Jun 2022 00:50:58 -0700 (PDT) Message-ID: <85ec2a50-e094-cc39-c42b-b36ce0f53010@redhat.com> Date: Tue, 14 Jun 2022 09:50:58 +0200 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.9.0 To: Peter Xu Cc: Nadav Amit , LKML , linux-mm , Linus Torvalds , Andrew Morton , Dave Hansen , Andrea Arcangeli , Yang Shi , Hugh Dickins , Mel Gorman , Peter Collingbourne References: <20220610181436.84713-1-david@redhat.com> <5DFB7262-6E32-4984-A346-B7DE5040B12F@gmail.com> <9b38302c-ed93-8825-f543-6ce8878748f9@redhat.com> From: David Hildenbrand Organization: Red Hat Subject: Re: [PATCH v2] mm/mprotect: try avoiding write faults for exclusive anonymous pages when changing protection In-Reply-To: X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Language: en-US Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1655193065; a=rsa-sha256; cv=none; b=Z7fCZHMMza+KB0WtqcE5OUKKAywAkiEftgLORmCAAwEd7Odg1jmTQdTmdIUiFF4c9kHsAC Z1sXu1Klh89s5D8b/CxUORON1chL1FID6PIlb6zIIjI/gtlBeIoV4imgipcPbxlyXTNYNi pWsauxhdWHkdlOgYiL4jMU0XJxrMZzs= ARC-Authentication-Results: i=1; imf15.hostedemail.com; dkim=pass header.d=redhat.com header.s=mimecast20190719 header.b=MGs5+Li+; dmarc=pass (policy=none) header.from=redhat.com; spf=none (imf15.hostedemail.com: domain of david@redhat.com has no SPF policy when checking 170.10.129.124) smtp.mailfrom=david@redhat.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1655193065; 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:dkim-signature; bh=P5vldjIUqidCO6vlFIaqoxzvdgolTmKJY9z8eS7S7dk=; b=rB0eQCmZIFbHT74fxbQifvJnMZFdjWkMAVeqk45a5EPiukBzwg9KwBscief1MgoRw/epvI PN5YvcjqCwGBq3LCF2RQTmxp9y2ADe7CYV+98h+FYA3TrHsX1RYV6ES3T0ENjrttVh+CIt kOS5Sraw6mISdZOfuejPb9wqheLvKaA= X-Rspamd-Queue-Id: 55D0FA00F8 X-Rspam-User: Authentication-Results: imf15.hostedemail.com; dkim=pass header.d=redhat.com header.s=mimecast20190719 header.b=MGs5+Li+; dmarc=pass (policy=none) header.from=redhat.com; spf=none (imf15.hostedemail.com: domain of david@redhat.com has no SPF policy when checking 170.10.129.124) smtp.mailfrom=david@redhat.com X-Rspamd-Server: rspam06 X-Stat-Signature: 9tpdd5aba6tiwypd6b6xzdx98i5rqcqo X-HE-Tag: 1655193064-155004 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: >> Not really, but I assume performance gain will be minimal and might not >> be worth the trouble. >> >> I'm fairly busy (and not aware of Andreas version), so I can look at >> this, but it will be part of a separate patch because it will go on my >> TODO list. Not mad if someone beats me to it ;) > > Just for the reference: > > https://github.com/aagit/aa/commit/34cd0d78db407af06d35a06b24be8e92593964be Thanks for that reference! > >> >>> >>>>> >>>>> Results of a simple microbenchmark on my Ryzen 9 3900X, comparing the new >>>>> optimization (avoiding write faults) during mprotect() with softdirty >>>>> tracking, where we require a write fault. >>> >>> Are we comparing the mprotect() sequence operations against softdirty >>> clearing operation? Would it make more sense if we compare the same >>> mprotect() sequence to kernels that are before/after this patch applied? >> >> For simplicity I compared on the same kernel, one time exploting the >> optimization and one time disabling the optimization via softdirty. >> >> I can also simply measure without+with. Extra work for me to combine >> outputs :P > > Well, still that's normally how we work on these, don't we? :) > > Still note that the SOFTDIRTY check (I think) was still reverted.. I meant > I kept thinking below check "vma->vm_flags & VM_SOFTDIRTY" should be > "!(vma->vm_flags & VM_SOFTDIRTY)", but again that's separate change so feel > free to ignore as we've discussed, but please make sure even if you want to > compare with softdirty that's taking into account. I wrapped my head around that softdirty check *a lot* and ended up figuring out that it unintuitively is correct. But maybe I ended up confusing myself. Anyhow, this patch merely moves that check. > >> >>> >>>>> >>>>> Running 1000 iterations each >>>>> >>>>> ========================================================== >>>>> Measuring memset() of 4096 bytes >>>>> First write access: >>>>> Min: 169 ns, Max: 8997 ns, Avg: 830 ns >>>>> Second write access: >>>>> Min: 80 ns, Max: 251 ns, Avg: 168 ns >>>>> Write access after mprotect(PROT_READ)+mprotect(PROT_READ|PROT_WRITE): >>>>> Min: 180 ns, Max: 290 ns, Avg: 190 ns >>>>> Write access after clearing softdirty: >>>>> Min: 451 ns, Max: 1774 ns, Avg: 470 ns >>>>> -> mprotect = 1.131 * second [avg] >>>>> -> mprotect = 0.404 * softdirty [avg] >>> >>> (I don't understand these two lines.. but maybe I'm the only one?) >> >> Most probably not. >> >> "mprotect(PROT_READ)+mprotect(PROT_READ|PROT_WRITE)" needs 113,1% the >> runtime compared with the "second write" access. >> >> "mprotect(PROT_READ)+mprotect(PROT_READ|PROT_WRITE)" needs 40% of the >> runtime compared with disabling the optimization via softdirty tracking. >> >> I may find time to clean that up a bit more to make it easier to consume >> for humans. > > I see, thanks. Appending the explanation after the test result will also > work for me. > > I'm curious is that 113.1% came from tlb miss? If that's the case, I'd > suggest drop those comparisons if there's a new version, since they're > probably not helping to explain what this patch is changing (avoid page > faluts), and IMHO it can slightly confuse reviewers, if you agree. As we seem to have easier benchmarks from Andrea and Peter, I can just reuse these and make my life easier :) [...] >>>> Looks good in general. Just wondering (out loud) whether it makes more sense >>>> to do all the vm_flags and cp_flags related checks in one of the callers >>>> (mprotect_fixup()?) and propagate whether to try to write-unprotect in >>>> cp_flags (e.g., by introducing new MM_CP_TRY_WRITE_UNPROTECT). >>> >>> I can see why David put it like that, because most of the checks are on >>> ptes not vm_flags. >>> >>> But I also agree on this point, especially if to put it in another way: >>> IMHO it'll be confusing if we keey MM_CP_DIRTY_ACCT==false for all private >>> pages even if we're going to take them into account and do smart unprotect >>> operations. >>> >>> So I'm wondering whether we could still at least move vm_flags check into >>> the mprotect_fixup() as suggested by Nadav, perhaps something like: >>> >>> diff --git a/mm/mprotect.c b/mm/mprotect.c >>> index ba5592655ee3..aefd5fe982af 100644 >>> --- a/mm/mprotect.c >>> +++ b/mm/mprotect.c >>> @@ -583,7 +583,11 @@ mprotect_fixup(struct mmu_gather *tlb, struct vm_area_struct *vma, >>> * held in write mode. >>> */ >>> vma->vm_flags = newflags; >>> - dirty_accountable = vma_wants_writenotify(vma, vma->vm_page_prot); >>> + if (vma->vm_flags & VM_SHARED) >>> + dirty_accountable = vma_wants_writenotify(vma, vma->vm_page_prot); >>> + else >>> + /* For private mappings, only if it's writable */ >>> + dirty_accountable = vma->vm_flags & VM_WRITE; >>> vma_set_page_prot(vma); >>> >>> change_protection(tlb, vma, start, end, vma->vm_page_prot, >>> >>> Then IIUC we could drop both the VM_WRITE check in change_pte_range(), and >>> also the VM_SHARED check above in can_change_pte_writable(). Not sure >>> whether that'll look slightly cleaner. >> >> I'll give it a shot and most probably rename dirty_accountable to >> something more expressive -- like Nadav proposed, for example. > > Sure. > >> >>> >>> I'm also copying Peter Collingbourne because afaict he >>> proposed the initial idea (maybe worth some credit in the commit message?), >> >> Do you have a link to that conversation? Either my memory is messing >> with me or I did this without reading that mail (which I think, because >> it simply made sense with PageAnonExclusive at hand). Still, I can add a >> reference to that mail and mention that this was suggested earlier by >> Peter C.. > > I see, no worry then I thought it was coming from that. In this case I'm > not sure whether it's still needed. > > PeterC's v1 was here: > > https://lore.kernel.org/linux-mm/20201212053152.3783250-1-pcc@google.com/ > > But there're a bunch of versions: > > https://lore.kernel.org/linux-mm/?q=mm%3A+improve+mprotect%28R%7CW%29+efficiency+on+pages+referenced+once No idea why I missed that completely as I tend to read a lot linux-mm. Thanks for the pointers! -- Thanks, David / dhildenb