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=-5.3 required=3.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,USER_AGENT_SANE_1 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 75487C10F14 for ; Wed, 9 Oct 2019 01:07:59 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id 8E422206C2 for ; Wed, 9 Oct 2019 01:07:58 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 8E422206C2 Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=huawei.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=owner-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix) id CB1F28E0005; Tue, 8 Oct 2019 21:07:57 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id C62918E0003; Tue, 8 Oct 2019 21:07:57 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id B9ED88E0005; Tue, 8 Oct 2019 21:07:57 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0252.hostedemail.com [216.40.44.252]) by kanga.kvack.org (Postfix) with ESMTP id 934798E0003 for ; Tue, 8 Oct 2019 21:07:57 -0400 (EDT) Received: from smtpin01.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay02.hostedemail.com (Postfix) with SMTP id 1F7AF2C12 for ; Wed, 9 Oct 2019 01:07:57 +0000 (UTC) X-FDA: 76022459394.01.paste24_282c04ce6531c X-HE-Tag: paste24_282c04ce6531c X-Filterd-Recvd-Size: 6119 Received: from huawei.com (szxga06-in.huawei.com [45.249.212.32]) by imf36.hostedemail.com (Postfix) with ESMTP for ; Wed, 9 Oct 2019 01:07:55 +0000 (UTC) Received: from DGGEMS406-HUB.china.huawei.com (unknown [172.30.72.60]) by Forcepoint Email with ESMTP id 08B3E613AE085AB13506; Wed, 9 Oct 2019 09:07:53 +0800 (CST) Received: from [127.0.0.1] (10.133.217.137) by DGGEMS406-HUB.china.huawei.com (10.3.19.206) with Microsoft SMTP Server id 14.3.439.0; Wed, 9 Oct 2019 09:07:45 +0800 Subject: Re: [Question] pfn_valid_within(page_to_pfn(buddy)) vs pfn_valid_within(buddy_pfn) in __free_one_page() To: Vlastimil Babka , CC: Michal Hocko , Andrew Morton , Mel Gorman , References: <67e418a0-d56b-8b2b-13da-c777e67f7013@suse.cz> From: Kefeng Wang Message-ID: Date: Wed, 9 Oct 2019 09:07:44 +0800 User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:60.0) Gecko/20100101 Thunderbird/60.7.0 MIME-Version: 1.0 In-Reply-To: <67e418a0-d56b-8b2b-13da-c777e67f7013@suse.cz> Content-Type: text/plain; charset="utf-8" Content-Language: en-US Content-Transfer-Encoding: 7bit X-Originating-IP: [10.133.217.137] X-CFilter-Loop: Reflected 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 2019/10/8 17:12, Vlastimil Babka wrote: > On 10/8/19 10:35 AM, Kefeng Wang wrote: >> Hi Vlastimil and all, >> >> We met a Null pointer when do page_to_pfn() in __free_one_page() in older kernel, __nr_to_section(__sec) return NULL, >> >> #define __page_to_pfn(pg) \ >> ({ const struct page *__pg = (pg); \ >> int __sec = page_to_section(__pg); \ >> (unsigned long)(__pg - __section_mem_map_addr(__nr_to_section(__sec))); \ >> }) >> >> Before v4.11, __free_one_page() use pfn_valid_within(page_to_pfn(buddy)) to check pfn, after the > > Hmm, looks like the code before v4.11 was wrong. pfn_valid_(within) > should be checked first, before obtaining and working with the struct > page. Here we already have a struct page obtained by pointer arithmetics, > and are calling page_to_pfn() on it, which means accessing its flags. > The pfn_valid_within() then comes too late. > >> following patches, it use buddy_pfn directly. >> >> b4fb8f66f1ae mm, page_alloc: Add missing check for memory holes >> 13ad59df67f1 mm, page_alloc: avoid page_to_pfn() when merging buddies // NOTE: directly use buddy_pfn >> 76741e776a37 mm, page_alloc: don't convert pfn to idx when merging > > Looks like my patches fixed that code without realizing there was > the bug. Commit b4fb8f66f1ae shows I've also introduced it elsewhere. > >> If use page_to_pfn(buddy) back in mainline 5.4-rc2, the same issue will occur, > > No surprise, we must validate pfn first before touching the page. > >> diff --git a/mm/page_alloc.c b/mm/page_alloc.c >> index c0b2e0306720..fbbfe8e8b4ca 100644 >> --- a/mm/page_alloc.c >> +++ b/mm/page_alloc.c >> @@ -934,7 +934,7 @@ static inline void __free_one_page(struct page *page, >> buddy_pfn = __find_buddy_pfn(pfn, order); >> buddy = page + (buddy_pfn - pfn); >> >> - if (!pfn_valid_within(buddy_pfn)) >> + if (!pfn_valid_within(page_to_pfn(buddy))) >> goto done_merging; >> if (!page_is_buddy(page, buddy, order)) >> goto done_merging; >> >> It shows the buddy->flags is wrong, that is, buddy is bad, we find the buddy by page + (buddy_pfn - pfn), >> so there is some issue in __find_buddy_pfn(pfn, order)? > > No, result of __find_buddy_pfn has to be validated first. Hi Vlastimil, Thank you for your explanation, got it. > >> The following is the debug print and CallTrace, any comment? >> >> Thanks, >> Kefeng >> >> 1) MEMBLOCK configuration: >> memory size = 0x0000000036800000 reserved size = 0x0000000004ca7fbc >> memory.cnt = 0x4 >> memory[0x0] [0x0000000000000000-0x0000000013ffffff], 0x0000000014000000 bytes flags: 0x0 >> memory[0x1] [0x000000002d600000-0x0000000033ffffff], 0x0000000006a00000 bytes flags: 0x0 >> memory[0x2] [0x0000000034800000-0x00000000445fffff], 0x000000000fe00000 bytes flags: 0x0 >> memory[0x3] [0x0000000044a00000-0x00000000509fffff], 0x000000000c000000 bytes flags: 0x0 >> reserved.cnt = 0x6 >> reserved[0x0] [0x000000002d680000-0x000000002e7c8fff], 0x0000000001149000 bytes flags: 0x0 >> reserved[0x1] [0x0000000030b00000-0x000000003239cfff], 0x000000000189d000 bytes flags: 0x0 >> reserved[0x2] [0x0000000032400000-0x00000000324fffff], 0x0000000000100000 bytes flags: 0x0 >> reserved[0x3] [0x000000004e000000-0x000000004fffffff], 0x0000000002000000 bytes flags: 0x0 >> reserved[0x4] [0x000000005083e040-0x0000000050849ffb], 0x000000000000bfbc bytes flags: 0x0 >> reserved[0x5] [0x000000005084a000-0x00000000509fffff], 0x00000000001b6000 bytes flags: 0x0 > > These might be holes in the zones, right. > >> 2) CONFIG >> CONFIG_SPARSEMEM_MANUAL=y >> CONFIG_SPARSEMEM=y >> CONFIG_HAVE_MEMORY_PRESENT=y >> CONFIG_SPARSEMEM_EXTREME=y >> CONFIG_SPARSEMEM_VMEMMAP_ENABLE=y >> # CONFIG_SPARSEMEM_VMEMMAP is not set > > Is CONFIG_HOLES_IN_ZONE enabled? Probably yes as that's arm64. Yes, arm64 force enable HOLES_IN_ZONE. > >> 3) debug print and CallTrace >> __free_one_page , order = 9, max_order = 11, page = ffffff804e128000, buddy = ffffff804e120000, sec = 42623, mem_section = 0000000000000000 > > I would assume buddy is in one of the holes, but you'd have to print the pfn's to be sure. buddy_pfn = 280576, buddy_addr = 44800000, and it is in the hole between memory[2] and memory[3]. > >> buddy = ffffff804e120000, __sec = 42623, buddy->flags = 299fcc27aebc552f, SECTIONS_PGSHIFT = 46, SECTIONS_MASK = 3ffff >> __section_mem_map_addr, section = 0000000000000000