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 339C5ECAAA1 for ; Fri, 28 Oct 2022 15:58:07 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id AFBB16B0073; Fri, 28 Oct 2022 11:58:06 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id AAB038E0002; Fri, 28 Oct 2022 11:58:06 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 99A558E0001; Fri, 28 Oct 2022 11:58:06 -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 8B7C46B0073 for ; Fri, 28 Oct 2022 11:58:06 -0400 (EDT) Received: from smtpin24.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay10.hostedemail.com (Postfix) with ESMTP id 34D7EC12A8 for ; Fri, 28 Oct 2022 15:58:06 +0000 (UTC) X-FDA: 80070814572.24.3F5B99E Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by imf24.hostedemail.com (Postfix) with ESMTP id B5728180009 for ; Fri, 28 Oct 2022 15:58:04 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1666972684; 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=4h7qjUXjTtRvctXOxGac74UvVU5N5blmNjcxZ+F+ZAE=; b=CqcSrMD2svsm1W/7rAbeIOsOamlqZkTWJTYj0KxKiap5Y30Fs31yKN7eqJ4mVWfFs2Ql2b EWnzEFj7Gedhbjrf8rWanVwBm1uRrvuoVupPHYkPVbK8O5lDBLnb9r0eycV7DT0OeFthsS lH38ATNCYX73KwhICvK58NsmsMoHEjw= Received: from mail-qv1-f72.google.com (mail-qv1-f72.google.com [209.85.219.72]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_128_GCM_SHA256) id us-mta-250-dgJiR8WUPSK_ZGosh2jYow-1; Fri, 28 Oct 2022 11:58:02 -0400 X-MC-Unique: dgJiR8WUPSK_ZGosh2jYow-1 Received: by mail-qv1-f72.google.com with SMTP id e13-20020ad450cd000000b004bb49d98da4so3140841qvq.9 for ; Fri, 28 Oct 2022 08:58:02 -0700 (PDT) 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=JZ6yJrsNWaZL1mSi1+tvHOq5SMXsifQHw4WS6aiyzU0=; b=k9CxmZW8ca4d6JeffKra3HiyTgnpF+DdP/cWnEyeSTC9S38svXWarR63S6wAyIjyMi u9h4wcv1iJRytH+I8sQk6bI8BQ5CSUQnakcl+uuGKKOQV5VqyWGCLQ+9a2nU0nMcL2fh 6rysYthfrBaBP6k//1hGZh5cIdGOmka2rcafktTh6T9+l46I6dgvWygshttYNfB4HC49 qyf7aGBeHbmHMjZncWJ93l7K8IcyNyXHI+XjB08Ak/TGHsdJUdypIo1RPne07uK0jZPY rRNIj4uBhO0H45t3eDS81pKtYxgj35fv06PTrFTvkGTtx2pezUbTfVgxKOnTAO3keCxi iUDg== X-Gm-Message-State: ACrzQf2HvmB4dx+VrJsvBOOcKKN8pqPOEbYbFnAhTy53ySYB6Y4SybR+ lAhsVL2IpQySKCy4GfWiGGtrIxStusBGqcuh4Q1cEVkOFwLwrpKHhaxcjreoxCfnCO7j2TWIFCs eUntWPUd2fWc= X-Received: by 2002:a05:620a:298a:b0:6ee:e31f:6247 with SMTP id r10-20020a05620a298a00b006eee31f6247mr37888889qkp.744.1666972681959; Fri, 28 Oct 2022 08:58:01 -0700 (PDT) X-Google-Smtp-Source: AMsMyM5zikpCggckW3kZmG1QOaNP6E0NjoUZTCfDs/eJV24ipKqDVWXHGGzTxuDAUamvi+z0+bbMXQ== X-Received: by 2002:a05:620a:298a:b0:6ee:e31f:6247 with SMTP id r10-20020a05620a298a00b006eee31f6247mr37888868qkp.744.1666972681621; Fri, 28 Oct 2022 08:58:01 -0700 (PDT) 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 e12-20020a37ac0c000000b006e9b3096482sm3096362qkm.64.2022.10.28.08.58.00 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 28 Oct 2022 08:58:00 -0700 (PDT) Date: Fri, 28 Oct 2022 11:57:59 -0400 From: Peter Xu To: Mike Kravetz Cc: linux-mm@kvack.org, linux-kernel@vger.kernel.org, linuxppc-dev@lists.ozlabs.org, linux-ia64@vger.kernel.org, Baolin Wang , David Hildenbrand , Christophe Leroy , "Aneesh Kumar K . V" , Naoya Horiguchi , Michael Ellerman , Muchun Song , Andrew Morton Subject: Re: [PATCH v3] hugetlb: simplify hugetlb handling in follow_page_mask Message-ID: References: <20220919021348.22151-1-mike.kravetz@oracle.com> MIME-Version: 1.0 In-Reply-To: X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: multipart/mixed; boundary="oZ7pR8BiPAaKWB7x" Content-Disposition: inline ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1666972684; a=rsa-sha256; cv=none; b=kAxvgvNgR2SyJRYx3OO+y8/4nPB/VBZ2DtNkjA8oQFfFl0KXDN8GiUDB1fUBD1LaMpA6ef lYhBZt6y8jnfY+6i7i+t8OMA5wcVeEtCfONEXRqtkF5tDD7bqpXi1Iz1OvyzqT2d6hiXdO JrzMruggrMvwbxYKnY1ne5kOqrjYHNE= ARC-Authentication-Results: i=1; imf24.hostedemail.com; dkim=pass header.d=redhat.com header.s=mimecast20190719 header.b=CqcSrMD2; dmarc=pass (policy=none) header.from=redhat.com; spf=pass (imf24.hostedemail.com: domain of peterx@redhat.com designates 170.10.133.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=1666972684; 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=4h7qjUXjTtRvctXOxGac74UvVU5N5blmNjcxZ+F+ZAE=; b=hAji76LRSDbHexW2cW9O2KJTdtuXTdRCOrCHzYxUBwVdKe1NK8P/yY9uGN4jM8r1N/qWXW NDdT15VcIQz4HB6Ij1mLcMZ4HEtFZ3HIEos/tLHhZiaef3yqcpxe1QMLvZv+8cISqFdrdT kVniXSzo7EhkTDjLwFdaD8xch9UkGa4= X-Stat-Signature: 6ntoyydbandsuc35t6oahjitwh9x8d8a X-Rspamd-Queue-Id: B5728180009 X-Rspam-User: Authentication-Results: imf24.hostedemail.com; dkim=pass header.d=redhat.com header.s=mimecast20190719 header.b=CqcSrMD2; dmarc=pass (policy=none) header.from=redhat.com; spf=pass (imf24.hostedemail.com: domain of peterx@redhat.com designates 170.10.133.124 as permitted sender) smtp.mailfrom=peterx@redhat.com X-Rspamd-Server: rspam09 X-HE-Tag: 1666972684-339325 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: --oZ7pR8BiPAaKWB7x Content-Type: text/plain; charset=utf-8 Content-Disposition: inline On Fri, Oct 28, 2022 at 08:27:57AM -0700, Mike Kravetz wrote: > On 10/27/22 15:34, Peter Xu wrote: > > On Wed, Oct 26, 2022 at 05:34:04PM -0700, Mike Kravetz wrote: > > > On 10/26/22 17:59, Peter Xu wrote: > > > > If we want to use the vma read lock to protect here as the slow gup path, > > then please check again with below [1] - I think we'll also need to protect > > it with fast-gup (probably with trylock only, because fast-gup cannot > > sleep) or it'll encounter the same race, iiuc. > > > > Actually, instead of using vma lock, I really think this is another problem > > and needs standalone fixing. The problem is we allows huge_pte_offset() to > > walk the process pgtable without any protection, while pmd unsharing can > > drop a page anytime. huge_pte_offset() is always facing use-after-free > > when walking the PUD page. > > > > We may want RCU lock to protect the pgtable pages from getting away when > > huge_pte_offset() is walking it, it'll be safe then because pgtable pages > > are released in RCU fashion only (e.g. in above example, process [2] will > > munmap() and release the last ref to the "used to be shared" pmd and the > > PUD that maps the shared pmds will be released only after a RCU grace > > period), and afaict that's also what's protecting fast-gup from accessing > > freed pgtable pages. > > > > If with all huge_pte_offset() callers becoming RCU-safe, then IIUC we can > > drop the vma lock in all GUP code, aka, in hugetlb_follow_page_mask() here, > > because both slow and fast gup should be safe too in the same manner. > > > > Thanks, > > > > > > IIUC it's also the same as fast-gup - afaiu we don't take the read vma lock > > > > in fast-gup too but I also think it's safe. But I hope I didn't miss > > > > something. > > > > [1] > > Thanks Peter! I think the best thing would be to eliminate the vma_lock > calls in this patch. The code it is replacing/simplifying does not do any > locking, so no real regression. Agreed. > > I think a scheme like you describe above is going to require some more > thought/work. It might be better as a follow on patch. So above is only a thought, but if you think it's so far not very wrong and worth trying, I can see what I can get from it by some upcoming patches. It shouldn't need a lot of change, but basically looking after all huge_pte_offset() to make sure they're safe regarding walking the PUD. I'm attaching an initial patch to just start to comment on the usage of huge_pte_offset() first because that'll be the gust of the upcoming patchset (if there'll be), further comments welcomed too. Thanks. -- Peter Xu --oZ7pR8BiPAaKWB7x Content-Type: text/plain; charset=utf-8 Content-Disposition: attachment; filename="0001-mm-hugetlb-Comment-huge_pte_offset-for-its-locking-r.patch" >From 186c56026ce8ccc555d3c7ebc0e7e8fd76e9e5c9 Mon Sep 17 00:00:00 2001 From: Peter Xu Date: Thu, 27 Oct 2022 17:41:11 -0400 Subject: [PATCH] mm/hugetlb: Comment huge_pte_offset() for its locking requirements Content-type: text/plain huge_pte_offset() is potentially a pgtable walker, looking up pte_t* for a hugetlb address. Normally, it's always safe to walk the pgtable as long as we're with the mmap lock held for either read or write, because that guarantees the pgtable pages will always be valid during the process. But it's not true for hugetlbfs: hugetlbfs has the pmd sharing feature, it means that even with mmap lock held, the PUD pgtable page can still go away from under us if pmd unsharing is possible during the walk. It's not always the case, e.g.: (1) If the mapping is private we're not prone to pmd sharing or unsharing, so it's okay. (2) If we're with the hugetlb vma lock held for either read/write, it's okay too because pmd unshare cannot happen at all. Document all these explicitly for huge_pte_offset(), because it's really not that obvious. This also tells all the callers on what it needs to guarantee huge_pte_offset() thread-safety. Signed-off-by: Peter Xu --- arch/arm64/mm/hugetlbpage.c | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/arch/arm64/mm/hugetlbpage.c b/arch/arm64/mm/hugetlbpage.c index 35e9a468d13e..90f084643718 100644 --- a/arch/arm64/mm/hugetlbpage.c +++ b/arch/arm64/mm/hugetlbpage.c @@ -329,6 +329,35 @@ pte_t *huge_pte_alloc(struct mm_struct *mm, struct vm_area_struct *vma, return ptep; } +/* + * huge_pte_offset(): Walk the hugetlb pgtable until the last level PTE. + * Returns the pte_t* if found, or NULL if the address is not mapped. + * + * NOTE: since this function will walk all the pgtable pages (including not + * only normal pgtable page, but also PUD that can be unshared concurrently + * for VM_SHARED), the caller of this function should be responsible of its + * thread safety. One can follow this rule: + * + * (1) For private mappings: pmd unsharing is not possible, so it'll + * always be safe if we're with the mmap sem for either read or write. + * This is normally always the case, so IOW we don't need to do + * anything special. + * + * (2) For shared mappings: pmd unsharing is possible (so PUD page can go + * away from under us! It can be done by a pmd unshare with a follow + * up munmap() on the other process), then we need either: + * + * (2.1) hugetlb vma lock read or write held, to make sure pmd unshare + * won't happen upon the range, or, + * + * (2.2) RCU read lock, to make sure even pmd unsharing happened, the + * old shared PUD page won't get freed from under us, so even of + * the pte_t* can be obsolete, at least it's still always safe to + * access the PUD page. + * + * PS: from the regard of (2.2), it's the same logic of fast-gup being safe + * for generic mm, as long as RCU is used to free any pgtable page. + */ pte_t *huge_pte_offset(struct mm_struct *mm, unsigned long addr, unsigned long sz) { -- 2.37.3 --oZ7pR8BiPAaKWB7x--