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 01C47C54E64 for ; Sat, 23 Mar 2024 00:46:16 +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=J1W0SghuTx9dOJsw5T+BH+zbu7BhGOEf1uIKlBMAAXU=; b=mPGZAhPmm5XOCJ 5AFI1lFYii/hNi0KAu/h4kxR11rvT8VQLXxwuTkppKgq89QBQSEVfEMjK1jxC4iSoS19U3/auvWja oHtxVttc6j9A8eGgXBm4vG+mt8mNxj7XQFw64sbvmQXoi/WprkN4qzz6Dd53488xjKNu3QKah1SoV /zq+sxDlw3E9Up1notlNO4j1oAT6vuQLVmmtVhJb5njmDBSbV5oeLwBa1nZ6xwys+xss2CeEOLPpG 1nHAQKaWd6kYW/hRS/q8ytcxwa0n+WUy1bbv3KjKInE0p4DeqoNJc1+N97gFEbaG0SsnsqK63+Xrs te9DEunp/oizLyR3IOgA==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.97.1 #2 (Red Hat Linux)) id 1rnpWV-0000000948g-3SuF; Sat, 23 Mar 2024 00:46:11 +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 1rnpWR-0000000946P-1EZl for linux-riscv@lists.infradead.org; Sat, 23 Mar 2024 00:46:09 +0000 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1711154766; 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=1OPeHdcChNLXNL4AZVVP+Dj6erLbPxRFcj0CZKU2hHc=; b=Suxl2RdSBOkopv3J73sHPjioyuGHRS4/2uhrnSDvYT8JbetMM5yZ5o7woN6co+uKZr9IJB ggX1f15JnW3e6RCJQQTjNRDSIHA4XsXfna9LL9jHhTKaj37yPCaoznSXlBGdpNoKZigdxJ e9gV/uG2iamfDlVJHMXQ4pyJZs1se5w= Received: from mail-qt1-f198.google.com (mail-qt1-f198.google.com [209.85.160.198]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-67-2lmrfUnbMrOTeth_yQag1A-1; Fri, 22 Mar 2024 20:46:05 -0400 X-MC-Unique: 2lmrfUnbMrOTeth_yQag1A-1 Received: by mail-qt1-f198.google.com with SMTP id d75a77b69052e-430b9ee6bfbso5526121cf.0 for ; Fri, 22 Mar 2024 17:46:05 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1711154763; x=1711759563; 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=1OPeHdcChNLXNL4AZVVP+Dj6erLbPxRFcj0CZKU2hHc=; b=HY0auQSeBWL4FHs7j0DTgaBXIy6VORybpdVerkb0FJkVWliBwJKvOi2qSgW8Pb63en lV7trxhUuAFAYP1xqdBUs2yxPmHzm1EcoO0Ou9lKSNx5PJbFM3kJ48XmXliHACVd/+ij 4A0ym9+UWy+ja2mXD32PQLv3Tq8bdtVlVxbSj0Lnr8gqip58MnhrsiGV0cLbZTUvsSvR rwQ8+Q5eZmmDeB5bAQP6zl07nWN4QXe90GyRen8s5kEZNuuKgPpxl7wE4FSB6aTOWQCo GZimhT3gXJj4pAtFIw7cNFZEbSEV6jIYPBHdyhz+LDULcBxzePWQzfNQZRgh/tC77Lpt e/JA== X-Forwarded-Encrypted: i=1; AJvYcCWMLQAwmqOmx78LTMafHtN2BAquFgcgZ4iMWJi+gYCaHAilwNmzDYE1YLIUlNFVi8UOD1of+B9ZyWW9SqO1hf8hXOosIC4gZlgMuqGC22iX X-Gm-Message-State: AOJu0Yzc1zNQHV2TuOaKwGZOwFyA3lPGr23xr+SHLv/QUFr31vENIt7c 4LSjiFb9gs+CXXjWe8ivsr5xBgYoqcva2Y/U7lxOb1hD44wUIr4cQsRtgdw8vwd1B99rhkbHa+u uTJ48DwmgajeZKUnFjPCTA+b5ftsAQ8048NOI2MTYI74/Z2g2BFV1VImiNPPyAF5CZA== X-Received: by 2002:a05:6214:3d8c:b0:696:1892:c19f with SMTP id om12-20020a0562143d8c00b006961892c19fmr1028536qvb.3.1711154762686; Fri, 22 Mar 2024 17:46:02 -0700 (PDT) X-Google-Smtp-Source: AGHT+IGxKOcFDVxpG3qeQleX58Tuq2r0TaBTmm4VWGOLqVds86GEmp/lVpAtwxST9mY/JIaYF94G+w== X-Received: by 2002:a05:6214:3d8c:b0:696:1892:c19f with SMTP id om12-20020a0562143d8c00b006961892c19fmr1028507qvb.3.1711154762082; Fri, 22 Mar 2024 17:46:02 -0700 (PDT) Received: from x1n ([99.254.121.117]) by smtp.gmail.com with ESMTPSA id jf8-20020a0562142a4800b006942dcac56asm1567334qvb.103.2024.03.22.17.46.00 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 22 Mar 2024 17:46:01 -0700 (PDT) Date: Fri, 22 Mar 2024 20:45:59 -0400 From: Peter Xu To: Andrew Morton Cc: linux-mm@kvack.org, linux-kernel@vger.kernel.org, linuxppc-dev@lists.ozlabs.org, Michael Ellerman , Christophe Leroy , Matthew Wilcox , Rik van Riel , Lorenzo Stoakes , Axel Rasmussen , Yang Shi , John Hubbard , linux-arm-kernel@lists.infradead.org, "Kirill A . Shutemov" , Andrew Jones , Vlastimil Babka , Mike Rapoport , Muchun Song , Christoph Hellwig , linux-riscv@lists.infradead.org, James Houghton , David Hildenbrand , Jason Gunthorpe , Andrea Arcangeli , "Aneesh Kumar K . V" , Mike Kravetz Subject: Re: [PATCH v3 12/12] mm/gup: Handle hugetlb in the generic follow_page_mask code Message-ID: References: <20240321220802.679544-1-peterx@redhat.com> <20240321220802.679544-13-peterx@redhat.com> <20240322134818.9b312f77629f79fcf1564b6f@linux-foundation.org> MIME-Version: 1.0 In-Reply-To: <20240322134818.9b312f77629f79fcf1564b6f@linux-foundation.org> 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-20240322_174607_506916_49612F33 X-CRM114-Status: GOOD ( 35.27 ) 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 Fri, Mar 22, 2024 at 01:48:18PM -0700, Andrew Morton wrote: > On Thu, 21 Mar 2024 18:08:02 -0400 peterx@redhat.com wrote: > > > From: Peter Xu > > > > Now follow_page() is ready to handle hugetlb pages in whatever form, and > > over all architectures. Switch to the generic code path. > > > > Time to retire hugetlb_follow_page_mask(), following the previous > > retirement of follow_hugetlb_page() in 4849807114b8. > > > > There may be a slight difference of how the loops run when processing slow > > GUP over a large hugetlb range on cont_pte/cont_pmd supported archs: each > > loop of __get_user_pages() will resolve one pgtable entry with the patch > > applied, rather than relying on the size of hugetlb hstate, the latter may > > cover multiple entries in one loop. > > > > A quick performance test on an aarch64 VM on M1 chip shows 15% degrade over > > a tight loop of slow gup after the path switched. That shouldn't be a > > problem because slow-gup should not be a hot path for GUP in general: when > > page is commonly present, fast-gup will already succeed, while when the > > page is indeed missing and require a follow up page fault, the slow gup > > degrade will probably buried in the fault paths anyway. It also explains > > why slow gup for THP used to be very slow before 57edfcfd3419 ("mm/gup: > > accelerate thp gup even for "pages != NULL"") lands, the latter not part of > > a performance analysis but a side benefit. If the performance will be a > > concern, we can consider handle CONT_PTE in follow_page(). > > > > Before that is justified to be necessary, keep everything clean and simple. > > > > mm/gup.c:33:21: warning: 'follow_hugepd' declared 'static' but never defined [-Wunused-function] > 33 | static struct page *follow_hugepd(struct vm_area_struct *vma, hugepd_t hugepd, > | ^~~~~~~~~~~~~ > > --- a/mm/gup.c~mm-gup-handle-hugepd-for-follow_page-fix > +++ a/mm/gup.c > @@ -30,10 +30,12 @@ struct follow_page_context { > unsigned int page_mask; > }; > > +#ifdef CONFIG_HAVE_FAST_GUP > static struct page *follow_hugepd(struct vm_area_struct *vma, hugepd_t hugepd, > unsigned long addr, unsigned int pdshift, > unsigned int flags, > struct follow_page_context *ctx); > +#endif > > static inline void sanity_check_pinned_pages(struct page **pages, > unsigned long npages) > _ > > > This looks inelegant. > > That's two build issues so far. Please be more expansive in the > Kconfig variations when testing. Especially when mucking with pgtable > macros. Andrew, Apologies for that, and also for a slightly late response. Yeah it's time I'll need my set of things to do serious build tests, and I'll at least start to cover a few error prone configs/archs in with that. I was trying to rely on the build bot in many of previous such cases, as that was quite useful to me to cover many build issues without investing my own test setups, but I think for some reason it retired and stopped working for a while. Maybe I shouldn't have relied on it at all. For this specific issue, I'm not sure if CONFIG_HAVE_FAST_GUP is proper? As follow_hugepd() is used in slow gup not fast. So maybe we can put that under CONFIG_MMU below that code (and I think we can drop "static" too as I don't think it's anything useful). My version of fixup attached at the end of email, and I verified it on m68k build. I do plan to post a small fixup series to fix these issues (so far it may contain 1 formal patch to touch up vmstat_item_print_in_thp, and 2 fixups where I'll mark the subject with "fixup!" properly). Either you can pick up below or you can wait for my small patchset, should be there either today or tomorrow. Thanks, ===8<=== diff --git a/mm/gup.c b/mm/gup.c index 4cd349390477..a2ed8203495a 100644 --- a/mm/gup.c +++ b/mm/gup.c @@ -30,11 +30,6 @@ struct follow_page_context { unsigned int page_mask; }; -static struct page *follow_hugepd(struct vm_area_struct *vma, hugepd_t hugepd, - unsigned long addr, unsigned int pdshift, - unsigned int flags, - struct follow_page_context *ctx); - static inline void sanity_check_pinned_pages(struct page **pages, unsigned long npages) { @@ -505,6 +500,12 @@ static inline void mm_set_has_pinned_flag(unsigned long *mm_flags) } #ifdef CONFIG_MMU + +struct page *follow_hugepd(struct vm_area_struct *vma, hugepd_t hugepd, + unsigned long addr, unsigned int pdshift, + unsigned int flags, + struct follow_page_context *ctx); + static struct page *no_page_table(struct vm_area_struct *vma, unsigned int flags, unsigned long address) { ===8<=== -- 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 52C41C47DD9 for ; Sat, 23 Mar 2024 00:46:59 +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=ac/1hIHU; 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=ac/1hIHU; dkim-atps=neutral Received: from boromir.ozlabs.org (localhost [IPv6:::1]) by lists.ozlabs.org (Postfix) with ESMTP id 4V1gWP621gz3vk8 for ; Sat, 23 Mar 2024 11:46:57 +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=ac/1hIHU; dkim=pass (1024-bit key) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=ac/1hIHU; dkim-atps=neutral Authentication-Results: lists.ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=redhat.com (client-ip=170.10.133.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.133.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 4V1gVX1WRXz3dTm for ; Sat, 23 Mar 2024 11:46:11 +1100 (AEDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1711154767; 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=1OPeHdcChNLXNL4AZVVP+Dj6erLbPxRFcj0CZKU2hHc=; b=ac/1hIHUi10/DvPszFnqX3YgmNqMGOPRLoWiI4hswIwakW5L0Wpf5Q7mrGuajVooPlWn+X WOkH9Jrxelp3OpfTGP2HemSb+i01nYlE+7Pqlj3zTJH0o/njaWf0JxMn+865wLImOLA6II KcGbg2W8xMXSR5N7NtbriUJfnWuwlUQ= DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1711154767; 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=1OPeHdcChNLXNL4AZVVP+Dj6erLbPxRFcj0CZKU2hHc=; b=ac/1hIHUi10/DvPszFnqX3YgmNqMGOPRLoWiI4hswIwakW5L0Wpf5Q7mrGuajVooPlWn+X WOkH9Jrxelp3OpfTGP2HemSb+i01nYlE+7Pqlj3zTJH0o/njaWf0JxMn+865wLImOLA6II KcGbg2W8xMXSR5N7NtbriUJfnWuwlUQ= Received: from mail-qt1-f200.google.com (mail-qt1-f200.google.com [209.85.160.200]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-52-EtwPa1QsNK-Q4nR5XNIvAA-1; Fri, 22 Mar 2024 20:46:04 -0400 X-MC-Unique: EtwPa1QsNK-Q4nR5XNIvAA-1 Received: by mail-qt1-f200.google.com with SMTP id d75a77b69052e-430b9ee6bfbso5525981cf.0 for ; Fri, 22 Mar 2024 17:46:03 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1711154763; x=1711759563; 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=1OPeHdcChNLXNL4AZVVP+Dj6erLbPxRFcj0CZKU2hHc=; b=sqrYgEtbIWsYQbf948zSgAcj6Y8dCAOE9hN9epyNcLXjJpOZmlccmmqvqN7aPrz2Wp nMD8Q/0spN3B/jrXDtedeNXFcNDDQScVNypiu0e+H/GaWXH0uUbrntfHqw6KQ8AlAcEx K3fIXwX9OTJJhwciYOp1MILQ5qOCf0SIgX4EO39g6Ka7O3PVdcHueOiGgr50myJJ7lft lztUru8PaOKON7hNAnw3/hv92sKweYGbGF1dZcoZQ+HqjJseQ7xrAKThN9bqSTtZNs4A 4HvuJG0H/qB5ifVHEjoiPRjFrNi0uXdQpp9IIJdaKuLeTkLt1sed/WT1klISBX6MG8lT ldmw== X-Forwarded-Encrypted: i=1; AJvYcCUbAq7z0DtV4RnIc9VPMOEPpKekOSTOcVtX/g6F7PoFbGDna47nUfdkGbwQx33aqblS7Gm/48Kq6Hq/PEazn9K3P6z9gvozPFhxM4hdbA== X-Gm-Message-State: AOJu0YymMpzB3MqLJI0HXcLeWgULdf1wvtU0yyNXnFb07lGRGWZtyK2Q uaUA5sZNqamo/vCtVcdQNjqIsj5TNZgSdeDeRTptzYGUvCPncUVlbnawoIH1TUL0pzvScg1oOzV fEznG/XYRVap1iYN4zvh0K52XJbYH6QpzQNPIM/MJleicMI+gSweX2hUstUxNWj0= X-Received: by 2002:a05:6214:3d8c:b0:696:1892:c19f with SMTP id om12-20020a0562143d8c00b006961892c19fmr1028545qvb.3.1711154762746; Fri, 22 Mar 2024 17:46:02 -0700 (PDT) X-Google-Smtp-Source: AGHT+IGxKOcFDVxpG3qeQleX58Tuq2r0TaBTmm4VWGOLqVds86GEmp/lVpAtwxST9mY/JIaYF94G+w== X-Received: by 2002:a05:6214:3d8c:b0:696:1892:c19f with SMTP id om12-20020a0562143d8c00b006961892c19fmr1028507qvb.3.1711154762082; Fri, 22 Mar 2024 17:46:02 -0700 (PDT) Received: from x1n ([99.254.121.117]) by smtp.gmail.com with ESMTPSA id jf8-20020a0562142a4800b006942dcac56asm1567334qvb.103.2024.03.22.17.46.00 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 22 Mar 2024 17:46:01 -0700 (PDT) Date: Fri, 22 Mar 2024 20:45:59 -0400 From: Peter Xu To: Andrew Morton Subject: Re: [PATCH v3 12/12] mm/gup: Handle hugetlb in the generic follow_page_mask code Message-ID: References: <20240321220802.679544-1-peterx@redhat.com> <20240321220802.679544-13-peterx@redhat.com> <20240322134818.9b312f77629f79fcf1564b6f@linux-foundation.org> MIME-Version: 1.0 In-Reply-To: <20240322134818.9b312f77629f79fcf1564b6f@linux-foundation.org> 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, linux-riscv@lists.infradead.org, Andrea Arcangeli , "Aneesh Kumar K . V" , Matthew Wilcox , Christoph Hellwig , linux-arm-kernel@lists.infradead.org, Jason Gunthorpe , Axel Rasmussen , Rik van Riel , John Hubbard , "Kirill A . Shutemov" , Vlastimil Babka , Lorenzo Stoakes , Muchun Song , linux-kernel@vger.kernel.org, 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 Fri, Mar 22, 2024 at 01:48:18PM -0700, Andrew Morton wrote: > On Thu, 21 Mar 2024 18:08:02 -0400 peterx@redhat.com wrote: > > > From: Peter Xu > > > > Now follow_page() is ready to handle hugetlb pages in whatever form, and > > over all architectures. Switch to the generic code path. > > > > Time to retire hugetlb_follow_page_mask(), following the previous > > retirement of follow_hugetlb_page() in 4849807114b8. > > > > There may be a slight difference of how the loops run when processing slow > > GUP over a large hugetlb range on cont_pte/cont_pmd supported archs: each > > loop of __get_user_pages() will resolve one pgtable entry with the patch > > applied, rather than relying on the size of hugetlb hstate, the latter may > > cover multiple entries in one loop. > > > > A quick performance test on an aarch64 VM on M1 chip shows 15% degrade over > > a tight loop of slow gup after the path switched. That shouldn't be a > > problem because slow-gup should not be a hot path for GUP in general: when > > page is commonly present, fast-gup will already succeed, while when the > > page is indeed missing and require a follow up page fault, the slow gup > > degrade will probably buried in the fault paths anyway. It also explains > > why slow gup for THP used to be very slow before 57edfcfd3419 ("mm/gup: > > accelerate thp gup even for "pages != NULL"") lands, the latter not part of > > a performance analysis but a side benefit. If the performance will be a > > concern, we can consider handle CONT_PTE in follow_page(). > > > > Before that is justified to be necessary, keep everything clean and simple. > > > > mm/gup.c:33:21: warning: 'follow_hugepd' declared 'static' but never defined [-Wunused-function] > 33 | static struct page *follow_hugepd(struct vm_area_struct *vma, hugepd_t hugepd, > | ^~~~~~~~~~~~~ > > --- a/mm/gup.c~mm-gup-handle-hugepd-for-follow_page-fix > +++ a/mm/gup.c > @@ -30,10 +30,12 @@ struct follow_page_context { > unsigned int page_mask; > }; > > +#ifdef CONFIG_HAVE_FAST_GUP > static struct page *follow_hugepd(struct vm_area_struct *vma, hugepd_t hugepd, > unsigned long addr, unsigned int pdshift, > unsigned int flags, > struct follow_page_context *ctx); > +#endif > > static inline void sanity_check_pinned_pages(struct page **pages, > unsigned long npages) > _ > > > This looks inelegant. > > That's two build issues so far. Please be more expansive in the > Kconfig variations when testing. Especially when mucking with pgtable > macros. Andrew, Apologies for that, and also for a slightly late response. Yeah it's time I'll need my set of things to do serious build tests, and I'll at least start to cover a few error prone configs/archs in with that. I was trying to rely on the build bot in many of previous such cases, as that was quite useful to me to cover many build issues without investing my own test setups, but I think for some reason it retired and stopped working for a while. Maybe I shouldn't have relied on it at all. For this specific issue, I'm not sure if CONFIG_HAVE_FAST_GUP is proper? As follow_hugepd() is used in slow gup not fast. So maybe we can put that under CONFIG_MMU below that code (and I think we can drop "static" too as I don't think it's anything useful). My version of fixup attached at the end of email, and I verified it on m68k build. I do plan to post a small fixup series to fix these issues (so far it may contain 1 formal patch to touch up vmstat_item_print_in_thp, and 2 fixups where I'll mark the subject with "fixup!" properly). Either you can pick up below or you can wait for my small patchset, should be there either today or tomorrow. Thanks, ===8<=== diff --git a/mm/gup.c b/mm/gup.c index 4cd349390477..a2ed8203495a 100644 --- a/mm/gup.c +++ b/mm/gup.c @@ -30,11 +30,6 @@ struct follow_page_context { unsigned int page_mask; }; -static struct page *follow_hugepd(struct vm_area_struct *vma, hugepd_t hugepd, - unsigned long addr, unsigned int pdshift, - unsigned int flags, - struct follow_page_context *ctx); - static inline void sanity_check_pinned_pages(struct page **pages, unsigned long npages) { @@ -505,6 +500,12 @@ static inline void mm_set_has_pinned_flag(unsigned long *mm_flags) } #ifdef CONFIG_MMU + +struct page *follow_hugepd(struct vm_area_struct *vma, hugepd_t hugepd, + unsigned long addr, unsigned int pdshift, + unsigned int flags, + struct follow_page_context *ctx); + static struct page *no_page_table(struct vm_area_struct *vma, unsigned int flags, unsigned long address) { ===8<=== -- 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 00E5CC54E64 for ; Sat, 23 Mar 2024 00:46:22 +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=eDAgaHN5zN7ELSlLJzeVfOPSdB9kQ5SSSSCRXoT1pjo=; b=4+NEIiclQkW4Wd NPKJczhaPiXSDpXPO0IzbSqXki4bhe7yT1nO/MWXfqWhjmWlWlNYf2EfABtUOwVt1iMDzUQr7FgLz y2oRyLCyl2CGNIFDbeY773EW/idrBeAkRpbyxEtdMnN9q2mjt/kaYyESFhvTQFjkgjam6QtRS3BU/ LpIg1amRoTO6GyxYqQCG+g9hgcXMdEHlrqhXuUQP4IGrtjha8G6mUR6RwNGRc174V3g4xHfRERSbQ iMPSBzz9yEltpkPKaPlV2vmdb+zT0RjJq2wJkk0aN/59MjIO7FOFFu0YC/bxMCLBkbDunzN/HGqKT epU2PBvr5sDWP5X33L4Q==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.97.1 #2 (Red Hat Linux)) id 1rnpWT-0000000947e-3BGb; Sat, 23 Mar 2024 00:46:09 +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 1rnpWR-0000000946M-05ar for linux-arm-kernel@lists.infradead.org; Sat, 23 Mar 2024 00:46:08 +0000 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1711154765; 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=1OPeHdcChNLXNL4AZVVP+Dj6erLbPxRFcj0CZKU2hHc=; b=iwDzaeR/UO/7AMXI9rKPWmWqXuXAp+Z0QXlW/nY4mz28I9fOUYtEAmWM7n8OqWxfVBIfNw lRr1OCBEE0ltwfkXw7eVWMdfN7flp92ZRJzPc41MqEx1KYABYGpBxZTG/jGTivxpllxaUI fHXnfMndr2xSuNDkEw6SayXdrAD9cUk= Received: from mail-qt1-f200.google.com (mail-qt1-f200.google.com [209.85.160.200]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-553-xvY3a9HfMhmxn-MwQGhx4Q-1; Fri, 22 Mar 2024 20:46:04 -0400 X-MC-Unique: xvY3a9HfMhmxn-MwQGhx4Q-1 Received: by mail-qt1-f200.google.com with SMTP id d75a77b69052e-43137863434so2173071cf.1 for ; Fri, 22 Mar 2024 17:46:04 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1711154763; x=1711759563; 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=1OPeHdcChNLXNL4AZVVP+Dj6erLbPxRFcj0CZKU2hHc=; b=E+zQfNt576GywOspQRJKpQSTSX1I8/wh4SfzZfBXt1dl2dX8f6ov0jg0yV5wBEzcwO gaAiVY14rlsDYLKg/+ozoAAEqkh3MdBRTMScJhHzV5DmGjhvOGQKqgHrvJvVK/Pq3Lbl nhP2lvtv4zCEfnLjnKnPiBTh82gJjbQ1n8zDyfLnq4hEACbiRcakCpkyfA4HXSKEcp2l SM0gyKhD3FO4Dh4M7rKa6d9HPM++mUjcdKeAVyzU+Zrjn1snkjgGK2F+u6cQbxt0w3+g ZF5UzftyHwthZuWDA7Kgxsk8ovXc+MD9o8yv7SAuH3faFTVAQCL9k2zioZ89xT5016iL bA/g== X-Forwarded-Encrypted: i=1; AJvYcCXqypv6orWN4of39rnI1iXEfiNr4CH+UePw5foHcWcB+anE9zxoGy9ApVX5z3vSaIu7kT+fALk8yTXb9EL8h5Ogpr3AqIsT65SSmtWR7kBYxH4fyc0= X-Gm-Message-State: AOJu0Yycqz9yAtjTagRw059sjTDZMBJbIksUfFW7W+fUgPghReOQgXIW wHeooMZ32HOsTl0eUwuzuNTVUJ2aYOv0kX7mQcLtEU6Yc/vl6jh8fH5ntQWjLpV5DC5LW2y9Pk1 IhxPjQcSPfaLra6cb/qw80vH3o/NJab5orfM+sHmoUMtk3+AV+eDwwRPpOoLDNV0mw8rA/lQt X-Received: by 2002:a05:6214:3d8c:b0:696:1892:c19f with SMTP id om12-20020a0562143d8c00b006961892c19fmr1028560qvb.3.1711154762752; Fri, 22 Mar 2024 17:46:02 -0700 (PDT) X-Google-Smtp-Source: AGHT+IGxKOcFDVxpG3qeQleX58Tuq2r0TaBTmm4VWGOLqVds86GEmp/lVpAtwxST9mY/JIaYF94G+w== X-Received: by 2002:a05:6214:3d8c:b0:696:1892:c19f with SMTP id om12-20020a0562143d8c00b006961892c19fmr1028507qvb.3.1711154762082; Fri, 22 Mar 2024 17:46:02 -0700 (PDT) Received: from x1n ([99.254.121.117]) by smtp.gmail.com with ESMTPSA id jf8-20020a0562142a4800b006942dcac56asm1567334qvb.103.2024.03.22.17.46.00 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 22 Mar 2024 17:46:01 -0700 (PDT) Date: Fri, 22 Mar 2024 20:45:59 -0400 From: Peter Xu To: Andrew Morton Cc: linux-mm@kvack.org, linux-kernel@vger.kernel.org, linuxppc-dev@lists.ozlabs.org, Michael Ellerman , Christophe Leroy , Matthew Wilcox , Rik van Riel , Lorenzo Stoakes , Axel Rasmussen , Yang Shi , John Hubbard , linux-arm-kernel@lists.infradead.org, "Kirill A . Shutemov" , Andrew Jones , Vlastimil Babka , Mike Rapoport , Muchun Song , Christoph Hellwig , linux-riscv@lists.infradead.org, James Houghton , David Hildenbrand , Jason Gunthorpe , Andrea Arcangeli , "Aneesh Kumar K . V" , Mike Kravetz Subject: Re: [PATCH v3 12/12] mm/gup: Handle hugetlb in the generic follow_page_mask code Message-ID: References: <20240321220802.679544-1-peterx@redhat.com> <20240321220802.679544-13-peterx@redhat.com> <20240322134818.9b312f77629f79fcf1564b6f@linux-foundation.org> MIME-Version: 1.0 In-Reply-To: <20240322134818.9b312f77629f79fcf1564b6f@linux-foundation.org> 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-20240322_174607_175629_00A3F5DD X-CRM114-Status: GOOD ( 36.90 ) 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 Fri, Mar 22, 2024 at 01:48:18PM -0700, Andrew Morton wrote: > On Thu, 21 Mar 2024 18:08:02 -0400 peterx@redhat.com wrote: > > > From: Peter Xu > > > > Now follow_page() is ready to handle hugetlb pages in whatever form, and > > over all architectures. Switch to the generic code path. > > > > Time to retire hugetlb_follow_page_mask(), following the previous > > retirement of follow_hugetlb_page() in 4849807114b8. > > > > There may be a slight difference of how the loops run when processing slow > > GUP over a large hugetlb range on cont_pte/cont_pmd supported archs: each > > loop of __get_user_pages() will resolve one pgtable entry with the patch > > applied, rather than relying on the size of hugetlb hstate, the latter may > > cover multiple entries in one loop. > > > > A quick performance test on an aarch64 VM on M1 chip shows 15% degrade over > > a tight loop of slow gup after the path switched. That shouldn't be a > > problem because slow-gup should not be a hot path for GUP in general: when > > page is commonly present, fast-gup will already succeed, while when the > > page is indeed missing and require a follow up page fault, the slow gup > > degrade will probably buried in the fault paths anyway. It also explains > > why slow gup for THP used to be very slow before 57edfcfd3419 ("mm/gup: > > accelerate thp gup even for "pages != NULL"") lands, the latter not part of > > a performance analysis but a side benefit. If the performance will be a > > concern, we can consider handle CONT_PTE in follow_page(). > > > > Before that is justified to be necessary, keep everything clean and simple. > > > > mm/gup.c:33:21: warning: 'follow_hugepd' declared 'static' but never defined [-Wunused-function] > 33 | static struct page *follow_hugepd(struct vm_area_struct *vma, hugepd_t hugepd, > | ^~~~~~~~~~~~~ > > --- a/mm/gup.c~mm-gup-handle-hugepd-for-follow_page-fix > +++ a/mm/gup.c > @@ -30,10 +30,12 @@ struct follow_page_context { > unsigned int page_mask; > }; > > +#ifdef CONFIG_HAVE_FAST_GUP > static struct page *follow_hugepd(struct vm_area_struct *vma, hugepd_t hugepd, > unsigned long addr, unsigned int pdshift, > unsigned int flags, > struct follow_page_context *ctx); > +#endif > > static inline void sanity_check_pinned_pages(struct page **pages, > unsigned long npages) > _ > > > This looks inelegant. > > That's two build issues so far. Please be more expansive in the > Kconfig variations when testing. Especially when mucking with pgtable > macros. Andrew, Apologies for that, and also for a slightly late response. Yeah it's time I'll need my set of things to do serious build tests, and I'll at least start to cover a few error prone configs/archs in with that. I was trying to rely on the build bot in many of previous such cases, as that was quite useful to me to cover many build issues without investing my own test setups, but I think for some reason it retired and stopped working for a while. Maybe I shouldn't have relied on it at all. For this specific issue, I'm not sure if CONFIG_HAVE_FAST_GUP is proper? As follow_hugepd() is used in slow gup not fast. So maybe we can put that under CONFIG_MMU below that code (and I think we can drop "static" too as I don't think it's anything useful). My version of fixup attached at the end of email, and I verified it on m68k build. I do plan to post a small fixup series to fix these issues (so far it may contain 1 formal patch to touch up vmstat_item_print_in_thp, and 2 fixups where I'll mark the subject with "fixup!" properly). Either you can pick up below or you can wait for my small patchset, should be there either today or tomorrow. Thanks, ===8<=== diff --git a/mm/gup.c b/mm/gup.c index 4cd349390477..a2ed8203495a 100644 --- a/mm/gup.c +++ b/mm/gup.c @@ -30,11 +30,6 @@ struct follow_page_context { unsigned int page_mask; }; -static struct page *follow_hugepd(struct vm_area_struct *vma, hugepd_t hugepd, - unsigned long addr, unsigned int pdshift, - unsigned int flags, - struct follow_page_context *ctx); - static inline void sanity_check_pinned_pages(struct page **pages, unsigned long npages) { @@ -505,6 +500,12 @@ static inline void mm_set_has_pinned_flag(unsigned long *mm_flags) } #ifdef CONFIG_MMU + +struct page *follow_hugepd(struct vm_area_struct *vma, hugepd_t hugepd, + unsigned long addr, unsigned int pdshift, + unsigned int flags, + struct follow_page_context *ctx); + static struct page *no_page_table(struct vm_area_struct *vma, unsigned int flags, unsigned long address) { ===8<=== -- 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 5196EC47DD9 for ; Sat, 23 Mar 2024 00:46:10 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id CECA66B0085; Fri, 22 Mar 2024 20:46:09 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id C76606B008A; Fri, 22 Mar 2024 20:46:09 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id AC8DC6B0092; Fri, 22 Mar 2024 20:46:09 -0400 (EDT) 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 963FB6B0085 for ; Fri, 22 Mar 2024 20:46:09 -0400 (EDT) Received: from smtpin01.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay08.hostedemail.com (Postfix) with ESMTP id 386C514087D for ; Sat, 23 Mar 2024 00:46:09 +0000 (UTC) X-FDA: 81926462058.01.19A249A Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by imf21.hostedemail.com (Postfix) with ESMTP id 1EE291C0016 for ; Sat, 23 Mar 2024 00:46:06 +0000 (UTC) Authentication-Results: imf21.hostedemail.com; dkim=pass header.d=redhat.com header.s=mimecast20190719 header.b=Suxl2RdS; dmarc=pass (policy=none) header.from=redhat.com; spf=pass (imf21.hostedemail.com: domain of peterx@redhat.com designates 170.10.133.124 as permitted sender) smtp.mailfrom=peterx@redhat.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1711154767; a=rsa-sha256; cv=none; b=4DnR09+TZtjEQ4FK6twneGrq23DJT69s++3uVXjIOwwaKOHpX8kZEk4RXIoageBZ+EPfFW 1kro5S92T66HzcvtqgmKQ31kx0u1RjdJ5LxFnq6QIo5noz3fds+Agy4L+IIIyVaNHJMWEx lGNSrhwelvDRgEdM+vvQba4+9h/6bGM= ARC-Authentication-Results: i=1; imf21.hostedemail.com; dkim=pass header.d=redhat.com header.s=mimecast20190719 header.b=Suxl2RdS; dmarc=pass (policy=none) header.from=redhat.com; spf=pass (imf21.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=1711154767; 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=1OPeHdcChNLXNL4AZVVP+Dj6erLbPxRFcj0CZKU2hHc=; b=J/T9hkUK5qiCchFQB/zQMfAUes5jpXIdTOXh2E/Dg9Pni7KVK51QVSD77dnzQjqHDYyY1S bG3dtrIkYk+cyD4JLCVRTp/X0srfvYXsQ8SpdoVfqazpbjtQnKZiYvOlddW+GR7ShL0j6d /MgbAnicfZpW7Mv283G2JKxe+Pq3774= DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1711154766; 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=1OPeHdcChNLXNL4AZVVP+Dj6erLbPxRFcj0CZKU2hHc=; b=Suxl2RdSBOkopv3J73sHPjioyuGHRS4/2uhrnSDvYT8JbetMM5yZ5o7woN6co+uKZr9IJB ggX1f15JnW3e6RCJQQTjNRDSIHA4XsXfna9LL9jHhTKaj37yPCaoznSXlBGdpNoKZigdxJ e9gV/uG2iamfDlVJHMXQ4pyJZs1se5w= Received: from mail-qk1-f197.google.com (mail-qk1-f197.google.com [209.85.222.197]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-650-jKShytGePXSIahKo8wpvhQ-1; Fri, 22 Mar 2024 20:46:05 -0400 X-MC-Unique: jKShytGePXSIahKo8wpvhQ-1 Received: by mail-qk1-f197.google.com with SMTP id af79cd13be357-789f8bb05e8so43850485a.0 for ; Fri, 22 Mar 2024 17:46:05 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1711154763; x=1711759563; 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=1OPeHdcChNLXNL4AZVVP+Dj6erLbPxRFcj0CZKU2hHc=; b=c0rFAKG0Z2VhGgtJ1d6cQ/uBD4S2cNCN7rOBVEq+Vhxw/ZQQQYtUABp8Co5llGbMPU ikeO58g1gNtPhaHLm5eypFLBk9ldKWZiMb6/DNTV0qUPMavixYi2kmUwho8vlH1GWqTF dpuBWxum/00VY7yjx+y5Lf4ZG3/Rtc0gqkTjyoOneMBoJ9v2Hg+PiR0oPXlYNpnYpPxR 5Tw6dIvYwmeJFI2faN9mGWucmf3PVXUwVrfOTo7ACIpHVeptYJlVI16H2S/uh3oOd9nw FuDyjT64Ina2G7vXvgQRTNIENpdTzUytB5X6xpR3khnt8bzXcGQY5UitqKllawBFE+49 +0yQ== X-Gm-Message-State: AOJu0YyF0tJjdeYQJmnYR1c2yPgvom/NDw9V4vekH8UuKSqiiPWX9nW5 l30mYPbkgJybj6Ecv5787BSD1Z0hIcxvwVt3AkoknvgW36lVlwn5yrw3soti/Dz5+KmyDJCWSkU n559Trt7W2S7js/anWq76+MfDiims/vfNeD1qqAmZiBoeJbpL X-Received: by 2002:a05:6214:3d8c:b0:696:1892:c19f with SMTP id om12-20020a0562143d8c00b006961892c19fmr1028556qvb.3.1711154762749; Fri, 22 Mar 2024 17:46:02 -0700 (PDT) X-Google-Smtp-Source: AGHT+IGxKOcFDVxpG3qeQleX58Tuq2r0TaBTmm4VWGOLqVds86GEmp/lVpAtwxST9mY/JIaYF94G+w== X-Received: by 2002:a05:6214:3d8c:b0:696:1892:c19f with SMTP id om12-20020a0562143d8c00b006961892c19fmr1028507qvb.3.1711154762082; Fri, 22 Mar 2024 17:46:02 -0700 (PDT) Received: from x1n ([99.254.121.117]) by smtp.gmail.com with ESMTPSA id jf8-20020a0562142a4800b006942dcac56asm1567334qvb.103.2024.03.22.17.46.00 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 22 Mar 2024 17:46:01 -0700 (PDT) Date: Fri, 22 Mar 2024 20:45:59 -0400 From: Peter Xu To: Andrew Morton Cc: linux-mm@kvack.org, linux-kernel@vger.kernel.org, linuxppc-dev@lists.ozlabs.org, Michael Ellerman , Christophe Leroy , Matthew Wilcox , Rik van Riel , Lorenzo Stoakes , Axel Rasmussen , Yang Shi , John Hubbard , linux-arm-kernel@lists.infradead.org, "Kirill A . Shutemov" , Andrew Jones , Vlastimil Babka , Mike Rapoport , Muchun Song , Christoph Hellwig , linux-riscv@lists.infradead.org, James Houghton , David Hildenbrand , Jason Gunthorpe , Andrea Arcangeli , "Aneesh Kumar K . V" , Mike Kravetz Subject: Re: [PATCH v3 12/12] mm/gup: Handle hugetlb in the generic follow_page_mask code Message-ID: References: <20240321220802.679544-1-peterx@redhat.com> <20240321220802.679544-13-peterx@redhat.com> <20240322134818.9b312f77629f79fcf1564b6f@linux-foundation.org> MIME-Version: 1.0 In-Reply-To: <20240322134818.9b312f77629f79fcf1564b6f@linux-foundation.org> X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=utf-8 Content-Disposition: inline X-Rspam-User: X-Rspamd-Server: rspam06 X-Rspamd-Queue-Id: 1EE291C0016 X-Stat-Signature: frqzhzhwpez1x15681apwkxafz4msdwe X-HE-Tag: 1711154766-701758 X-HE-Meta: U2FsdGVkX19Fc/cUiBPCoprj6xxs933A3YP+ObHn9+V66kRyu6uTNh0XJaY6jY6kfZ6cX8ViWviWDwFJyqU9k+x1waQbhKxZcHWNVpjyTzgajYEfwWJo31XLM5NSava92ntABVrfMajDmwthLbaDL/C2Ne/mochB2Ws9J1pb8NhJvhYdN5hjVuZenuvbcdIKEbVktbzlD9pEXcXWEpBS5k2aQCjyoJR3DNryNDg6zkR3BgqMgGHbFz2Lq0fIzqniWLPeZ66He6fxCSqXBgwtZ+i3fDnLaT3DtdeJs+uqEPCgHPsb45b/kqcJbrvoj2U/UTkvnNO3E3DVJFOyzxW+AFBskkIcdgAdA+hV3g+29YA6lstY13Jlr5QQ0ulZj8EegkYCBZP0EaTURxewG/+T0tAncGaYSJH3YNZ/TTJZRLq4WYykmExKATD+JuSUl3Hq4Vf/QHoqzYbYOauLa0R63FdUcxD88r4mDe22fsTuf2J2dMePM0pp75SmVmKaG2OaizMUjMp7WtEubyUEkqu8p2Wb/gP3qPlDnCj0BqcTSR1mjpjZSKUXSq5MXmHNcH4esfKzd8+k6caGg6puA9/lQGlY0HCLF3GQGChd9Ht69SiaR2Ux981B8/k4eJRc8v5lCCtHCHSEtzkgQZ4a7SbaYb3h4WePrvYe7BxOZhfJynPFMp7xiaom4JIXEB6QsKl9UALpossVBEIOztYlj0PIBCLgTJTUBdjlEkY2CTIFBg0D8Nap23HZ0wt813jDc2wh4Cp+1TgRRceTzrP+jNSP3QHDtWLyYtEft2En2T9qJFDYgNDdYoWtrZQp0x10s+oItVEswCZeKus33PEvITZkOWvWDxizfnnnTgxZXVuADtEd/kdzG/5r6xfN4kC63GhINhmuDh94HQqzsIdTWOt8YRi1kp2c6UGEiWlKzcIlnhwlRSI3iLi6Ca/uhdYKB4Bp3uw+W0jAYo5qZvCSv2q POAp5lfG qVrPY5hJQ8eoah4i0CCPM42iCnaEWcLzG3fI732ZOOxy5sDesFAkwDeqw3sbpgwkvevDm5Df9e//V6vjInmmUMqlXPBVGRkB42lNz7zONwWb6l4UaluU1KfqMmNy9HIuEfNcJuz+YPrmfK+Vp09JZeX7FMdV6HRU6D5IfzPSHsyvfqWU8ixFYhBQaaMyqHDUydmstA06OcpEuN2AXiXOrKsyRLRu2n2dS485QIg9S8d9IYrUCUw8JoEhglWzyhHGQY5Q4M3lYGbZN9Gu/ZqhJ2/sp78HaY4ar7Cy3elC64lsow3yhLe5u+H7UoVSAyvfAJD+rUh51r9CNMya3TPRICXqtCk0p89DdE6hlT8NKii6SFBw1gysctym+Sz5mfB9rhoiitvcexu0PrRNRlk5jW1xkUZfLDvcwUYCCrMtLga5NsCbiKSsEuF5uGQ== 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 Fri, Mar 22, 2024 at 01:48:18PM -0700, Andrew Morton wrote: > On Thu, 21 Mar 2024 18:08:02 -0400 peterx@redhat.com wrote: > > > From: Peter Xu > > > > Now follow_page() is ready to handle hugetlb pages in whatever form, and > > over all architectures. Switch to the generic code path. > > > > Time to retire hugetlb_follow_page_mask(), following the previous > > retirement of follow_hugetlb_page() in 4849807114b8. > > > > There may be a slight difference of how the loops run when processing slow > > GUP over a large hugetlb range on cont_pte/cont_pmd supported archs: each > > loop of __get_user_pages() will resolve one pgtable entry with the patch > > applied, rather than relying on the size of hugetlb hstate, the latter may > > cover multiple entries in one loop. > > > > A quick performance test on an aarch64 VM on M1 chip shows 15% degrade over > > a tight loop of slow gup after the path switched. That shouldn't be a > > problem because slow-gup should not be a hot path for GUP in general: when > > page is commonly present, fast-gup will already succeed, while when the > > page is indeed missing and require a follow up page fault, the slow gup > > degrade will probably buried in the fault paths anyway. It also explains > > why slow gup for THP used to be very slow before 57edfcfd3419 ("mm/gup: > > accelerate thp gup even for "pages != NULL"") lands, the latter not part of > > a performance analysis but a side benefit. If the performance will be a > > concern, we can consider handle CONT_PTE in follow_page(). > > > > Before that is justified to be necessary, keep everything clean and simple. > > > > mm/gup.c:33:21: warning: 'follow_hugepd' declared 'static' but never defined [-Wunused-function] > 33 | static struct page *follow_hugepd(struct vm_area_struct *vma, hugepd_t hugepd, > | ^~~~~~~~~~~~~ > > --- a/mm/gup.c~mm-gup-handle-hugepd-for-follow_page-fix > +++ a/mm/gup.c > @@ -30,10 +30,12 @@ struct follow_page_context { > unsigned int page_mask; > }; > > +#ifdef CONFIG_HAVE_FAST_GUP > static struct page *follow_hugepd(struct vm_area_struct *vma, hugepd_t hugepd, > unsigned long addr, unsigned int pdshift, > unsigned int flags, > struct follow_page_context *ctx); > +#endif > > static inline void sanity_check_pinned_pages(struct page **pages, > unsigned long npages) > _ > > > This looks inelegant. > > That's two build issues so far. Please be more expansive in the > Kconfig variations when testing. Especially when mucking with pgtable > macros. Andrew, Apologies for that, and also for a slightly late response. Yeah it's time I'll need my set of things to do serious build tests, and I'll at least start to cover a few error prone configs/archs in with that. I was trying to rely on the build bot in many of previous such cases, as that was quite useful to me to cover many build issues without investing my own test setups, but I think for some reason it retired and stopped working for a while. Maybe I shouldn't have relied on it at all. For this specific issue, I'm not sure if CONFIG_HAVE_FAST_GUP is proper? As follow_hugepd() is used in slow gup not fast. So maybe we can put that under CONFIG_MMU below that code (and I think we can drop "static" too as I don't think it's anything useful). My version of fixup attached at the end of email, and I verified it on m68k build. I do plan to post a small fixup series to fix these issues (so far it may contain 1 formal patch to touch up vmstat_item_print_in_thp, and 2 fixups where I'll mark the subject with "fixup!" properly). Either you can pick up below or you can wait for my small patchset, should be there either today or tomorrow. Thanks, ===8<=== diff --git a/mm/gup.c b/mm/gup.c index 4cd349390477..a2ed8203495a 100644 --- a/mm/gup.c +++ b/mm/gup.c @@ -30,11 +30,6 @@ struct follow_page_context { unsigned int page_mask; }; -static struct page *follow_hugepd(struct vm_area_struct *vma, hugepd_t hugepd, - unsigned long addr, unsigned int pdshift, - unsigned int flags, - struct follow_page_context *ctx); - static inline void sanity_check_pinned_pages(struct page **pages, unsigned long npages) { @@ -505,6 +500,12 @@ static inline void mm_set_has_pinned_flag(unsigned long *mm_flags) } #ifdef CONFIG_MMU + +struct page *follow_hugepd(struct vm_area_struct *vma, hugepd_t hugepd, + unsigned long addr, unsigned int pdshift, + unsigned int flags, + struct follow_page_context *ctx); + static struct page *no_page_table(struct vm_area_struct *vma, unsigned int flags, unsigned long address) { ===8<=== -- Peter Xu