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 bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id E18AFC48BC3 for ; Wed, 21 Feb 2024 09:38:02 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:References: Message-ID:Subject:Cc:To:From:Date:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=/hOqQwxiu3LLedF3EAUnUIH0ArfoG1HdXsFZ6e8XFyE=; b=Vc16jFnwdKV/y8 V5HlNYLYMXpyQKerV6naun8c9VkGdKcieRygKJ5dbDr2U9G2RtWWN7w1EzuW/2BrDyRnQQLisn0yp nstaBU8RBO/7UtIP7luD60oXkZC8jp0NxbnfWyUOgfe4bxwvSYh7Y6aaZiR84t4eoIv/BRuTzZ1FH brUQOexWJMobA+C9WvSbOMCe0eHHx1iBMnE7brvUInM+k5M6xzZDUmTL/jN0djRBWbdOTGeZZy4tV L9SP8KR3PFj7OcTDexaYskiMMi6Y+DzAfkeGq3hR0oEsbODOMWDx8DQ5u3jGytydGZRxj32+BAkAk wWdC4138x7cxeyXYJtDQ==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.97.1 #2 (Red Hat Linux)) id 1rcj37-00000000I0d-0YEj; Wed, 21 Feb 2024 09:37:57 +0000 Received: from us-smtp-delivery-124.mimecast.com ([170.10.133.124]) by bombadil.infradead.org with esmtps (Exim 4.97.1 #2 (Red Hat Linux)) id 1rcj34-00000000Hy7-33ZA for linux-riscv@lists.infradead.org; Wed, 21 Feb 2024 09:37:56 +0000 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1708508272; 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=BdaKpo1zKogZCn5PWAqhqXGnoFVUrqpY1LtEmaKu+FM=; b=UFHnGmnLLF1dyPsTJH5oE/3mBxEFT/++jT/G+HjD7XNGKQ3fM50r6Ae/bvTlMF2ZPJ50sA u4HFcu/ZT+AYd4PF/tCzxX75rWp8XDI2yzbRON2gREhNo/34KFrv5SRAVpeLgG8bJJHgaT AYFbp2Yq5fFDoJfM16nbxysxbaKm4+E= Received: from mail-pf1-f199.google.com (mail-pf1-f199.google.com [209.85.210.199]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-628-4ET42IxAMyijlcUM-uXXvQ-1; Wed, 21 Feb 2024 04:37:51 -0500 X-MC-Unique: 4ET42IxAMyijlcUM-uXXvQ-1 Received: by mail-pf1-f199.google.com with SMTP id d2e1a72fcca58-6db0e05548fso1863906b3a.1 for ; Wed, 21 Feb 2024 01:37:50 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1708508270; x=1709113070; 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=BdaKpo1zKogZCn5PWAqhqXGnoFVUrqpY1LtEmaKu+FM=; b=PSBVPuHgy3yovYf88bBoMDHcOjIjGbt/WUbhA5BDBGwcCIHXQ6coJxQp6eTo8N+EhJ ook3bBxF4rV2BiHLkoUgs/BUZUllZVGQ90rg2hZXxnlGANdSWUvmz+BJg02FWAlpoccq byDuKAlS06srzYL83tmwhZxgoMPnjXWcdiksvheurrIXxetrrDk57dq5G4XmCM4VsP4f lV+8OrjAdW0pX7JVIHzmfVtR02Sw/RiNgvNejArA+zXPJNTuY+GZFyuykx3KKQ8T51nq G+GJ17FJ/Axp84tyfS3gyi4RuLNyd3x74IqNeAwLKXo4S85Uu4ElkzLr8ret/NCLjW2T mBAg== X-Forwarded-Encrypted: i=1; AJvYcCVpQmM4RAEzf8cqPZinsbOlMcis06f6jEeNqJaouBG6BwqLsjbSeSNyvDGmjM7djMYuffZIjygMrXehw+p9qbgxfTgr3P2dSOJdOCiDRyKl X-Gm-Message-State: AOJu0YwACb7W55ZRz7IzQs3NTOr+rJc9bVXC7UXTYXlUDEznElrE1bT7 8PgZub9CEdyzBZLzXZOqe+YPQGmz1g20WG4rqxDda0TiOC6w4PGun9vP1G+WkFZB+ijNMLuQ1cG obJpdr/7pIeY2UP5Rwv7pkyur6JwUUQFY4XfnvuSr1eQ2Ry669t5cV2H7RTIfW4ecNw== X-Received: by 2002:a05:6a00:3d13:b0:6e4:672b:f384 with SMTP id lo19-20020a056a003d1300b006e4672bf384mr7669542pfb.2.1708508269926; Wed, 21 Feb 2024 01:37:49 -0800 (PST) X-Google-Smtp-Source: AGHT+IGnK/5Xc9SKm0wQPEnUuYiqinVCHstqfWVnR5OuYxZ4xQLJmopCSPYZqNRQ2oNZWFAP27lsbg== X-Received: by 2002:a05:6a00:3d13:b0:6e4:672b:f384 with SMTP id lo19-20020a056a003d1300b006e4672bf384mr7669524pfb.2.1708508269578; Wed, 21 Feb 2024 01:37:49 -0800 (PST) Received: from x1n ([43.228.180.230]) by smtp.gmail.com with ESMTPSA id e11-20020aa7980b000000b006e4698d53casm4990624pfl.140.2024.02.21.01.37.43 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 21 Feb 2024 01:37:49 -0800 (PST) Date: Wed, 21 Feb 2024 17:37:37 +0800 From: Peter Xu To: Jason Gunthorpe Cc: linux-mm@kvack.org, linux-kernel@vger.kernel.org, James Houghton , David Hildenbrand , "Kirill A . Shutemov" , Yang Shi , linux-riscv@lists.infradead.org, Andrew Morton , "Aneesh Kumar K . V" , Rik van Riel , Andrea Arcangeli , Axel Rasmussen , Mike Rapoport , John Hubbard , Vlastimil Babka , Michael Ellerman , Christophe Leroy , Andrew Jones , linuxppc-dev@lists.ozlabs.org, Mike Kravetz , Muchun Song , linux-arm-kernel@lists.infradead.org, Christoph Hellwig , Lorenzo Stoakes , Matthew Wilcox Subject: Re: [PATCH v2 03/13] mm: Provide generic pmd_thp_or_huge() Message-ID: References: <20240103091423.400294-1-peterx@redhat.com> <20240103091423.400294-4-peterx@redhat.com> <20240115175551.GP734935@nvidia.com> MIME-Version: 1.0 In-Reply-To: <20240115175551.GP734935@nvidia.com> X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Disposition: inline X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20240221_013754_887950_03861A9B X-CRM114-Status: GOOD ( 35.23 ) X-BeenThere: linux-riscv@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-riscv" Errors-To: linux-riscv-bounces+linux-riscv=archiver.kernel.org@lists.infradead.org On Mon, Jan 15, 2024 at 01:55:51PM -0400, Jason Gunthorpe wrote: > On Wed, Jan 03, 2024 at 05:14:13PM +0800, peterx@redhat.com wrote: > > From: Peter Xu > > > > ARM defines pmd_thp_or_huge(), detecting either a THP or a huge PMD. It > > can be a helpful helper if we want to merge more THP and hugetlb code > > paths. Make it a generic default implementation, only exist when > > CONFIG_MMU. Arch can overwrite it by defining its own version. > > > > For example, ARM's pgtable-2level.h defines it to always return false. > > > > Keep the macro declared with all config, it should be optimized to a false > > anyway if !THP && !HUGETLB. > > > > Signed-off-by: Peter Xu > > --- > > include/linux/pgtable.h | 4 ++++ > > mm/gup.c | 3 +-- > > 2 files changed, 5 insertions(+), 2 deletions(-) > > > > diff --git a/include/linux/pgtable.h b/include/linux/pgtable.h > > index 466cf477551a..2b42e95a4e3a 100644 > > --- a/include/linux/pgtable.h > > +++ b/include/linux/pgtable.h > > @@ -1362,6 +1362,10 @@ static inline int pmd_write(pmd_t pmd) > > #endif /* pmd_write */ > > #endif /* CONFIG_TRANSPARENT_HUGEPAGE */ > > > > +#ifndef pmd_thp_or_huge > > +#define pmd_thp_or_huge(pmd) (pmd_huge(pmd) || pmd_trans_huge(pmd)) > > +#endif > > Why not just use pmd_leaf() ? > > This GUP case seems to me exactly like what pmd_leaf() should really > do and be used for.. I think I mostly agree with you, and these APIs are indeed confusing. IMHO the challenge is about the risk of breaking others on small changes in the details where evil resides. > > eg x86 does: > > #define pmd_leaf pmd_large > static inline int pmd_large(pmd_t pte) > return pmd_flags(pte) & _PAGE_PSE; > > static inline int pmd_trans_huge(pmd_t pmd) > return (pmd_val(pmd) & (_PAGE_PSE|_PAGE_DEVMAP)) == _PAGE_PSE; > > int pmd_huge(pmd_t pmd) > return !pmd_none(pmd) && > (pmd_val(pmd) & (_PAGE_PRESENT|_PAGE_PSE)) != _PAGE_PRESENT; For example, here I don't think it's strictly pmd_leaf()? As pmd_huge() will return true if PRESENT=0 && PSE=0 (as long as none pte ruled out first), while pmd_leaf() will return false; I think that came from cbef8478bee5. I'm not sure whether that is the best solution, e.g., from a 1st glance it seems better to me to process swap entries separately (including both migration and poisoned entries).. Sparc has similar things there, which in that case I'm not sure whether a direct replace is always safe. Besides that, there're also other cases where it's not clear of such direct replacement, not until further investigated. E.g., arm-3level has: #define pmd_leaf(pmd) pmd_sect(pmd) #define pmd_sect(pmd) ((pmd_val(pmd) & PMD_TYPE_MASK) == \ PMD_TYPE_SECT) #define PMD_TYPE_SECT (_AT(pmdval_t, 1) << 0) While pmd_huge() there relies on PMD_TABLE_BIT () int pmd_huge(pmd_t pmd) { return pmd_val(pmd) && !(pmd_val(pmd) & PMD_TABLE_BIT); } #define PMD_TABLE_BIT (_AT(pmdval_t, 1) << 1) These are just the trivial details that I wanted to avoid to touch in this series, so as to resolve the hugetlb issue separately from others. The new pmd_huge_or_thp() is not ideal, but that easily isolates all these trivial details / evils out of the picture, so that we can tackle them one by one. It is strictly an OR or huge||thp, so it's hopefully safe to not break anything yet from that regard. > > I spot checked a couple arches and it looks like it holds up. > > Further, it looks to me like this site in GUP is the only core code > caller.. > > So, I'd suggest a small series to go arch by arch and convert the arch > to use pmd_huge() == pmd_leaf(). Then retire pmd_huge() as a public > API. > > > diff --git a/mm/gup.c b/mm/gup.c > > index df83182ec72d..eebae70d2465 100644 > > --- a/mm/gup.c > > +++ b/mm/gup.c > > @@ -3004,8 +3004,7 @@ static int gup_pmd_range(pud_t *pudp, pud_t pud, unsigned long addr, unsigned lo > > if (!pmd_present(pmd)) > > return 0; > > > > - if (unlikely(pmd_trans_huge(pmd) || pmd_huge(pmd) || > > - pmd_devmap(pmd))) { > > + if (unlikely(pmd_thp_or_huge(pmd) || pmd_devmap(pmd))) { > > /* See gup_pte_range() */ > > if (pmd_protnone(pmd)) > > return 0; > > And the devmap thing here doesn't make any sense either. The arch > should ensure that pmd_devmap() implies pmd_leaf(). Since devmap is a > purely SW construct it almost certainly does already anyhow. Yep, but only if pmd_leaf() is safe to be put here. A pmd devmap should always imply as a pmd_leaf() indeed. Thanks, -- Peter Xu _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv 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 lists.ozlabs.org (lists.ozlabs.org [112.213.38.117]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 86B8CC48BC3 for ; Wed, 21 Feb 2024 09:38:44 +0000 (UTC) Authentication-Results: lists.ozlabs.org; dkim=fail reason="signature verification failed" (1024-bit key; unprotected) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=TWbID7n+; dkim=fail reason="signature verification failed" (1024-bit key) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=TWbID7n+; dkim-atps=neutral Received: from boromir.ozlabs.org (localhost [IPv6:::1]) by lists.ozlabs.org (Postfix) with ESMTP id 4TfrnH17wYz3dSg for ; Wed, 21 Feb 2024 20:38:43 +1100 (AEDT) Authentication-Results: lists.ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=TWbID7n+; dkim=pass (1024-bit key) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=TWbID7n+; dkim-atps=neutral Authentication-Results: lists.ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=redhat.com (client-ip=170.10.129.124; helo=us-smtp-delivery-124.mimecast.com; envelope-from=peterx@redhat.com; receiver=lists.ozlabs.org) Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by lists.ozlabs.org (Postfix) with ESMTPS id 4TfrmN6Wqjz2xct for ; Wed, 21 Feb 2024 20:37:56 +1100 (AEDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1708508273; 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=BdaKpo1zKogZCn5PWAqhqXGnoFVUrqpY1LtEmaKu+FM=; b=TWbID7n+qbDmuF4T2xzcnY5fP3IEI2OKoLN5qO2Xzf/mWmj4777RmxQYNuT8IPYJjvZIvK xL2depBBZbYVeGeluk7AWUiutzNPb919EQDblH3GnmGunihd4L35Vt4G4l+0Ul+3zZ9irv e3P4yOZONvEhAaj7dYKP5DScZEk5Axc= DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1708508273; 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=BdaKpo1zKogZCn5PWAqhqXGnoFVUrqpY1LtEmaKu+FM=; b=TWbID7n+qbDmuF4T2xzcnY5fP3IEI2OKoLN5qO2Xzf/mWmj4777RmxQYNuT8IPYJjvZIvK xL2depBBZbYVeGeluk7AWUiutzNPb919EQDblH3GnmGunihd4L35Vt4G4l+0Ul+3zZ9irv e3P4yOZONvEhAaj7dYKP5DScZEk5Axc= Received: from mail-pf1-f198.google.com (mail-pf1-f198.google.com [209.85.210.198]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-151-4cqlvdpmMpK395xYbqJ6eQ-1; Wed, 21 Feb 2024 04:37:51 -0500 X-MC-Unique: 4cqlvdpmMpK395xYbqJ6eQ-1 Received: by mail-pf1-f198.google.com with SMTP id d2e1a72fcca58-6db0e05548fso1863915b3a.1 for ; Wed, 21 Feb 2024 01:37:50 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1708508270; x=1709113070; 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=BdaKpo1zKogZCn5PWAqhqXGnoFVUrqpY1LtEmaKu+FM=; b=BTO952HugBtTAhhuyZgtYdt1KZf/V2NrPrcKqyuwc3PeOJod+OlcuUJknY0lb8Jc0I wY+5l1j7dgr8kESfAeaVwm4uPVVGhfY+kdEVeF2wi4PQiJ/ts0D6LYoiEpUBmtoA/h3G TcAeeAjuz4+P7ecoS03E379jD8cJkmGy09Gxm/cHX+emaEPAotmk/Lug+yzzJdhozy4H V8tL7eaxVyhMVNa//n5C5B0RsZSYsMqjCjma+HB7w+5ZSo3XdgcF2c8pc4goPZaZR3LW C461rFFKkLDkvcLtEONUyWdjL/Mjst9nIBzBiSbN8SYJY2TxjWd6SKlnprQMT4ddAlnl 9pTw== X-Forwarded-Encrypted: i=1; AJvYcCWuRhPzNz4EeanH/XZriFeTN+c84X4Rp3wJoLNhLIHjl+6RU4MnoD6SohMA5fwXY96RbdmEl0cn1F9O1kTU++st9gq0i5BAzRPZ2QVxNw== X-Gm-Message-State: AOJu0Yw6WuSClGMaMx3F7W7pLSvRcje7QT8wof7UA0DZRT4N5CfJ5q82 FV2UjGQLuj7DvLuOaUki4ZuptZTfSnymOVzCxFukTeBOxipQPUk3v/+JpsSuGVYfS8S9ZrBjEI9 18p6ahYTCpSnr2a9AoU4D+muofAk8MbbPvacDVq6PkOJ0U6kuo4Sz66Ee7z0YVxE= X-Received: by 2002:a05:6a00:3d13:b0:6e4:672b:f384 with SMTP id lo19-20020a056a003d1300b006e4672bf384mr7669552pfb.2.1708508269937; Wed, 21 Feb 2024 01:37:49 -0800 (PST) X-Google-Smtp-Source: AGHT+IGnK/5Xc9SKm0wQPEnUuYiqinVCHstqfWVnR5OuYxZ4xQLJmopCSPYZqNRQ2oNZWFAP27lsbg== X-Received: by 2002:a05:6a00:3d13:b0:6e4:672b:f384 with SMTP id lo19-20020a056a003d1300b006e4672bf384mr7669524pfb.2.1708508269578; Wed, 21 Feb 2024 01:37:49 -0800 (PST) Received: from x1n ([43.228.180.230]) by smtp.gmail.com with ESMTPSA id e11-20020aa7980b000000b006e4698d53casm4990624pfl.140.2024.02.21.01.37.43 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 21 Feb 2024 01:37:49 -0800 (PST) Date: Wed, 21 Feb 2024 17:37:37 +0800 From: Peter Xu To: Jason Gunthorpe Subject: Re: [PATCH v2 03/13] mm: Provide generic pmd_thp_or_huge() Message-ID: References: <20240103091423.400294-1-peterx@redhat.com> <20240103091423.400294-4-peterx@redhat.com> <20240115175551.GP734935@nvidia.com> MIME-Version: 1.0 In-Reply-To: <20240115175551.GP734935@nvidia.com> X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=utf-8 Content-Disposition: inline X-BeenThere: linuxppc-dev@lists.ozlabs.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Linux on PowerPC Developers Mail List List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: James Houghton , David Hildenbrand , Yang Shi , Andrew Jones , linux-mm@kvack.org, Matthew Wilcox , linux-riscv@lists.infradead.org, Andrea Arcangeli , Christoph Hellwig , "Aneesh Kumar K . V" , Vlastimil Babka , Axel Rasmussen , Rik van Riel , John Hubbard , "Kirill A . Shutemov" , linux-arm-kernel@lists.infradead.org, Lorenzo Stoakes , Muchun Song , linux-kernel@vger.kernel.org, Andrew Morton , linuxppc-dev@lists.ozlabs.org, Mike Rapoport , Mike Kravetz Errors-To: linuxppc-dev-bounces+linuxppc-dev=archiver.kernel.org@lists.ozlabs.org Sender: "Linuxppc-dev" On Mon, Jan 15, 2024 at 01:55:51PM -0400, Jason Gunthorpe wrote: > On Wed, Jan 03, 2024 at 05:14:13PM +0800, peterx@redhat.com wrote: > > From: Peter Xu > > > > ARM defines pmd_thp_or_huge(), detecting either a THP or a huge PMD. It > > can be a helpful helper if we want to merge more THP and hugetlb code > > paths. Make it a generic default implementation, only exist when > > CONFIG_MMU. Arch can overwrite it by defining its own version. > > > > For example, ARM's pgtable-2level.h defines it to always return false. > > > > Keep the macro declared with all config, it should be optimized to a false > > anyway if !THP && !HUGETLB. > > > > Signed-off-by: Peter Xu > > --- > > include/linux/pgtable.h | 4 ++++ > > mm/gup.c | 3 +-- > > 2 files changed, 5 insertions(+), 2 deletions(-) > > > > diff --git a/include/linux/pgtable.h b/include/linux/pgtable.h > > index 466cf477551a..2b42e95a4e3a 100644 > > --- a/include/linux/pgtable.h > > +++ b/include/linux/pgtable.h > > @@ -1362,6 +1362,10 @@ static inline int pmd_write(pmd_t pmd) > > #endif /* pmd_write */ > > #endif /* CONFIG_TRANSPARENT_HUGEPAGE */ > > > > +#ifndef pmd_thp_or_huge > > +#define pmd_thp_or_huge(pmd) (pmd_huge(pmd) || pmd_trans_huge(pmd)) > > +#endif > > Why not just use pmd_leaf() ? > > This GUP case seems to me exactly like what pmd_leaf() should really > do and be used for.. I think I mostly agree with you, and these APIs are indeed confusing. IMHO the challenge is about the risk of breaking others on small changes in the details where evil resides. > > eg x86 does: > > #define pmd_leaf pmd_large > static inline int pmd_large(pmd_t pte) > return pmd_flags(pte) & _PAGE_PSE; > > static inline int pmd_trans_huge(pmd_t pmd) > return (pmd_val(pmd) & (_PAGE_PSE|_PAGE_DEVMAP)) == _PAGE_PSE; > > int pmd_huge(pmd_t pmd) > return !pmd_none(pmd) && > (pmd_val(pmd) & (_PAGE_PRESENT|_PAGE_PSE)) != _PAGE_PRESENT; For example, here I don't think it's strictly pmd_leaf()? As pmd_huge() will return true if PRESENT=0 && PSE=0 (as long as none pte ruled out first), while pmd_leaf() will return false; I think that came from cbef8478bee5. I'm not sure whether that is the best solution, e.g., from a 1st glance it seems better to me to process swap entries separately (including both migration and poisoned entries).. Sparc has similar things there, which in that case I'm not sure whether a direct replace is always safe. Besides that, there're also other cases where it's not clear of such direct replacement, not until further investigated. E.g., arm-3level has: #define pmd_leaf(pmd) pmd_sect(pmd) #define pmd_sect(pmd) ((pmd_val(pmd) & PMD_TYPE_MASK) == \ PMD_TYPE_SECT) #define PMD_TYPE_SECT (_AT(pmdval_t, 1) << 0) While pmd_huge() there relies on PMD_TABLE_BIT () int pmd_huge(pmd_t pmd) { return pmd_val(pmd) && !(pmd_val(pmd) & PMD_TABLE_BIT); } #define PMD_TABLE_BIT (_AT(pmdval_t, 1) << 1) These are just the trivial details that I wanted to avoid to touch in this series, so as to resolve the hugetlb issue separately from others. The new pmd_huge_or_thp() is not ideal, but that easily isolates all these trivial details / evils out of the picture, so that we can tackle them one by one. It is strictly an OR or huge||thp, so it's hopefully safe to not break anything yet from that regard. > > I spot checked a couple arches and it looks like it holds up. > > Further, it looks to me like this site in GUP is the only core code > caller.. > > So, I'd suggest a small series to go arch by arch and convert the arch > to use pmd_huge() == pmd_leaf(). Then retire pmd_huge() as a public > API. > > > diff --git a/mm/gup.c b/mm/gup.c > > index df83182ec72d..eebae70d2465 100644 > > --- a/mm/gup.c > > +++ b/mm/gup.c > > @@ -3004,8 +3004,7 @@ static int gup_pmd_range(pud_t *pudp, pud_t pud, unsigned long addr, unsigned lo > > if (!pmd_present(pmd)) > > return 0; > > > > - if (unlikely(pmd_trans_huge(pmd) || pmd_huge(pmd) || > > - pmd_devmap(pmd))) { > > + if (unlikely(pmd_thp_or_huge(pmd) || pmd_devmap(pmd))) { > > /* See gup_pte_range() */ > > if (pmd_protnone(pmd)) > > return 0; > > And the devmap thing here doesn't make any sense either. The arch > should ensure that pmd_devmap() implies pmd_leaf(). Since devmap is a > purely SW construct it almost certainly does already anyhow. Yep, but only if pmd_leaf() is safe to be put here. A pmd devmap should always imply as a pmd_leaf() indeed. Thanks, -- Peter Xu 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 bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 5B639C48BC3 for ; Wed, 21 Feb 2024 09:38:15 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:References: Message-ID:Subject:Cc:To:From:Date:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=UyCrImXbb6ZXkVfo6RosA5xotfjBYn1DLyqla/QLCMA=; b=woFLfRFY7ZtM7d xd4e0TSV4LosSeTit3AWZG46AbS2giFilx8uefdQdOe3u5OjtTUK6J6Gc/juD4bSMhfdyjbzN5VmZ kIiQf+VUoRKm79hX7OdvEDfZ1syJOWzCgRq07TJ8VGioBgWHPeCHnJHSMi4bbDRh4P9lh1oL31OYe uQVZDtMBmbb/PERREfFN/qxOyf61T4Cvj5mM7A/H6Z8Ymmu5bXv6bSctUTEE0CN686CmoC1UypeoE gu9ZMTGabiAEoplU99qF4dBktThJAWkPHgYTydni8CKFbevSzLvFwROK2duB4u/OlAvi5is1ds5q6 c+nZ2FAhwxLm2wWGKgig==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.97.1 #2 (Red Hat Linux)) id 1rcj39-00000000I1I-0JFF; Wed, 21 Feb 2024 09:37:59 +0000 Received: from us-smtp-delivery-124.mimecast.com ([170.10.129.124]) by bombadil.infradead.org with esmtps (Exim 4.97.1 #2 (Red Hat Linux)) id 1rcj35-00000000Hze-3tKE for linux-arm-kernel@lists.infradead.org; Wed, 21 Feb 2024 09:37:57 +0000 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1708508275; 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=BdaKpo1zKogZCn5PWAqhqXGnoFVUrqpY1LtEmaKu+FM=; b=fMR0Z0lfvYAh8TQwo8UF+VD4QSch0E4j0GAEIYsarLe5ecJz6/WpqQmkXPOvCDXRlWwZBB hqWBzTGZOf+Ruj1hkluoqLFisD4rU3IvM8fnJeSE9UuxuADswA6gsJPBFy6aSTSVCRUu2c 18r8NXE0LCEU96MvYO7ptcF7075La5c= Received: from mail-pf1-f199.google.com (mail-pf1-f199.google.com [209.85.210.199]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-447-9nljTXzNNfitVsXvDAxRjw-1; Wed, 21 Feb 2024 04:37:51 -0500 X-MC-Unique: 9nljTXzNNfitVsXvDAxRjw-1 Received: by mail-pf1-f199.google.com with SMTP id d2e1a72fcca58-6db0e05548fso1863903b3a.1 for ; Wed, 21 Feb 2024 01:37:50 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1708508270; x=1709113070; 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=BdaKpo1zKogZCn5PWAqhqXGnoFVUrqpY1LtEmaKu+FM=; b=KrrNLgy/FwTaCYbyI9ivWipN5dQwrgTrbB9HOVndgqlrgnLvr9ZPefTrFJogbTS4+B sBeJ4PuBmOrJwc97IG0R1Hk/O9WdNPvQ3cpIoE6+TfyWoa3+YbK0ZhXpfa+RSs//ly60 FaSOfdYhA1AtGvU15ZAlHc57m6NGd1l7MboXdhbajXJz925jBtntAgf97JN9sdcipujS 8pyePnfbH+rMw4kXk953J0jNrP4sjMTooWxeF21qERB8MeUKRu0WK2vIwAUon0lmWSXC VJBFuqGt17yz9PYYaaTG45APTPNV8ddw6ZKR7b+0lTi9iohZ3g4UO9faBM8gLUJ1MR7e 0j8g== X-Forwarded-Encrypted: i=1; AJvYcCXBjW4lGGz7z2WyTHj8JPCEKb+3LIdR7f0u7tf6xAvDUoVe8HPczVyi0N699WSH0r49gjQk+5l52KOk8U9Rfghp5BPD4cJ6nllcdaH2negqaNt+/64= X-Gm-Message-State: AOJu0YxUn9MLDtiWtfGfgIGz282j4J/0FSLwuTc1SEtpv378Vh8rNPu3 XvP1kNewGaNH5BiFzsy0oQPM6pbVHtbkSgCJwSOR7vLoe0ecz5zucBPIn1rwSYGHx5jBTxCXIjt HwtLm3g0Z3iTjk5XU2hwqOL9Z7nAIeeOQGeSPc9j/16qE2TniWN8Gm6nzUYdOuJhnf01HgOAJ X-Received: by 2002:a05:6a00:3d13:b0:6e4:672b:f384 with SMTP id lo19-20020a056a003d1300b006e4672bf384mr7669551pfb.2.1708508269937; Wed, 21 Feb 2024 01:37:49 -0800 (PST) X-Google-Smtp-Source: AGHT+IGnK/5Xc9SKm0wQPEnUuYiqinVCHstqfWVnR5OuYxZ4xQLJmopCSPYZqNRQ2oNZWFAP27lsbg== X-Received: by 2002:a05:6a00:3d13:b0:6e4:672b:f384 with SMTP id lo19-20020a056a003d1300b006e4672bf384mr7669524pfb.2.1708508269578; Wed, 21 Feb 2024 01:37:49 -0800 (PST) Received: from x1n ([43.228.180.230]) by smtp.gmail.com with ESMTPSA id e11-20020aa7980b000000b006e4698d53casm4990624pfl.140.2024.02.21.01.37.43 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 21 Feb 2024 01:37:49 -0800 (PST) Date: Wed, 21 Feb 2024 17:37:37 +0800 From: Peter Xu To: Jason Gunthorpe Cc: linux-mm@kvack.org, linux-kernel@vger.kernel.org, James Houghton , David Hildenbrand , "Kirill A . Shutemov" , Yang Shi , linux-riscv@lists.infradead.org, Andrew Morton , "Aneesh Kumar K . V" , Rik van Riel , Andrea Arcangeli , Axel Rasmussen , Mike Rapoport , John Hubbard , Vlastimil Babka , Michael Ellerman , Christophe Leroy , Andrew Jones , linuxppc-dev@lists.ozlabs.org, Mike Kravetz , Muchun Song , linux-arm-kernel@lists.infradead.org, Christoph Hellwig , Lorenzo Stoakes , Matthew Wilcox Subject: Re: [PATCH v2 03/13] mm: Provide generic pmd_thp_or_huge() Message-ID: References: <20240103091423.400294-1-peterx@redhat.com> <20240103091423.400294-4-peterx@redhat.com> <20240115175551.GP734935@nvidia.com> MIME-Version: 1.0 In-Reply-To: <20240115175551.GP734935@nvidia.com> X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Disposition: inline X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20240221_013756_062269_B5C44B9B X-CRM114-Status: GOOD ( 36.87 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Mon, Jan 15, 2024 at 01:55:51PM -0400, Jason Gunthorpe wrote: > On Wed, Jan 03, 2024 at 05:14:13PM +0800, peterx@redhat.com wrote: > > From: Peter Xu > > > > ARM defines pmd_thp_or_huge(), detecting either a THP or a huge PMD. It > > can be a helpful helper if we want to merge more THP and hugetlb code > > paths. Make it a generic default implementation, only exist when > > CONFIG_MMU. Arch can overwrite it by defining its own version. > > > > For example, ARM's pgtable-2level.h defines it to always return false. > > > > Keep the macro declared with all config, it should be optimized to a false > > anyway if !THP && !HUGETLB. > > > > Signed-off-by: Peter Xu > > --- > > include/linux/pgtable.h | 4 ++++ > > mm/gup.c | 3 +-- > > 2 files changed, 5 insertions(+), 2 deletions(-) > > > > diff --git a/include/linux/pgtable.h b/include/linux/pgtable.h > > index 466cf477551a..2b42e95a4e3a 100644 > > --- a/include/linux/pgtable.h > > +++ b/include/linux/pgtable.h > > @@ -1362,6 +1362,10 @@ static inline int pmd_write(pmd_t pmd) > > #endif /* pmd_write */ > > #endif /* CONFIG_TRANSPARENT_HUGEPAGE */ > > > > +#ifndef pmd_thp_or_huge > > +#define pmd_thp_or_huge(pmd) (pmd_huge(pmd) || pmd_trans_huge(pmd)) > > +#endif > > Why not just use pmd_leaf() ? > > This GUP case seems to me exactly like what pmd_leaf() should really > do and be used for.. I think I mostly agree with you, and these APIs are indeed confusing. IMHO the challenge is about the risk of breaking others on small changes in the details where evil resides. > > eg x86 does: > > #define pmd_leaf pmd_large > static inline int pmd_large(pmd_t pte) > return pmd_flags(pte) & _PAGE_PSE; > > static inline int pmd_trans_huge(pmd_t pmd) > return (pmd_val(pmd) & (_PAGE_PSE|_PAGE_DEVMAP)) == _PAGE_PSE; > > int pmd_huge(pmd_t pmd) > return !pmd_none(pmd) && > (pmd_val(pmd) & (_PAGE_PRESENT|_PAGE_PSE)) != _PAGE_PRESENT; For example, here I don't think it's strictly pmd_leaf()? As pmd_huge() will return true if PRESENT=0 && PSE=0 (as long as none pte ruled out first), while pmd_leaf() will return false; I think that came from cbef8478bee5. I'm not sure whether that is the best solution, e.g., from a 1st glance it seems better to me to process swap entries separately (including both migration and poisoned entries).. Sparc has similar things there, which in that case I'm not sure whether a direct replace is always safe. Besides that, there're also other cases where it's not clear of such direct replacement, not until further investigated. E.g., arm-3level has: #define pmd_leaf(pmd) pmd_sect(pmd) #define pmd_sect(pmd) ((pmd_val(pmd) & PMD_TYPE_MASK) == \ PMD_TYPE_SECT) #define PMD_TYPE_SECT (_AT(pmdval_t, 1) << 0) While pmd_huge() there relies on PMD_TABLE_BIT () int pmd_huge(pmd_t pmd) { return pmd_val(pmd) && !(pmd_val(pmd) & PMD_TABLE_BIT); } #define PMD_TABLE_BIT (_AT(pmdval_t, 1) << 1) These are just the trivial details that I wanted to avoid to touch in this series, so as to resolve the hugetlb issue separately from others. The new pmd_huge_or_thp() is not ideal, but that easily isolates all these trivial details / evils out of the picture, so that we can tackle them one by one. It is strictly an OR or huge||thp, so it's hopefully safe to not break anything yet from that regard. > > I spot checked a couple arches and it looks like it holds up. > > Further, it looks to me like this site in GUP is the only core code > caller.. > > So, I'd suggest a small series to go arch by arch and convert the arch > to use pmd_huge() == pmd_leaf(). Then retire pmd_huge() as a public > API. > > > diff --git a/mm/gup.c b/mm/gup.c > > index df83182ec72d..eebae70d2465 100644 > > --- a/mm/gup.c > > +++ b/mm/gup.c > > @@ -3004,8 +3004,7 @@ static int gup_pmd_range(pud_t *pudp, pud_t pud, unsigned long addr, unsigned lo > > if (!pmd_present(pmd)) > > return 0; > > > > - if (unlikely(pmd_trans_huge(pmd) || pmd_huge(pmd) || > > - pmd_devmap(pmd))) { > > + if (unlikely(pmd_thp_or_huge(pmd) || pmd_devmap(pmd))) { > > /* See gup_pte_range() */ > > if (pmd_protnone(pmd)) > > return 0; > > And the devmap thing here doesn't make any sense either. The arch > should ensure that pmd_devmap() implies pmd_leaf(). Since devmap is a > purely SW construct it almost certainly does already anyhow. Yep, but only if pmd_leaf() is safe to be put here. A pmd devmap should always imply as a pmd_leaf() indeed. Thanks, -- Peter Xu _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel 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 56C30C48BC3 for ; Wed, 21 Feb 2024 09:37:57 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id CA0536B0088; Wed, 21 Feb 2024 04:37:56 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id C28CF6B0089; Wed, 21 Feb 2024 04:37:56 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id AA4036B008A; Wed, 21 Feb 2024 04:37:56 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0015.hostedemail.com [216.40.44.15]) by kanga.kvack.org (Postfix) with ESMTP id 941236B0088 for ; Wed, 21 Feb 2024 04:37:56 -0500 (EST) Received: from smtpin10.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay08.hostedemail.com (Postfix) with ESMTP id 59F6C1409A8 for ; Wed, 21 Feb 2024 09:37:56 +0000 (UTC) X-FDA: 81815309352.10.AB8AB42 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by imf08.hostedemail.com (Postfix) with ESMTP id 4F5A216000F for ; Wed, 21 Feb 2024 09:37:53 +0000 (UTC) Authentication-Results: imf08.hostedemail.com; dkim=pass header.d=redhat.com header.s=mimecast20190719 header.b=UFHnGmnL; dmarc=pass (policy=none) header.from=redhat.com; spf=pass (imf08.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=1708508273; 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=BdaKpo1zKogZCn5PWAqhqXGnoFVUrqpY1LtEmaKu+FM=; b=nzepekUW8sqRMpJ8ycLY3LxroF+H4BrAc2eEvRcDga2FwaLtc7fy0ZmIE2fzDgUl+sSIQ2 4KJd9M9yvUAizyANDOrIQkanL2wTAKIEBweLn0SE8VOSTpUOJjFG/pA14JbMMz8YKmnZCr qf8T1Dh46UAO3o6jbY6xI4xHzfLNyKQ= ARC-Authentication-Results: i=1; imf08.hostedemail.com; dkim=pass header.d=redhat.com header.s=mimecast20190719 header.b=UFHnGmnL; dmarc=pass (policy=none) header.from=redhat.com; spf=pass (imf08.hostedemail.com: domain of peterx@redhat.com designates 170.10.129.124 as permitted sender) smtp.mailfrom=peterx@redhat.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1708508273; a=rsa-sha256; cv=none; b=yvzQY/wIcSQQ9V3+OtsXb1naFnhKlnGjYDRFdvEu6EAgZsTlspUgT0I/6WvinK7lKwyazW WAhupIjnFWTdOJKC77UcbH+UXPbi+wjG8xVXWjtiBZ8G0+qBshk0DDNnyZP34D6MoRXf85 6cyk0PcHLiUgFQXHeUSDjrHQD3E4oJs= DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1708508272; 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=BdaKpo1zKogZCn5PWAqhqXGnoFVUrqpY1LtEmaKu+FM=; b=UFHnGmnLLF1dyPsTJH5oE/3mBxEFT/++jT/G+HjD7XNGKQ3fM50r6Ae/bvTlMF2ZPJ50sA u4HFcu/ZT+AYd4PF/tCzxX75rWp8XDI2yzbRON2gREhNo/34KFrv5SRAVpeLgG8bJJHgaT AYFbp2Yq5fFDoJfM16nbxysxbaKm4+E= Received: from mail-pf1-f197.google.com (mail-pf1-f197.google.com [209.85.210.197]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-659-Z-zq3zJDMtK_Rt9nZb43HA-1; Wed, 21 Feb 2024 04:37:51 -0500 X-MC-Unique: Z-zq3zJDMtK_Rt9nZb43HA-1 Received: by mail-pf1-f197.google.com with SMTP id d2e1a72fcca58-6db0e05548fso1863916b3a.1 for ; Wed, 21 Feb 2024 01:37:50 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1708508270; x=1709113070; 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=BdaKpo1zKogZCn5PWAqhqXGnoFVUrqpY1LtEmaKu+FM=; b=Ha6oGFf8ivHqz/ieXX9cLpNX8mZ1aRdgpeskJ/0YV0tRCi+6CtvqDQP5V/20xkZ3sd Rpis5UwLuOyL6yyOiDZUGsrJvNtMI8talKivCcQYGMK5hgKIKxCMIQVH5DD3DUxEE4nR xE5QF9VLGmNWlSYCAVuMpvZGxVRCjOe0Yuz8Ejk72DP7tD3RV7ggyyBi87foJFcCqPrn 9hNfBJPNX+qN3Zd/yRfF3ZLxSq7h66aOfxeh2GXT00IQCNlSieFss6N2nscExprfeiDS k8tcLRwTu/Cr6wmSPjuEKlDd0Hn9ktyYjd7ucSNmVYxdlZSUUNn9NCOq0LdqPu7pAZDl hgDA== X-Gm-Message-State: AOJu0Yy4tNzYJDx5eA1ZELnnP9yxODwZrogyrYHCRfAMgQnLZWTnhXJw zsKgR5/Dd41c6uxfZOhabB5DvwuHKwf606a/ewmvo7HXrD6M9T8GcfouFagUpEWoXVVLsxPapXq nt4aYIXyyJ8pTp33tr8hMNyKQUlDBkV8cnBKnz7TGMib4E2e4 X-Received: by 2002:a05:6a00:3d13:b0:6e4:672b:f384 with SMTP id lo19-20020a056a003d1300b006e4672bf384mr7669554pfb.2.1708508269941; Wed, 21 Feb 2024 01:37:49 -0800 (PST) X-Google-Smtp-Source: AGHT+IGnK/5Xc9SKm0wQPEnUuYiqinVCHstqfWVnR5OuYxZ4xQLJmopCSPYZqNRQ2oNZWFAP27lsbg== X-Received: by 2002:a05:6a00:3d13:b0:6e4:672b:f384 with SMTP id lo19-20020a056a003d1300b006e4672bf384mr7669524pfb.2.1708508269578; Wed, 21 Feb 2024 01:37:49 -0800 (PST) Received: from x1n ([43.228.180.230]) by smtp.gmail.com with ESMTPSA id e11-20020aa7980b000000b006e4698d53casm4990624pfl.140.2024.02.21.01.37.43 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 21 Feb 2024 01:37:49 -0800 (PST) Date: Wed, 21 Feb 2024 17:37:37 +0800 From: Peter Xu To: Jason Gunthorpe Cc: linux-mm@kvack.org, linux-kernel@vger.kernel.org, James Houghton , David Hildenbrand , "Kirill A . Shutemov" , Yang Shi , linux-riscv@lists.infradead.org, Andrew Morton , "Aneesh Kumar K . V" , Rik van Riel , Andrea Arcangeli , Axel Rasmussen , Mike Rapoport , John Hubbard , Vlastimil Babka , Michael Ellerman , Christophe Leroy , Andrew Jones , linuxppc-dev@lists.ozlabs.org, Mike Kravetz , Muchun Song , linux-arm-kernel@lists.infradead.org, Christoph Hellwig , Lorenzo Stoakes , Matthew Wilcox Subject: Re: [PATCH v2 03/13] mm: Provide generic pmd_thp_or_huge() Message-ID: References: <20240103091423.400294-1-peterx@redhat.com> <20240103091423.400294-4-peterx@redhat.com> <20240115175551.GP734935@nvidia.com> MIME-Version: 1.0 In-Reply-To: <20240115175551.GP734935@nvidia.com> X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=utf-8 Content-Disposition: inline X-Rspam-User: X-Stat-Signature: su61rieyy1zmz1e3wcz1m17cfibz53oa X-Rspamd-Server: rspam07 X-Rspamd-Queue-Id: 4F5A216000F X-HE-Tag: 1708508273-933260 X-HE-Meta: U2FsdGVkX1+6pXeHyHApBv/FqU7FZmUR/uFp8Xcok9Neq4VCh6uBpxMx7XZcC6JbBy5xrW/bmN2OFNA8io50SkDZFlCHBB2c/eWy2QXvfyNbcG5dzuDY+2kZXU1p3Ag8k8LhNWCCigZJsdusRTv8AuYV56X9vOI/m/Nsjvgn26+o25NiTM5ArmlrFIbryIsmQJnoC1ye4PJu0LlTAatQ+zOvbPNaC2n7lAKlFyQ0+Up4p9PuJPA+ok6Z1+t3k8JTABN/o3IoYzC+etz2ezFqp04NCQMDToHo0OdA0RkGbuCaZYOArs1IbIq4upKChhdSEJ63uvm8300SHmA3LYy1bq3Ede7GcGE8/LxzIWh7f0g1XZ+/rScuOFWfopRzy/uEjp5ZReTc6lo1W+n0pnDtZXB+zcBsviH3RqRQyRNXGtwKxO0vYmtEy8/4I711IDBM2tLh6b3LC+MPNJJBRcs8+XUcdTBeRC4b7xAHVgi0c55jlQANowUQbYkdLi6WLsw8NhQ/UlYlud2wJvpVGlYA9dA4Cd89VzdiwQEqhTUNgj5K2uzqtu+1j4JJiUNbPaEyrzLd5t8UYA5OIxgqznF6+RKk8SZOEnaiU+8EZybUvjhnaLIIEE5SsJvRqlQ/QrdqAdPy+fMURnRzzkgQjuE5RBaIg+5vee2gOk1lvxHZ1P1V2g6GdTV2FsEiBibPGrt13eO7J2CFj2hs4wrMAhfcjmo2/yuOGb9y/XGO7aZC0ikGptzBKf4j+oMX8u3A/He71AYh702dCGwJaUt/0JPG8Z7UB+mgB5t+U6q7ZG0iBr7YvMlPalnjZSz6uL0xuLL6H383iyduZLw1tx37y7jbX1BZbmXEULtXirKC6zWCArqK/ZjFoyl2CTH4C6VLpqbQ5VL7+kypuHcGHfF7mMM+vXYxcqXDdSm+VWRoabk2Fxlvgt0uGeIll0002+0sSMw8+4bZaET3XoUdVcBcgh8 Lm176Qbj 7HZdDbYETLd22xNkitBeWSbLhZjpVwLtwEIKVLDcc2i0JxlYmS417bYuYI7u7g2JgFF/AfO29RP+iZThjilc6WgVSoiWwtvYCJCt2qBabu1G+GFPgjyVv6ZLmF19BDUCR2MQKmPLn58UPyJKAxlE+jat9Jx3+ut40JIKE6KnvMJj2dwheypwRb2568CHV7WIIicqe+2eKdEywgfZtAhjD/SY4fvTR9MQCC9soEwlKKlj4KhqPMLJ9CRmc4aRcLsBzFOkyw/uTzWQlzKHS3+X4U4enqvtWYywcXmGuGtz5pm6ce8oyKUYeQKjBSK2JHaQGZpMnIig04rW7Xhs1TueMw9VGRKHK7hIf8FKhpbWtakV7UUWmPSyVZk216Nw5Gffam+JVgAlB8rSjogc9tKD3AfSacbwJhp+h6tah+g9g4RcPWwOBQ1WSoxc9pQ== 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: List-Subscribe: List-Unsubscribe: On Mon, Jan 15, 2024 at 01:55:51PM -0400, Jason Gunthorpe wrote: > On Wed, Jan 03, 2024 at 05:14:13PM +0800, peterx@redhat.com wrote: > > From: Peter Xu > > > > ARM defines pmd_thp_or_huge(), detecting either a THP or a huge PMD. It > > can be a helpful helper if we want to merge more THP and hugetlb code > > paths. Make it a generic default implementation, only exist when > > CONFIG_MMU. Arch can overwrite it by defining its own version. > > > > For example, ARM's pgtable-2level.h defines it to always return false. > > > > Keep the macro declared with all config, it should be optimized to a false > > anyway if !THP && !HUGETLB. > > > > Signed-off-by: Peter Xu > > --- > > include/linux/pgtable.h | 4 ++++ > > mm/gup.c | 3 +-- > > 2 files changed, 5 insertions(+), 2 deletions(-) > > > > diff --git a/include/linux/pgtable.h b/include/linux/pgtable.h > > index 466cf477551a..2b42e95a4e3a 100644 > > --- a/include/linux/pgtable.h > > +++ b/include/linux/pgtable.h > > @@ -1362,6 +1362,10 @@ static inline int pmd_write(pmd_t pmd) > > #endif /* pmd_write */ > > #endif /* CONFIG_TRANSPARENT_HUGEPAGE */ > > > > +#ifndef pmd_thp_or_huge > > +#define pmd_thp_or_huge(pmd) (pmd_huge(pmd) || pmd_trans_huge(pmd)) > > +#endif > > Why not just use pmd_leaf() ? > > This GUP case seems to me exactly like what pmd_leaf() should really > do and be used for.. I think I mostly agree with you, and these APIs are indeed confusing. IMHO the challenge is about the risk of breaking others on small changes in the details where evil resides. > > eg x86 does: > > #define pmd_leaf pmd_large > static inline int pmd_large(pmd_t pte) > return pmd_flags(pte) & _PAGE_PSE; > > static inline int pmd_trans_huge(pmd_t pmd) > return (pmd_val(pmd) & (_PAGE_PSE|_PAGE_DEVMAP)) == _PAGE_PSE; > > int pmd_huge(pmd_t pmd) > return !pmd_none(pmd) && > (pmd_val(pmd) & (_PAGE_PRESENT|_PAGE_PSE)) != _PAGE_PRESENT; For example, here I don't think it's strictly pmd_leaf()? As pmd_huge() will return true if PRESENT=0 && PSE=0 (as long as none pte ruled out first), while pmd_leaf() will return false; I think that came from cbef8478bee5. I'm not sure whether that is the best solution, e.g., from a 1st glance it seems better to me to process swap entries separately (including both migration and poisoned entries).. Sparc has similar things there, which in that case I'm not sure whether a direct replace is always safe. Besides that, there're also other cases where it's not clear of such direct replacement, not until further investigated. E.g., arm-3level has: #define pmd_leaf(pmd) pmd_sect(pmd) #define pmd_sect(pmd) ((pmd_val(pmd) & PMD_TYPE_MASK) == \ PMD_TYPE_SECT) #define PMD_TYPE_SECT (_AT(pmdval_t, 1) << 0) While pmd_huge() there relies on PMD_TABLE_BIT () int pmd_huge(pmd_t pmd) { return pmd_val(pmd) && !(pmd_val(pmd) & PMD_TABLE_BIT); } #define PMD_TABLE_BIT (_AT(pmdval_t, 1) << 1) These are just the trivial details that I wanted to avoid to touch in this series, so as to resolve the hugetlb issue separately from others. The new pmd_huge_or_thp() is not ideal, but that easily isolates all these trivial details / evils out of the picture, so that we can tackle them one by one. It is strictly an OR or huge||thp, so it's hopefully safe to not break anything yet from that regard. > > I spot checked a couple arches and it looks like it holds up. > > Further, it looks to me like this site in GUP is the only core code > caller.. > > So, I'd suggest a small series to go arch by arch and convert the arch > to use pmd_huge() == pmd_leaf(). Then retire pmd_huge() as a public > API. > > > diff --git a/mm/gup.c b/mm/gup.c > > index df83182ec72d..eebae70d2465 100644 > > --- a/mm/gup.c > > +++ b/mm/gup.c > > @@ -3004,8 +3004,7 @@ static int gup_pmd_range(pud_t *pudp, pud_t pud, unsigned long addr, unsigned lo > > if (!pmd_present(pmd)) > > return 0; > > > > - if (unlikely(pmd_trans_huge(pmd) || pmd_huge(pmd) || > > - pmd_devmap(pmd))) { > > + if (unlikely(pmd_thp_or_huge(pmd) || pmd_devmap(pmd))) { > > /* See gup_pte_range() */ > > if (pmd_protnone(pmd)) > > return 0; > > And the devmap thing here doesn't make any sense either. The arch > should ensure that pmd_devmap() implies pmd_leaf(). Since devmap is a > purely SW construct it almost certainly does already anyhow. Yep, but only if pmd_leaf() is safe to be put here. A pmd devmap should always imply as a pmd_leaf() indeed. Thanks, -- Peter Xu