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 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 94FEAC433ED for ; Fri, 30 Apr 2021 21:56:57 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id 222F5613C5 for ; Fri, 30 Apr 2021 21:56:57 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 222F5613C5 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 593416B0036; Fri, 30 Apr 2021 17:56:56 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 5443A6B006E; Fri, 30 Apr 2021 17:56:56 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 3E4726B0070; Fri, 30 Apr 2021 17:56:56 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0066.hostedemail.com [216.40.44.66]) by kanga.kvack.org (Postfix) with ESMTP id 21B616B0036 for ; Fri, 30 Apr 2021 17:56:56 -0400 (EDT) Received: from smtpin03.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay02.hostedemail.com (Postfix) with ESMTP id CC93712F1 for ; Fri, 30 Apr 2021 21:56:55 +0000 (UTC) X-FDA: 78090393990.03.CB7885E Received: from mail-ed1-f54.google.com (mail-ed1-f54.google.com [209.85.208.54]) by imf05.hostedemail.com (Postfix) with ESMTP id 86595E000113 for ; Fri, 30 Apr 2021 21:56:52 +0000 (UTC) Received: by mail-ed1-f54.google.com with SMTP id d14so20483363edc.12 for ; Fri, 30 Apr 2021 14:56:55 -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:content-transfer-encoding; bh=2G4yLAFcRqPd+NnouKTDTr3z68JyxuHQPUsNrVFoJJc=; b=f/9ZMyx1kIeeA48lngWqwDg8RFtDJAFANUk0l7Gec/yuxT7XDeQbCA/OiMXjRmwLiE qRjBmIZUHMJvpo76nfv2wNZqLQBjj1cgX1ZjqoygZNBO5axsH1M8DYbKeAJ7Oy/m/70k VW7Tab7YagPwtiWBuo8XQRBYkYKqbPLRlO6+GCb44brTJSELebNfMsmac0KG1wvFVQUR jlomJh6bbfl29F23BtERjZFUgNW20wCxJ8rgBx3+jL2ShNlXsB0aL4pcKP0zxL7XrRVl ZYdJbYGsDotPiIL0aRVZiUwU/Or/LTUi+ZCIRHniekp47zbsVa651GR8X13nDFxFGs3O pk+A== 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:content-transfer-encoding; bh=2G4yLAFcRqPd+NnouKTDTr3z68JyxuHQPUsNrVFoJJc=; b=qBz5ghRoF+O+7B7V8ucQN8jyJbzLqCSmEpG7qW9fyQnjAM2XezxDH4tlxq+PKKM4FU pzMdgML0yQDMTrm3t6hEbZ/bNl3mFCxMiZlsdSwh2ECC6pTT5+XVzwhONOcYMSbt4t9S 0ZTSy6WBdWXEGRGZK/pz8SKl2Zs9zNuZj0C9sUlo5zLNjV+nreWIbKiYneItxMaOyri8 n+O60JwIkjnalsZ4oUcobz/WvgN9TiKOGoSp6obJ5LPhPbNsyKRq6virS0nB6r8STXXm Griz4Kgu7KXl1L9Ra4ei6cZ5+WolKEr2VkFmnheY1y3m/cDHADKJckvy1PJPLxrWbwl8 m17A== X-Gm-Message-State: AOAM530+flINmR0w1r4ue3wgvxO/PuGJAnj+eNClprctbgZinSeyBJNm +sYESS8H0Vrplo+MmVMAImoTuPy5PaNPA9ZEsSs= X-Google-Smtp-Source: ABdhPJzO/PfuzO8Hk7UXhvlwGaC64O5IlOgXEYXq3J2mdDn+lFqvq6aRYuPaZygMdYpwHgKDcsbsMi/GZr20aXshtd8= X-Received: by 2002:a05:6402:1907:: with SMTP id e7mr8563133edz.313.1619819814179; Fri, 30 Apr 2021 14:56:54 -0700 (PDT) MIME-Version: 1.0 References: <20210430210744.216095-1-shy828301@gmail.com> In-Reply-To: From: Yang Shi Date: Fri, 30 Apr 2021 14:56:42 -0700 Message-ID: Subject: Re: [PATCH] mm: thp: check total_mapcount instead of page_mapcount To: Zi Yan Cc: "Kirill A. Shutemov" , Wang Yugui , Hugh Dickins , Andrew Morton , Linux MM , Linux Kernel Mailing List Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Rspamd-Server: rspam05 X-Rspamd-Queue-Id: 86595E000113 Authentication-Results: imf05.hostedemail.com; dkim=pass header.d=gmail.com header.s=20161025 header.b="f/9ZMyx1"; dmarc=pass (policy=none) header.from=gmail.com; spf=pass (imf05.hostedemail.com: domain of shy828301@gmail.com designates 209.85.208.54 as permitted sender) smtp.mailfrom=shy828301@gmail.com X-Stat-Signature: kudqcrtpmphm6cdj3otegpj9gukd1s4h Received-SPF: none (gmail.com>: No applicable sender policy available) receiver=imf05; identity=mailfrom; envelope-from=""; helo=mail-ed1-f54.google.com; client-ip=209.85.208.54 X-HE-DKIM-Result: pass/pass X-HE-Tag: 1619819812-729892 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 Fri, Apr 30, 2021 at 2:30 PM Zi Yan wrote: > > On 30 Apr 2021, at 17:07, Yang Shi wrote: > > > When debugging the bug reported by Wang Yugui [1], try_to_unmap() may > > return false positive for PTE-mapped THP since page_mapcount() is used > > to check if the THP is unmapped, but it just checks compound mapount an= d > > head page's mapcount. If the THP is PTE-mapped and head page is not > > mapped, it may return false positive. > > > > Use total_mapcount() instead of page_mapcount() and do so for the > > VM_BUG_ON_PAGE in split_huge_page_to_list as well. > > > > [1] https://lore.kernel.org/linux-mm/20210412180659.B9E3.409509F4@e16-t= ech.com/ > > > > Signed-off-by: Yang Shi > > --- > > mm/huge_memory.c | 2 +- > > mm/rmap.c | 2 +- > > 2 files changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/mm/huge_memory.c b/mm/huge_memory.c > > index 63ed6b25deaa..2122c3e853b9 100644 > > --- a/mm/huge_memory.c > > +++ b/mm/huge_memory.c > > @@ -2718,7 +2718,7 @@ int split_huge_page_to_list(struct page *page, st= ruct list_head *list) > > } > > > > unmap_page(head); > > - VM_BUG_ON_PAGE(compound_mapcount(head), head); > > + VM_BUG_ON_PAGE(total_mapcount(head), head); > > I am not sure about this change. The code below also checks total_mapcoun= t(head) > and returns EBUSY if the count is non-zero. This change makes the code de= ad. It is actually dead if CONFIG_DEBUG_VM is enabled and total_mapcount is not 0 regardless of this change due to the below code, right? if (IS_ENABLED(CONFIG_DEBUG_VM) && mapcount) { pr_alert("total_mapcount: %u, page_count(): %u\n", mapcount, count); if (PageTail(page)) dump_page(head, NULL); dump_page(page, "total_mapcount(head) > 0"); BUG(); } > On the other hand, the change will force all mappings to the page have to= be > successfully unmapped all the time. I am not sure if we want to do that. > Maybe it is better to just check total_mapcount() and fail the split. > The same situation happens with the code change below. IIUC, the code did force all mappings to the page to be unmapped in order to split it. > > > > > /* block interrupt reentry in xa_lock and spinlock */ > > local_irq_disable(); > > diff --git a/mm/rmap.c b/mm/rmap.c > > index 693a610e181d..2e547378ab5f 100644 > > --- a/mm/rmap.c > > +++ b/mm/rmap.c > > @@ -1777,7 +1777,7 @@ bool try_to_unmap(struct page *page, enum ttu_fla= gs flags) > > else > > rmap_walk(page, &rwc); > > > > - return !page_mapcount(page) ? true : false; > > + return !total_mapcount(page) ? true : false; > > } > > In unmap_page(), VM_BUG_ON_PAGE(!unmap_success, page) will force all mapp= ings > to the page have to be all unmapped, which might not be the case we want. AFAICT, I don't see such a case from all the callers of try_to_unmap(). Imay miss something, but I do have a hard time thinking of a usecase which can proceed safely with "not fully unmapped" page. > Maybe you will want to remove the VM_BUG_ON_PAGE here, check total_mapcou= nt() > above, and fail the split if not all mappings to the pages are unmapped. > > > > =E2=80=94 > Best Regards, > Yan Zi