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 X-Spam-Level: X-Spam-Status: No, score=-12.7 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,FREEMAIL_FORGED_FROMDOMAIN,FREEMAIL_FROM, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 4E447C636C8 for ; Tue, 20 Jul 2021 22:22:29 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 3A63761209 for ; Tue, 20 Jul 2021 22:22:29 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S231545AbhGTVlt (ORCPT ); Tue, 20 Jul 2021 17:41:49 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:36428 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229909AbhGTVkz (ORCPT ); Tue, 20 Jul 2021 17:40:55 -0400 Received: from mail-ed1-x52e.google.com (mail-ed1-x52e.google.com [IPv6:2a00:1450:4864:20::52e]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 59A63C061574 for ; Tue, 20 Jul 2021 15:21:30 -0700 (PDT) Received: by mail-ed1-x52e.google.com with SMTP id h8so30376462eds.4 for ; Tue, 20 Jul 2021 15:21:30 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=IUTmtTejbEnPRbu3cg3+KcdqGXEVg+RQFqP1834V1W8=; b=oTaTN1zAc2XovdqoMGL9tGPcZxHy+uTyPFvZma1Xs2GZBOZsYqBEBQX21g9dmGdzYJ 5V2f+jPLPfwIuauC0Qyvqb8lfZTU2JUDubJmRCzWV6xNUhf/qBKAOVTLXpuWrEmlQgs8 qA/6JMjdoQ9aqObLG+2AAmuXy1xhHNINHCY1ToEfkjImp4N5LmubaxPOpi/9pPoLtuLx ubBTH2U6QSbqTSSiLFkuI+OK4BwoxL9bgCktUMLxvVlHs7wnC1Fg7fvLaQreBfkaa0KJ ovyE5wRzxqUlb+usX2FuzlKvEZienUmydtXG57KmiRjLdF5P3qzitQmTRHFojUJnt0yS +Ftg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=IUTmtTejbEnPRbu3cg3+KcdqGXEVg+RQFqP1834V1W8=; b=XQAq4Ox8hOmeKB/LStPcqAntYP+CNh80fQQSE5Iu9VTN/PZZ5lhheVoBYP02bamGPA mQ4Va/nFvJOZS/PNM4GZeliSUqwQkz7lQxvsgPmU87T5OoxFUWQo3HonG20kRawc2bwz pIyxFjqAbNedYqdeAl7qkzfJ5zD7DjpF9yX6jqenHW+8i2yY2D+/UncS5g3mgMfdBUam sETLgLcmO2OWpANjy4X4H1h7QAt8PHUnIsIpiA/GMIznEyHxuXphQ4bEFvW/iwJ9WQzQ IY0P18Gdyj1rFdB6GWuf5hCLhGKlSswtwycgmV6E/EInsqYdq8TvJVWkAiKO+i6qn8j0 QP7A== X-Gm-Message-State: AOAM5315dIkQbBm6Lku7eFIwJsH2dycXOHB3haKHEV/DfhI8YKvd6S4Y 1SMIstc2TwqFPZKucmC0VexZ1TXknLk77KqRJNA= X-Google-Smtp-Source: ABdhPJwdwBIjPIhxMElcrJDHN7H8Naz43IOlQjuV3+TkDHVJUoYB06Oe3nclcfDJpwuqlDUnygbmwdorai+cH5PkELE= X-Received: by 2002:aa7:ca54:: with SMTP id j20mr44196757edt.137.1626819688962; Tue, 20 Jul 2021 15:21:28 -0700 (PDT) MIME-Version: 1.0 References: <20210720065529.716031-1-ying.huang@intel.com> In-Reply-To: From: Yang Shi Date: Tue, 20 Jul 2021 15:21:06 -0700 Message-ID: Subject: Re: [PATCH] mm,do_huge_pmd_numa_page: remove unnecessary TLB flushing code To: Huang Ying Cc: Andrew Morton , Linux MM , Linux Kernel Mailing List , Dan Carpenter , Mel Gorman , Christian Borntraeger , Gerald Schaefer , Heiko Carstens , Hugh Dickins , Andrea Arcangeli , "Kirill A . Shutemov" , Michal Hocko , Vasily Gorbik , Zi Yan Content-Type: text/plain; charset="UTF-8" Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue, Jul 20, 2021 at 1:48 PM Yang Shi wrote: > > On Mon, Jul 19, 2021 at 11:56 PM Huang Ying wrote: > > > > Before the commit c5b5a3dd2c1f ("mm: thp: refactor NUMA fault > > handling"), the TLB flushing is done in do_huge_pmd_numa_page() itself > > via flush_tlb_range(). > > > > But after commit c5b5a3dd2c1f ("mm: thp: refactor NUMA fault > > handling"), the TLB flushing is done in migrate_pages() as in the > > following code path anyway. > > > > do_huge_pmd_numa_page > > migrate_misplaced_page > > migrate_pages > > > > So now, the TLB flushing code in do_huge_pmd_numa_page() becomes > > unnecessary. So the code is deleted in this patch to simplify the > > code. This is only code cleanup, there's no visible performance > > difference. > > Yes, there is tlb flush in try_to_migrate(), but it seems mmu notifier > invalidate is missed for the THP migration case. I'm not quite sure > why it is not needed, maybe just missed? > > So, you may need the below change too: > > diff --git a/mm/rmap.c b/mm/rmap.c > index 2d29a57d29e8..e1c8b654563d 100644 > --- a/mm/rmap.c > +++ b/mm/rmap.c > @@ -1749,6 +1749,8 @@ static bool try_to_migrate_one(struct page > *page, struct vm_area_struct *vma, > !PageTransCompound(page), page); > > set_pmd_migration_entry(&pvmw, page); > + mmu_notifier_invalidate_range(mm, range.start, > + range.end); > continue; > } > #endif Per the discussion with Zi Yan the mmu notifier invalidate should be not needed. The patch looks good to me. Reviewed-by: Yang Shi > > > > > Signed-off-by: "Huang, Ying" > > Cc: Yang Shi > > Cc: Dan Carpenter > > Cc: Mel Gorman > > Cc: Christian Borntraeger > > Cc: Gerald Schaefer > > Cc: Heiko Carstens > > Cc: Hugh Dickins > > Cc: Andrea Arcangeli > > Cc: Kirill A. Shutemov > > Cc: Michal Hocko > > Cc: Vasily Gorbik > > Cc: Zi Yan > > --- > > mm/huge_memory.c | 26 -------------------------- > > 1 file changed, 26 deletions(-) > > > > diff --git a/mm/huge_memory.c b/mm/huge_memory.c > > index afff3ac87067..9f21e44c9030 100644 > > --- a/mm/huge_memory.c > > +++ b/mm/huge_memory.c > > @@ -1440,32 +1440,6 @@ vm_fault_t do_huge_pmd_numa_page(struct vm_fault *vmf) > > goto out; > > } > > > > - /* > > - * Since we took the NUMA fault, we must have observed the !accessible > > - * bit. Make sure all other CPUs agree with that, to avoid them > > - * modifying the page we're about to migrate. > > - * > > - * Must be done under PTL such that we'll observe the relevant > > - * inc_tlb_flush_pending(). > > - * > > - * We are not sure a pending tlb flush here is for a huge page > > - * mapping or not. Hence use the tlb range variant > > - */ > > - if (mm_tlb_flush_pending(vma->vm_mm)) { > > - flush_tlb_range(vma, haddr, haddr + HPAGE_PMD_SIZE); > > - /* > > - * change_huge_pmd() released the pmd lock before > > - * invalidating the secondary MMUs sharing the primary > > - * MMU pagetables (with ->invalidate_range()). The > > - * mmu_notifier_invalidate_range_end() (which > > - * internally calls ->invalidate_range()) in > > - * change_pmd_range() will run after us, so we can't > > - * rely on it here and we need an explicit invalidate. > > - */ > > - mmu_notifier_invalidate_range(vma->vm_mm, haddr, > > - haddr + HPAGE_PMD_SIZE); > > - } > > - > > pmd = pmd_modify(oldpmd, vma->vm_page_prot); > > page = vm_normal_page_pmd(vma, haddr, pmd); > > if (!page) > > -- > > 2.30.2 > > 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 X-Spam-Level: X-Spam-Status: No, score=-12.7 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,FREEMAIL_FORGED_FROMDOMAIN,FREEMAIL_FROM, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id DBC2DC07E9B for ; Tue, 20 Jul 2021 22:21:31 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id 7274761209 for ; Tue, 20 Jul 2021 22:21:31 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 7274761209 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=gmail.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=owner-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix) id 045DB6B006C; Tue, 20 Jul 2021 18:21:32 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id F37986B0070; Tue, 20 Jul 2021 18:21:31 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id DB1296B0071; Tue, 20 Jul 2021 18:21:31 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0116.hostedemail.com [216.40.44.116]) by kanga.kvack.org (Postfix) with ESMTP id B58FE6B006C for ; Tue, 20 Jul 2021 18:21:31 -0400 (EDT) Received: from smtpin15.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay02.hostedemail.com (Postfix) with ESMTP id 33B7420BFC for ; Tue, 20 Jul 2021 22:21:30 +0000 (UTC) X-FDA: 78384388740.15.61118F4 Received: from mail-ed1-f48.google.com (mail-ed1-f48.google.com [209.85.208.48]) by imf24.hostedemail.com (Postfix) with ESMTP id E7885B00082E for ; Tue, 20 Jul 2021 22:21:29 +0000 (UTC) Received: by mail-ed1-f48.google.com with SMTP id ca14so30382669edb.2 for ; Tue, 20 Jul 2021 15:21:29 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=IUTmtTejbEnPRbu3cg3+KcdqGXEVg+RQFqP1834V1W8=; b=oTaTN1zAc2XovdqoMGL9tGPcZxHy+uTyPFvZma1Xs2GZBOZsYqBEBQX21g9dmGdzYJ 5V2f+jPLPfwIuauC0Qyvqb8lfZTU2JUDubJmRCzWV6xNUhf/qBKAOVTLXpuWrEmlQgs8 qA/6JMjdoQ9aqObLG+2AAmuXy1xhHNINHCY1ToEfkjImp4N5LmubaxPOpi/9pPoLtuLx ubBTH2U6QSbqTSSiLFkuI+OK4BwoxL9bgCktUMLxvVlHs7wnC1Fg7fvLaQreBfkaa0KJ ovyE5wRzxqUlb+usX2FuzlKvEZienUmydtXG57KmiRjLdF5P3qzitQmTRHFojUJnt0yS +Ftg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=IUTmtTejbEnPRbu3cg3+KcdqGXEVg+RQFqP1834V1W8=; b=pGYRv4VkmUJYAfUJzOMaKUI1JaneuwvLww/2Wa624AnFs94/unWX0bigx/KJHnlZ6m rTPnDvN20Z77kzInl3+XCGw/6ovRRRUTtAZvYi9Jn+Gv3nVh+CcYoBkv9yEk/IVqk3Jh cPzlp14TNmI5dOwagvXEfQk4ZYSCMDqGWJoTMPMl6JKzw2JNpTz0wKoB3XquEoNAuJ6a +8pxahkncTOCcn7mXZ0QMNvB92d3G2rLlJtBo0njqBby3/DoY4rmfOkDd1S27Cw7sEIA i8pyXWb7gwTv5CCk/nit6zgQHeenc9BRITkVXoOWrkKRO3RHCaHJ5w0e7mqQBpfokIhP MXug== X-Gm-Message-State: AOAM530cCDF2fZMcOILb+6wXueBx4bGCS79J8PqQqv/ylIZrJQH3Bq2z 4mdmuh8Q5Yp2petiPxuPnrBXhfT7kve/Dlm4yKs= X-Google-Smtp-Source: ABdhPJwdwBIjPIhxMElcrJDHN7H8Naz43IOlQjuV3+TkDHVJUoYB06Oe3nclcfDJpwuqlDUnygbmwdorai+cH5PkELE= X-Received: by 2002:aa7:ca54:: with SMTP id j20mr44196757edt.137.1626819688962; Tue, 20 Jul 2021 15:21:28 -0700 (PDT) MIME-Version: 1.0 References: <20210720065529.716031-1-ying.huang@intel.com> In-Reply-To: From: Yang Shi Date: Tue, 20 Jul 2021 15:21:06 -0700 Message-ID: Subject: Re: [PATCH] mm,do_huge_pmd_numa_page: remove unnecessary TLB flushing code To: Huang Ying Cc: Andrew Morton , Linux MM , Linux Kernel Mailing List , Dan Carpenter , Mel Gorman , Christian Borntraeger , Gerald Schaefer , Heiko Carstens , Hugh Dickins , Andrea Arcangeli , "Kirill A . Shutemov" , Michal Hocko , Vasily Gorbik , Zi Yan Content-Type: text/plain; charset="UTF-8" Authentication-Results: imf24.hostedemail.com; dkim=pass header.d=gmail.com header.s=20161025 header.b=oTaTN1zA; spf=pass (imf24.hostedemail.com: domain of shy828301@gmail.com designates 209.85.208.48 as permitted sender) smtp.mailfrom=shy828301@gmail.com; dmarc=pass (policy=none) header.from=gmail.com X-Stat-Signature: u7mchpswnid8wfbqaqb7u8qdwoxmc89n X-Rspamd-Queue-Id: E7885B00082E X-Rspamd-Server: rspam01 X-HE-Tag: 1626819689-836608 X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: On Tue, Jul 20, 2021 at 1:48 PM Yang Shi wrote: > > On Mon, Jul 19, 2021 at 11:56 PM Huang Ying wrote: > > > > Before the commit c5b5a3dd2c1f ("mm: thp: refactor NUMA fault > > handling"), the TLB flushing is done in do_huge_pmd_numa_page() itself > > via flush_tlb_range(). > > > > But after commit c5b5a3dd2c1f ("mm: thp: refactor NUMA fault > > handling"), the TLB flushing is done in migrate_pages() as in the > > following code path anyway. > > > > do_huge_pmd_numa_page > > migrate_misplaced_page > > migrate_pages > > > > So now, the TLB flushing code in do_huge_pmd_numa_page() becomes > > unnecessary. So the code is deleted in this patch to simplify the > > code. This is only code cleanup, there's no visible performance > > difference. > > Yes, there is tlb flush in try_to_migrate(), but it seems mmu notifier > invalidate is missed for the THP migration case. I'm not quite sure > why it is not needed, maybe just missed? > > So, you may need the below change too: > > diff --git a/mm/rmap.c b/mm/rmap.c > index 2d29a57d29e8..e1c8b654563d 100644 > --- a/mm/rmap.c > +++ b/mm/rmap.c > @@ -1749,6 +1749,8 @@ static bool try_to_migrate_one(struct page > *page, struct vm_area_struct *vma, > !PageTransCompound(page), page); > > set_pmd_migration_entry(&pvmw, page); > + mmu_notifier_invalidate_range(mm, range.start, > + range.end); > continue; > } > #endif Per the discussion with Zi Yan the mmu notifier invalidate should be not needed. The patch looks good to me. Reviewed-by: Yang Shi > > > > > Signed-off-by: "Huang, Ying" > > Cc: Yang Shi > > Cc: Dan Carpenter > > Cc: Mel Gorman > > Cc: Christian Borntraeger > > Cc: Gerald Schaefer > > Cc: Heiko Carstens > > Cc: Hugh Dickins > > Cc: Andrea Arcangeli > > Cc: Kirill A. Shutemov > > Cc: Michal Hocko > > Cc: Vasily Gorbik > > Cc: Zi Yan > > --- > > mm/huge_memory.c | 26 -------------------------- > > 1 file changed, 26 deletions(-) > > > > diff --git a/mm/huge_memory.c b/mm/huge_memory.c > > index afff3ac87067..9f21e44c9030 100644 > > --- a/mm/huge_memory.c > > +++ b/mm/huge_memory.c > > @@ -1440,32 +1440,6 @@ vm_fault_t do_huge_pmd_numa_page(struct vm_fault *vmf) > > goto out; > > } > > > > - /* > > - * Since we took the NUMA fault, we must have observed the !accessible > > - * bit. Make sure all other CPUs agree with that, to avoid them > > - * modifying the page we're about to migrate. > > - * > > - * Must be done under PTL such that we'll observe the relevant > > - * inc_tlb_flush_pending(). > > - * > > - * We are not sure a pending tlb flush here is for a huge page > > - * mapping or not. Hence use the tlb range variant > > - */ > > - if (mm_tlb_flush_pending(vma->vm_mm)) { > > - flush_tlb_range(vma, haddr, haddr + HPAGE_PMD_SIZE); > > - /* > > - * change_huge_pmd() released the pmd lock before > > - * invalidating the secondary MMUs sharing the primary > > - * MMU pagetables (with ->invalidate_range()). The > > - * mmu_notifier_invalidate_range_end() (which > > - * internally calls ->invalidate_range()) in > > - * change_pmd_range() will run after us, so we can't > > - * rely on it here and we need an explicit invalidate. > > - */ > > - mmu_notifier_invalidate_range(vma->vm_mm, haddr, > > - haddr + HPAGE_PMD_SIZE); > > - } > > - > > pmd = pmd_modify(oldpmd, vma->vm_page_prot); > > page = vm_normal_page_pmd(vma, haddr, pmd); > > if (!page) > > -- > > 2.30.2 > >