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=-14.2 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,NICE_REPLY_A,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED, 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 59914C4338F for ; Fri, 13 Aug 2021 00:21:09 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id EDEA2610A7 for ; Fri, 13 Aug 2021 00:21:08 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org EDEA2610A7 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=gmail.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=kvack.org Received: by kanga.kvack.org (Postfix) id 146BD6B006C; Thu, 12 Aug 2021 20:21:08 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 0F7296B0071; Thu, 12 Aug 2021 20:21:08 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id F27578D0001; Thu, 12 Aug 2021 20:21:07 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0117.hostedemail.com [216.40.44.117]) by kanga.kvack.org (Postfix) with ESMTP id D7EBE6B006C for ; Thu, 12 Aug 2021 20:21:07 -0400 (EDT) Received: from smtpin40.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay01.hostedemail.com (Postfix) with ESMTP id 682A6180ACC2C for ; Fri, 13 Aug 2021 00:21:07 +0000 (UTC) X-FDA: 78468152574.40.3299F44 Received: from mail-pl1-f169.google.com (mail-pl1-f169.google.com [209.85.214.169]) by imf25.hostedemail.com (Postfix) with ESMTP id 1D080B009FF5 for ; Fri, 13 Aug 2021 00:21:07 +0000 (UTC) Received: by mail-pl1-f169.google.com with SMTP id e15so9610221plh.8 for ; Thu, 12 Aug 2021 17:21:06 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=subject:to:cc:references:from:message-id:date:user-agent :mime-version:in-reply-to:content-language:content-transfer-encoding; bh=jPOasYiBAcjG9pv11zmhOsUezGxQh8Ee+W8L/ZGWhXE=; b=Ucsw6v7F1/7OrQNGaQv6pTULwv5MHBtWCvb27jnzWJtP2n85q1GxWLopCu354aJfLM MR/o+T1kNXS3HBXN6/BXF2t5qtOsc7kaHFIgH2OUXrm2MTHoGQgcDfYhuPlfUHp9NvlQ TuS+xd4bdJbA4Tse+EZr7HouuH634AaflULJR+4AuaJLK1EXeq9RyquIdWVw1MZIH/tZ WO+3+y8M0asx6RjlDnpiEdeJ171TN8Ib10X08Z6ism/3AUZgng6P8Ha/Nueokg90u1DL kfdPt9MDV1aTQb0mWjr22j/HM8O1T7XkxDtxLkPOeGwG/MAiVLIwzIPPZ2QTRLlhqdqD AzGQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:to:cc:references:from:message-id:date :user-agent:mime-version:in-reply-to:content-language :content-transfer-encoding; bh=jPOasYiBAcjG9pv11zmhOsUezGxQh8Ee+W8L/ZGWhXE=; b=I1eD46FkE6RMMwAHDUgtpB6BRc897aRO/PP7+0cH1I9+1ALHOwyER1Y4OGUSMkH5RE qyTFSmh1ENTn4cSRGNnLaIxnyAQvmdTJgvUlAa/pXzgfNpyx5Ya3p8T3Q3/BNDoT/VKw xtZ1x2ThFVT74EneujXIwCXT6rEo0nNa6Ol6/OBXIX88K89F6xz8CKsjScXgPwPhqe23 GN1AKSvQLLDL/ejdMFzY9GX7yu+GZGvEEyMgfLUNsV6lIdxcQZPPGZZ0+Z0JTL1w+ZFv y3AuYVphqf6jwehsUtVMZgbqaTnP9WuChKJGJJpaHh2o+xAUaEjskeAkT61w173IM8JD A7nQ== X-Gm-Message-State: AOAM531CrwlryOm+UoPc0dMqPyvdQzg+LhUlh7rtJzRUZLdpPiXyuaxz a5e0Nh4suzJDChvGUSGFc14= X-Google-Smtp-Source: ABdhPJxeJzQTTki0QzVX/bcIs+hRPolDk7b10eftkkr7ujda30hSu8XaKsdtDBfvSl21VRgIAKMYaQ== X-Received: by 2002:a63:4e11:: with SMTP id c17mr6120172pgb.54.1628814066103; Thu, 12 Aug 2021 17:21:06 -0700 (PDT) Received: from [10.69.57.159] ([192.19.223.252]) by smtp.gmail.com with ESMTPSA id n23sm4846116pgv.76.2021.08.12.17.21.02 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Thu, 12 Aug 2021 17:21:05 -0700 (PDT) Subject: Re: [PATCH] mm/page_alloc: don't corrupt pcppage_migratetype To: Vlastimil Babka , Andrew Morton Cc: Mel Gorman , "Peter Zijlstra (Intel)" , linux-mm@kvack.org, linux-kernel@vger.kernel.org References: <20210811182917.2607994-1-opendmb@gmail.com> From: Doug Berger Message-ID: Date: Thu, 12 Aug 2021 17:21:00 -0700 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:78.0) Gecko/20100101 Thunderbird/78.13.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit Authentication-Results: imf25.hostedemail.com; dkim=pass header.d=gmail.com header.s=20161025 header.b=Ucsw6v7F; dmarc=pass (policy=none) header.from=gmail.com; spf=pass (imf25.hostedemail.com: domain of opendmb@gmail.com designates 209.85.214.169 as permitted sender) smtp.mailfrom=opendmb@gmail.com X-Stat-Signature: xr97g81agmrqd4r5ks5i4u6ut1nyrqxq X-Rspamd-Queue-Id: 1D080B009FF5 X-Rspamd-Server: rspam05 X-HE-Tag: 1628814067-304595 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 8/12/2021 1:17 AM, Vlastimil Babka wrote: > On 8/11/21 8:29 PM, Doug Berger wrote: >> When placing pages on a pcp list, migratetype values over >> MIGRATE_PCPTYPES get added to the MIGRATE_MOVABLE pcp list. >> >> However, the actual migratetype is preserved in the page and >> should not be changed to MIGRATE_MOVABLE or the page may end >> up on the wrong free_list. > > Nice, how did you find out? Were there any user-visible effects? (Hint: which? > the changelog should say that so that the severity of the bug can be judged). > Otherwise I agree the bug is there and patch is fixing it. Thanks. I did not observe the bug "in the wild", but rather noticed it while reviewing the current CMA implementation. I would imagine in the worst case that CMA memory could leak onto MIGRATE_MOVABLE free_lists and get "stolen" through fallback for MIGRATE_UNMOVABLE allocations effectively breaking CMA. For full disclosure I should take this opportunity to say that I am investigating ways to improve the CMA/Movable memory implementations somewhat in line with what Joonsoo Kim has been trying, which is why I observed this bug. However, I largely agree with the proposal Mel Gorman made in his comments on: https://lkml.org/lkml/2016/4/28/244 I can appreciate that zones may provide a convenient way of grouping memory with common attributes, but it is a bastardization of the original intention of zones. I would prefer a mechanism for designating blocks of memory as "sticky" movable such that they can only be used to satisfy movable memory requests and not be converted to other migratetypes. I've wrestled with the terminology and currently favor Designated Movable Blocks (as opposed to "sticky") because these are blocks of memory that have the movable attribute by designation rather than by default or accident. While CMA blocks should be a subset of Designated Movable Blocks since they have the same migration properties, it may be desirable/necessary to maintain a distinction to accommodate competing allocation objectives. Specifically, Designated Movable Blocks used by CMA to satisfy the needs of drivers will likely want the page allocator to be less aggressive than users of Memory Hot-plug or kernelcore= Designated Movable Blocks. Perhaps a round-robin scheme with MIGRATE_MOVABLE can satisfy all users, but I'm not too optimistic. I don't know if Mel has changed his opinions on this matter, but any suggestions and encouragement are appreciated. Thanks, Doug > >> Fixes: df1acc856923 ("mm/page_alloc: avoid conflating IRQs disabled with zone->lock") >> Signed-off-by: Doug Berger > > Acked-by: Vlastimil Babka > >> --- >> mm/page_alloc.c | 25 ++++++++++++------------- >> 1 file changed, 12 insertions(+), 13 deletions(-) >> >> diff --git a/mm/page_alloc.c b/mm/page_alloc.c >> index 73704e836649..8addb4919f75 100644 >> --- a/mm/page_alloc.c >> +++ b/mm/page_alloc.c >> @@ -3436,19 +3436,10 @@ void free_unref_page_list(struct list_head *list) >> * comment in free_unref_page. >> */ >> migratetype = get_pcppage_migratetype(page); >> - if (unlikely(migratetype >= MIGRATE_PCPTYPES)) { >> - if (unlikely(is_migrate_isolate(migratetype))) { >> - list_del(&page->lru); >> - free_one_page(page_zone(page), page, pfn, 0, >> - migratetype, FPI_NONE); >> - continue; >> - } >> - >> - /* >> - * Non-isolated types over MIGRATE_PCPTYPES get added >> - * to the MIGRATE_MOVABLE pcp list. >> - */ >> - set_pcppage_migratetype(page, MIGRATE_MOVABLE); >> + if (unlikely(is_migrate_isolate(migratetype))) { >> + list_del(&page->lru); >> + free_one_page(page_zone(page), page, pfn, 0, migratetype, FPI_NONE); >> + continue; >> } >> >> set_page_private(page, pfn); >> @@ -3458,7 +3449,15 @@ void free_unref_page_list(struct list_head *list) >> list_for_each_entry_safe(page, next, list, lru) { >> pfn = page_private(page); >> set_page_private(page, 0); >> + >> + /* >> + * Non-isolated types over MIGRATE_PCPTYPES get added >> + * to the MIGRATE_MOVABLE pcp list. >> + */ >> migratetype = get_pcppage_migratetype(page); >> + if (unlikely(migratetype >= MIGRATE_PCPTYPES)) >> + migratetype = MIGRATE_MOVABLE; >> + >> trace_mm_page_free_batched(page); >> free_unref_page_commit(page, pfn, migratetype, 0); >> >> >