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 9587EC433F5 for ; Tue, 11 Jan 2022 23:14:27 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 175446B00F0; Tue, 11 Jan 2022 18:14:27 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 125C56B00F1; Tue, 11 Jan 2022 18:14:27 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id F08116B00F2; Tue, 11 Jan 2022 18:14:26 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0112.hostedemail.com [216.40.44.112]) by kanga.kvack.org (Postfix) with ESMTP id DD0656B00F0 for ; Tue, 11 Jan 2022 18:14:26 -0500 (EST) Received: from smtpin22.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay01.hostedemail.com (Postfix) with ESMTP id 855EE1802E8D1 for ; Tue, 11 Jan 2022 23:14:26 +0000 (UTC) X-FDA: 79019562132.22.F2C0217 Received: from mail-ed1-f52.google.com (mail-ed1-f52.google.com [209.85.208.52]) by imf17.hostedemail.com (Postfix) with ESMTP id 075F04000C for ; Tue, 11 Jan 2022 23:14:25 +0000 (UTC) Received: by mail-ed1-f52.google.com with SMTP id 30so2563562edv.3 for ; Tue, 11 Jan 2022 15:14:25 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=zodC5tAE+LTr3PkA8XyvCfIELEWqA7/RlSo76tDLh4k=; b=WTfGHqmDGKI2sZTDXuV9Z8kojFWW8RuR6q7BEdopfQiCk6PSJ1iD9Agn6yuLFIUlDm e3mTLubSBVpOEB8qLpsYNVqkcOkLs7Zj/rsK8yu5Un9z65NPwpRECgqz/0oNuzWYPQwA /YQ7dLbGnSQI5JKoXlsnV5lkd3ZFgOifu2NpbNghElhdFojZBhCiiRgVMh4xpMKiQN7a FIcqsMnv2bX2zjnix8L1U0Vj/YkqI9NRy0VsMNhKa1MFjXTvGvLO3jkwL9qbtaEpD4v/ I2KUE95JdHDFS+cNf+NEu7CHnDy7opQL4R07OOeaa1NTPgd6MNZ9LDn3ELF/6yClUh+L F1GA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=zodC5tAE+LTr3PkA8XyvCfIELEWqA7/RlSo76tDLh4k=; b=ATrKY4G0UjTpNE4M9UP+fmhgiyO9EbWTogCxUrA+PQxStgpW6jbsUcNp4yWBCrbCd3 54IL3v+sUxETD6zOmW0fk+DlixKfTSrjkkngksZ1T92yYmVqLdlw84nDw/AgJjauRyeY m+5tkStlvA8W5ofq2a9edTcoSSR49I/mZEVkIhe5ClRNytWCJJPfk9dlbkvbDoE4GYxm pqDICzcTUDBnz5KtFNS4SZj91rNdtohe53FiUoAbZ/cj2zx/Sqnpl82Sc6IrY8ilojz2 KX7WH7obqJgBKjbkL+awbZdO/l7l9Hnof7Y5ETEyrlZFNLif06GvzMwUSisdi/d299Gm 6AYA== X-Gm-Message-State: AOAM533WOuuV9Df44LQtdISRZtbyMITl0p2Oz8jVapNPFH39xvRIhKGN UD4Zaa/mG0abv0bWQTesV+PH5oUbh0FO9P+Lh6c= X-Google-Smtp-Source: ABdhPJw89ofAd9jHvMdmTk5XtB/xn9dI/SVADS6MAC33VWWg4HxB/h1vzzxGLaaAVqG0IvndIhm15awHTZjAFITzRkU= X-Received: by 2002:a17:907:608b:: with SMTP id ht11mr4248949ejc.644.1641942864650; Tue, 11 Jan 2022 15:14:24 -0800 (PST) MIME-Version: 1.0 References: <00000000000017977605c395a751@google.com> <0000000000009411bb05d3ab468f@google.com> In-Reply-To: From: Yang Shi Date: Tue, 11 Jan 2022 15:14:12 -0800 Message-ID: Subject: Re: [syzbot] kernel BUG in __page_mapcount To: Matthew Wilcox Cc: syzbot , Andrew Morton , Alistair Popple , chinwen.chang@mediatek.com, fgheet255t@gmail.com, Jann Horn , Konstantin Khlebnikov , "Kirill A. Shutemov" , "Kirill A. Shutemov" , Linux FS-devel Mailing List , Linux Kernel Mailing List , Linux MM , Peter Xu , Peter Zijlstra , syzkaller-bugs@googlegroups.com, tonymarislogistics@yandex.com, Vlastimil Babka , Zi Yan Content-Type: text/plain; charset="UTF-8" X-Rspamd-Queue-Id: 075F04000C X-Stat-Signature: iehycwxcqoiw5osucui8mucmc56x1jwf Authentication-Results: imf17.hostedemail.com; dkim=pass header.d=gmail.com header.s=20210112 header.b=WTfGHqmD; dmarc=pass (policy=none) header.from=gmail.com; spf=pass (imf17.hostedemail.com: domain of shy828301@gmail.com designates 209.85.208.52 as permitted sender) smtp.mailfrom=shy828301@gmail.com X-Rspamd-Server: rspam02 X-HE-Tag: 1641942865-615940 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 Wed, Jan 5, 2022 at 11:05 AM Yang Shi wrote: > > On Tue, Dec 21, 2021 at 10:40 AM Matthew Wilcox wrote: > > > > On Tue, Dec 21, 2021 at 10:24:27AM -0800, Yang Shi wrote: > > > It seems the THP is split during smaps walk. The reproducer does call > > > MADV_FREE on partial THP which may split the huge page. > > > > > > The below fix (untested) should be able to fix it. > > > > Did you read the rest of the thread on this? If the page is being > > I just revisited this. Now I see what you mean about "the rest of the > thread". My gmail client doesn't put them in the same thread, sigh... > > Yeah, try_get_compound_head() seems like the right way. > > Or we just simply treat migration entries as mapcount == 1 as Kirill > suggested or just skip migration entries since they are transient or > show migration entries separately. I think Kirill's suggestion makes some sense. The migration entry's mapcount is 0 so "pss /= mapcount" is not called at all, so the migration entry is actually treated like mapcount == 1. This doesn't change the behavior. Not like swap entry, we actually can't tell how many references for the migration entry. But we should handle private device entry differently since its mapcount is inc'ed when it is shared between processes. The regular migration entry could be identified by is_migration_entry() easily. Using try_get_compound_head() seems overkilling IMHO. I just came up with the below patch (built and running the producer didn't trigger the bug for me so far). If it looks fine, I will submit it in a formal patch with more comments. diff --git a/fs/proc/task_mmu.c b/fs/proc/task_mmu.c index ad667dbc96f5..6a48bbb51efa 100644 --- a/fs/proc/task_mmu.c +++ b/fs/proc/task_mmu.c @@ -429,7 +429,8 @@ static void smaps_page_accumulate(struct mem_size_stats *mss, } static void smaps_account(struct mem_size_stats *mss, struct page *page, - bool compound, bool young, bool dirty, bool locked) + bool compound, bool young, bool dirty, bool locked, + bool migration) { int i, nr = compound ? compound_nr(page) : 1; unsigned long size = nr * PAGE_SIZE; @@ -457,7 +458,7 @@ static void smaps_account(struct mem_size_stats *mss, struct page *page, * If any subpage of the compound page mapped with PTE it would elevate * page_count(). */ - if (page_count(page) == 1) { + if ((page_count(page) == 1) || migration) { smaps_page_accumulate(mss, page, size, size << PSS_SHIFT, dirty, locked, true); return; @@ -506,6 +507,7 @@ static void smaps_pte_entry(pte_t *pte, unsigned long addr, struct vm_area_struct *vma = walk->vma; bool locked = !!(vma->vm_flags & VM_LOCKED); struct page *page = NULL; + bool migration = false; if (pte_present(*pte)) { page = vm_normal_page(vma, addr, *pte); @@ -525,8 +527,11 @@ static void smaps_pte_entry(pte_t *pte, unsigned long addr, } else { mss->swap_pss += (u64)PAGE_SIZE << PSS_SHIFT; } - } else if (is_pfn_swap_entry(swpent)) + } else if (is_pfn_swap_entry(swpent)) { + if (is_migration_entry(swpent)) + migration = true; page = pfn_swap_entry_to_page(swpent); + } } else { smaps_pte_hole_lookup(addr, walk); return; @@ -535,7 +540,8 @@ static void smaps_pte_entry(pte_t *pte, unsigned long addr, if (!page) return; - smaps_account(mss, page, false, pte_young(*pte), pte_dirty(*pte), locked); + smaps_account(mss, page, false, pte_young(*pte), pte_dirty(*pte), + locked, migration); } #ifdef CONFIG_TRANSPARENT_HUGEPAGE @@ -546,6 +552,7 @@ static void smaps_pmd_entry(pmd_t *pmd, unsigned long addr, struct vm_area_struct *vma = walk->vma; bool locked = !!(vma->vm_flags & VM_LOCKED); struct page *page = NULL; + bool migration = false; if (pmd_present(*pmd)) { /* FOLL_DUMP will return -EFAULT on huge zero page */ @@ -553,8 +560,10 @@ static void smaps_pmd_entry(pmd_t *pmd, unsigned long addr, } else if (unlikely(thp_migration_supported() && is_swap_pmd(*pmd))) { swp_entry_t entry = pmd_to_swp_entry(*pmd); - if (is_migration_entry(entry)) + if (is_migration_entry(entry)) { + migration = true; page = pfn_swap_entry_to_page(entry); + } } if (IS_ERR_OR_NULL(page)) return; @@ -566,7 +575,9 @@ static void smaps_pmd_entry(pmd_t *pmd, unsigned long addr, /* pass */; else mss->file_thp += HPAGE_PMD_SIZE; - smaps_account(mss, page, true, pmd_young(*pmd), pmd_dirty(*pmd), locked); + + smaps_account(mss, page, true, pmd_young(*pmd), pmd_dirty(*pmd), + locked, migration); } #else static void smaps_pmd_entry(pmd_t *pmd, unsigned long addr, > > > > migrated, we should still account it ... also, you've changed the > > refcount, so this: > > > > if (page_count(page) == 1) { > > smaps_page_accumulate(mss, page, size, size << PSS_SHIFT, dirty, > > locked, true); > > return; > > } > > > > will never trigger.