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 5E28EC4332F for ; Tue, 15 Nov 2022 16:08:22 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 9F3D38E0001; Tue, 15 Nov 2022 11:08:21 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 9A42C6B0074; Tue, 15 Nov 2022 11:08:21 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 86BD78E0001; Tue, 15 Nov 2022 11:08:21 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0017.hostedemail.com [216.40.44.17]) by kanga.kvack.org (Postfix) with ESMTP id 73FAD6B0073 for ; Tue, 15 Nov 2022 11:08:21 -0500 (EST) Received: from smtpin24.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay07.hostedemail.com (Postfix) with ESMTP id 16EFE16102B for ; Tue, 15 Nov 2022 16:08:21 +0000 (UTC) X-FDA: 80136158802.24.278EC86 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by imf01.hostedemail.com (Postfix) with ESMTP id 9A6194002C for ; Tue, 15 Nov 2022 16:08:17 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1668528496; 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: in-reply-to:in-reply-to:references:references; bh=DqBPgUe4Sm3rQRHQ4BZn8ze3h0OIxC8sEcLBb0IfLqw=; b=U5qQ3fGXXXLImqIQpU5HLvI1WJ0pFdgroAZ/1DEi0ZHmL4hj+PjimUO1Ly3M6vcn2J946t 4R9JpLa0+5WLn9TDDZSsX6v9nZRht3G0sUFcx3v36+ljusW1Cg9G9SVdjRE4j54pwxcKa8 8G90RSEKD2xgMP5yz6dLECFhJCuwngo= Received: from mail-qk1-f198.google.com (mail-qk1-f198.google.com [209.85.222.198]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_128_GCM_SHA256) id us-mta-275-sMA_ggkZPQS2LgZwrIQBBw-1; Tue, 15 Nov 2022 11:08:15 -0500 X-MC-Unique: sMA_ggkZPQS2LgZwrIQBBw-1 Received: by mail-qk1-f198.google.com with SMTP id h8-20020a05620a284800b006b5c98f09fbso14201713qkp.21 for ; Tue, 15 Nov 2022 08:08:15 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=DqBPgUe4Sm3rQRHQ4BZn8ze3h0OIxC8sEcLBb0IfLqw=; b=2IKjRe/h73R01H/Ci5pD8tpvtduHzDI38ThPFWanuXJOMrQ/gOMtoYzkd6Cr8+MiRg wVwy6/n5sRPwWSZGeHMkV4kON7XZKuS1jOlFTSCcctBgCMMheJg1PeE4qDjM7JB1DQDk aP/tkWae4gkfMn5UlPxiPKNgjpPmK1DE/HbmseOkfSi47vbKEthO/yk4GVPM7VjOmldZ JU6foJf6PIvZCyi3mrPx9TK5M02xXud/IoA6GuUangqgpfq2k3g0xiXCPg1hkqDDUFW3 NZ0TxTKD3nYDJZt6QGrSUguapnwjzd2PjWBA93+Sys/9FQiqwhTG+fLQjH4Ygu8Cnx/a ZNdA== X-Gm-Message-State: ANoB5pllxOcLBIsytouf0usWeSlLX4hvo5jSwGvqUfQZTHgbOQ8+axvo IBZHICi5+2Tb3YewsAE9wUDYZzVDGxzcR+rOMwC26WcFgzxyiLE+nHHz1XobqUwjHYGHFDqWEzm 3WJGdWfgbHg0= X-Received: by 2002:ad4:444b:0:b0:4bc:22fe:b07c with SMTP id l11-20020ad4444b000000b004bc22feb07cmr17166164qvt.7.1668528494703; Tue, 15 Nov 2022 08:08:14 -0800 (PST) X-Google-Smtp-Source: AA0mqf70mQJUfQ19xOCkCRFYx0GkmNRSEJPLG9Rt+n1xw5LBbvpYTpsOBchqj+H6ysPG6urM3V1fvw== X-Received: by 2002:ad4:444b:0:b0:4bc:22fe:b07c with SMTP id l11-20020ad4444b000000b004bc22feb07cmr17166134qvt.7.1668528494412; Tue, 15 Nov 2022 08:08:14 -0800 (PST) Received: from x1n (bras-base-aurron9127w-grc-46-70-31-27-79.dsl.bell.ca. [70.31.27.79]) by smtp.gmail.com with ESMTPSA id s11-20020a05620a29cb00b006ec62032d3dsm8580912qkp.30.2022.11.15.08.08.13 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 15 Nov 2022 08:08:13 -0800 (PST) Date: Tue, 15 Nov 2022 11:08:12 -0500 From: Peter Xu To: David Hildenbrand Cc: linux-kernel@vger.kernel.org, linux-mm@kvack.org, Andrea Arcangeli , Axel Rasmussen , Ives van Hoorne , Nadav Amit , Andrew Morton , Mike Rapoport , stable@vger.kernel.org Subject: Re: [PATCH v2 1/2] mm/migrate: Fix read-only page got writable when recover pte Message-ID: References: <20221110203132.1498183-1-peterx@redhat.com> <20221110203132.1498183-2-peterx@redhat.com> <9af36be3-313b-e39c-85bb-bf30011bccb8@redhat.com> MIME-Version: 1.0 In-Reply-To: X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=utf-8 Content-Disposition: inline ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1668528497; a=rsa-sha256; cv=none; b=FBDhFWbYMIofXLCawtpPEkjmjVZ2jkLhN53UcPRezuoth2WQHo8g3rPu4D0mmXDvjvxLa5 mRSiowcdorI++n6VJIFZ3Tk7fVAcgWI0Gg1Bgd+lRN0Ijn6L2pmy0UjiGMmBP9U0w8NVrv uvj2BG0HM03Y8IGX/BAwR5Nm3SHV5zc= ARC-Authentication-Results: i=1; imf01.hostedemail.com; dkim=pass header.d=redhat.com header.s=mimecast20190719 header.b=U5qQ3fGX; dmarc=pass (policy=none) header.from=redhat.com; spf=pass (imf01.hostedemail.com: domain of peterx@redhat.com designates 170.10.129.124 as permitted sender) smtp.mailfrom=peterx@redhat.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1668528497; 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: in-reply-to:in-reply-to:references:references:dkim-signature; bh=DqBPgUe4Sm3rQRHQ4BZn8ze3h0OIxC8sEcLBb0IfLqw=; b=MEqZ/fpn/l2jGAuRG6MjqILuCec10KoNaCstGsIQx2rmYCR+HSkUcksqOyDP1NHN8eO2YC sxvpaFvskkO5Ulq7b5NnrzINhkoIXLcz4+vhX5X3nPSu78t1CgrouI5CtdCbBgnBbG/Dfg Q1CTuq6/rec661nDGeChaoBcJgYjUuY= Authentication-Results: imf01.hostedemail.com; dkim=pass header.d=redhat.com header.s=mimecast20190719 header.b=U5qQ3fGX; dmarc=pass (policy=none) header.from=redhat.com; spf=pass (imf01.hostedemail.com: domain of peterx@redhat.com designates 170.10.129.124 as permitted sender) smtp.mailfrom=peterx@redhat.com X-Rspam-User: X-Stat-Signature: qhyptpp6t9bwgnf4g77fpseyrz1jz4hp X-Rspamd-Queue-Id: 9A6194002C X-Rspamd-Server: rspam11 X-HE-Tag: 1668528497-989238 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 Tue, Nov 15, 2022 at 10:13:24AM +0100, David Hildenbrand wrote: > > > > > > Any particular reason why not to simply glue this to pte_swp_uffd_wp(), > > > because only that needs special care: > > > > > > if (pte_swp_uffd_wp(*pvmw.pte)) { > > > pte = pte_wrprotect(pte); > > > pte = pte_mkuffd_wp(pte); > > > } > > > > > > > > > And that would match what actually should have been done in commit > > > f45ec5ff16a7 -- only special-case uffd-wp. > > > > > > Note that I think there are cases where we have a PTE that was !writable, > > > but after migration we can map it writable. > > > > The thing is recovering the pte into its original form is the safest > > approach to me, so I think we need justification on why it's always safe to > > set the write bit. > > > > Or do you perhaps have solid clue and think it's always safe > The problem I am having with this broader change, is that this changes > something independent of your original patch/problem. > > If we identify this to be an actual problem, it should most probably be > separate fix + backport. > > > My understanding is that vma->vm_page_prot always tells you what the default > PTE protection in a mapping is. > > If the mapping is private, it is never writable (due to COW). Similarly, if > the shared file mapping needs writenotify, it is never writable. Right. > > > I consider UFFD-wp a special case: while the default VMA protection might > state that it is writable, you actually want individual PTEs to be > write-protected and have to manually remove the protection. > > softdirty tracking is another special case: however, softdirty tracking is > enabled for the whole VMA. For remove_migration_pte() that should be fine (I > guess) because writenotify is active when the VMA needs to track softdirty > bits, and consequently vma->vm_page_prot has the proper default permissions. > > > I wonder if the following (valid), for example is possible: > > > 1) clear_refs() clears VM_SOFTDIRTY and pte_wrprotect() the pte. > -> writenotify is active and vma->vm_page_prot updated accordingly > > VM_SOFTDIRTY is reset due to VMA merging and vma->vm_page_prot is updated > accordingly. See mmap_region() where we set VM_SOFTDIRTY. > > If you now migrate the (still write-protected in the PTE) page, it was not > writable, but it can be writable on the destination. I didn't even notice merging could work with soft-dirty enabled, that's interesting to know. Yes I think it's possible and I agree it's safe, as VM_SOFTDIRTY is set for the merged vma so afaiu the write bit is safe to set. We get a bunch of false positives but that's how soft-dirty works. I think the whole problem is easier if we see this at a higher level. You're discussing this from vma pov and it's fair to do so, at least I agree with what you mentioned so far and I can't see anything outside uffd-wp that can be affected. However, it is also true when you noticed we already have quite a few paragraphs trying to discuss the safety for this and that, that's the part where I think we need justification and it's not that "natural". For "natural", I meant fundamentally we're talking about page migrations here. The natural way (at least to me) for page migration to happen as a fundamental rule is that, we leverag the migration pte to make sure the pte being stable so we can do the migration work, then we "recover" the pte to present either by a full recovery or just (hopefully) only replace the pfn, keeping all the rest untouched. One thing to prove that is we have two migration entries not one (I'm temporarily put the exclusive read one aside since that's solving different problems): MIGRATION_READ and MIGRATION_WRITE. If we only rely on vma flags logically we don't need MIGRATION_READ and MIGRATION_WRITE, we only need MIGRATION generic swap pte then we recover the write bit from vma flags and other things (like uffd-wp, currently we have the bit set in swap pte besides the swap entry type). So maybe one day we can use two migration types rather than three (MIGRATION and MIGRATION_EXCLUSIVE)? I can't tell, but hopefully that shows what I meant, that we need further justification to grant write bit only base on vma, rather than recovering write bit based on migration entry type. > > > > > > > > > BTW, does unuse_pte() need similar care? > > > > > > new_pte = pte_mkold(mk_pte(page, vma->vm_page_prot)); > > > if (pte_swp_uffd_wp(*pte)) > > > new_pte = pte_mkuffd_wp(new_pte); > > > set_pte_at(vma->vm_mm, addr, pte, new_pte); > > > > I think unuse path is fine because unuse only applies to private mappings, > > so we should always have the W bit removed there within mk_pte(). > > You're right, however, shmem swapping confuses me. Maybe that does not apply > here. Yeah these are confusing. Actually I think the unuse path should apply to private mappings on shmem when the page was CoWed already and then got swapped out, but again as long as it's private mapping I think we don't have write bit anyway in retval of mk_pte() even if it's shmem. -- Peter Xu