Received: by 2002:a25:d80d:0:0:0:0:0 with SMTP id p13csp39893ybg; Sat, 23 May 2020 06:57:41 -0700 (PDT) X-Google-Smtp-Source: ABdhPJyCb6kc/2TKUNrjiAt7C/JZW53NzMcyGLrXVRe5Z3hgQ5XQtnkJCz/Ns69oD50GYtrqRS4P X-Received: by 2002:a17:906:3652:: with SMTP id r18mr11781084ejb.526.1590242260862; Sat, 23 May 2020 06:57:40 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1590242260; cv=none; d=google.com; s=arc-20160816; b=0kMFx/StlleYGeMJxSZBmLlU6ia5Bju5nn0g9tAGEFeR29ptmEG0/fyt1pizm46kbj dPyax6gxm3dcjwXCyZ+tQllcIoIqmapzTADfuQ6jDnkN/BxL/gGIFlf3ODa36tkeIgq4 MLbGj/sMPHDjQ5oarhX8dEEEPBvO3yobq6JKr1YblEiiPY6UqxIejnlInxTIraHqdhbR yyxs4M0ZVaEpLMBLUuvWBBkCLzhAGkz8CO6dF5jcMexg57us+muy5TPE9xnp7/1uzvrA muXJxeuKR7YXZ1VkKMzjvSWCkwNhEYncQUeBprfCkdYlyeMHnRKj+pbyS1ifjL3jxRLA xiMw== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:sender:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:dkim-signature; bh=F0hCwA7JT4X1b7+zad20E8g0U+y7W7iuywlvSyNnvCU=; b=Ginnitre4crqZH6T49eU9IBnG3+Rp8Sj9upj68lZ+w5713cOUyCd5+IcvgZoiNsbCE b9iNWMgz0PdqxHOW638aM9vpgg5QX+V4mmw5SDIAiQ8BSGyZDsjz8lmvODZD9N5etR5o S0euYopUDmoL34GNuCSMOU412HwqY2C4DNvDCGqp98FN4gQn1bpCTtNUQQuP6K7Z29tt Is1p9mYrbpjClTjFXtiNGKNYU7MjTvkPRXbJh8WVqqdDfODVfHRb1QcY9eqODFDexmkc fVxDT8tc2MIkslV3V24P2u6ExGpZwOPwFLbXirBGKguDVBJsijeV+EHcLfWRYzHV1cj4 afQA== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@gmail.com header.s=20161025 header.b=jLBvHkLr; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=QUARANTINE dis=NONE) header.from=gmail.com Return-Path: Received: from vger.kernel.org (vger.kernel.org. [23.128.96.18]) by mx.google.com with ESMTP id hb12si6209609ejb.695.2020.05.23.06.57.16; Sat, 23 May 2020 06:57:40 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18; Authentication-Results: mx.google.com; dkim=pass header.i=@gmail.com header.s=20161025 header.b=jLBvHkLr; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=QUARANTINE dis=NONE) header.from=gmail.com Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S2387815AbgEWNzN (ORCPT + 99 others); Sat, 23 May 2020 09:55:13 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:45680 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S2387529AbgEWNzM (ORCPT ); Sat, 23 May 2020 09:55:12 -0400 Received: from mail-io1-xd42.google.com (mail-io1-xd42.google.com [IPv6:2607:f8b0:4864:20::d42]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id A4FA2C061A0E for ; Sat, 23 May 2020 06:55:12 -0700 (PDT) Received: by mail-io1-xd42.google.com with SMTP id s18so564304ioe.2 for ; Sat, 23 May 2020 06:55:12 -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=F0hCwA7JT4X1b7+zad20E8g0U+y7W7iuywlvSyNnvCU=; b=jLBvHkLrjVGhDJAT27jK7QCt/N3HEh2eL7QU0V+B5Jn0WlyR/s+jOq+sN9bviIaHLu ekGjNj0X9+pI2jwqEt4twdP910Gb7pZ47jgDKUOVGHvwr8Uo525eTRaQXnCv2AcYT+uM moi4SMIcOkFZxnI2GpJSyImmQaJ+0CRgnaODi/fUNo4FCgiND77oKAQJ24Xf0hkDOXxP nm00cUAZI4AaQL12TdVs00lFdNbwZJn4Svd2ZvD1E17CfM2fikrVOSgk5VL9xUWeMJds W/eESM2TtfWzLJRLwdnP5n91i5laW1ZekL2Snq0iVm2BkHvSQQNqEIY1ENHiBrvyATKU ZnZg== 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=F0hCwA7JT4X1b7+zad20E8g0U+y7W7iuywlvSyNnvCU=; b=TcgwB4qIZbf5eJSw+hFJEYRwTYDjPVTwFRzOSEUMhnXKZ3CE0fCSEj+G/391tT1l4a aTIUnCxEPZfO1YFilIjOLkMS7kHc9h26SzdQ17m+2YqHSsS2brnMS8e9xyvlEzMUFhzS yW5ejftpRls9gADf2De44nmbv0B8pvrvcQ4wBHS35m94xuTLifTkTxu2DsIkvTm/r0p4 UrB49xrpbGxqKrV6eU1vnL7RlRRrx6DYx/VMyyN0z0g4SqhyYvS2xW9MBmbwgspLcQG/ 8g5sMy7IztBQLNh/A4sa1cSYOGnGk0WkE1stRPsynEWOmZKcWptrX08AI7SNBwMmWOuA QFsg== X-Gm-Message-State: AOAM530uyGx0nC0SCxNNfhFdmKi8uX7zF+Z8p0Pr/DQjz44G4aOkj5MK fvCT7WGemM7iIIGpx08qfMrMz4mJsiz8sTQ1QEg= X-Received: by 2002:a05:6638:bc4:: with SMTP id g4mr12353671jad.55.1590242111991; Sat, 23 May 2020 06:55:11 -0700 (PDT) MIME-Version: 1.0 References: <158937872515.474360.5066096871639561424.stgit@buzz> <20200522183421.7c52ba9650a2ad11cec2be8d@linux-foundation.org> In-Reply-To: <20200522183421.7c52ba9650a2ad11cec2be8d@linux-foundation.org> From: Konstantin Khlebnikov Date: Sat, 23 May 2020 16:55:30 +0300 Message-ID: Subject: Re: [PATCH] mm/compaction: avoid VM_BUG_ON(PageSlab()) in page_mapcount() To: Andrew Morton Cc: Konstantin Khlebnikov , Linux Kernel Mailing List , linux-mm@kvack.org, Vlastimil Babka , David Rientjes , Hugh Dickins , Mel Gorman Content-Type: text/plain; charset="UTF-8" Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Sat, May 23, 2020 at 4:34 AM Andrew Morton wrote: > > On Wed, 13 May 2020 17:05:25 +0300 Konstantin Khlebnikov wrote: > > > Function isolate_migratepages_block() runs some checks out of lru_lock > > when choose pages for migration. After checking PageLRU() it checks extra > > page references by comparing page_count() and page_mapcount(). Between > > these two checks page could be removed from lru, freed and taken by slab. > > > > As a result this race triggers VM_BUG_ON(PageSlab()) in page_mapcount(). > > Race window is tiny. For certain workload this happens around once a year. > > > > > > page:ffffea0105ca9380 count:1 mapcount:0 mapping:ffff88ff7712c180 index:0x0 compound_mapcount: 0 > > flags: 0x500000000008100(slab|head) > > raw: 0500000000008100 dead000000000100 dead000000000200 ffff88ff7712c180 > > raw: 0000000000000000 0000000080200020 00000001ffffffff 0000000000000000 > > page dumped because: VM_BUG_ON_PAGE(PageSlab(page)) > > ------------[ cut here ]------------ > > kernel BUG at ./include/linux/mm.h:628! > > invalid opcode: 0000 [#1] SMP NOPTI > > CPU: 77 PID: 504 Comm: kcompactd1 Tainted: G W 4.19.109-27 #1 > > Hardware name: Yandex T175-N41-Y3N/MY81-EX0-Y3N, BIOS R05 06/20/2019 > > RIP: 0010:isolate_migratepages_block+0x986/0x9b0 > > > > > > To fix just opencode page_mapcount() in racy check for 0-order case and > > recheck carefully under lru_lock when page cannot escape from lru. > > > > Also add checking extra references for file pages and swap cache. > > I dunno, this code looks quite nasty. I'm more thinking we should > revert and rethink David's 119d6d59dcc0980dcd58 ("mm, compaction: avoid > isolating pinned pages"). > > > --- a/mm/compaction.c > > +++ b/mm/compaction.c > > @@ -935,12 +935,16 @@ isolate_migratepages_block(struct compact_control *cc, unsigned long low_pfn, > > } > > > > /* > > - * Migration will fail if an anonymous page is pinned in memory, > > + * Migration will fail if an page is pinned in memory, > > * so avoid taking lru_lock and isolating it unnecessarily in an > > - * admittedly racy check. > > + * admittedly racy check simplest case for 0-order pages. > > + * > > + * Open code page_mapcount() to avoid VM_BUG_ON(PageSlab(page)). > > + * Page could have extra reference from mapping or swap cache. > > */ > > - if (!page_mapping(page) && > > - page_count(page) > page_mapcount(page)) > > + if (!PageCompound(page) && > > + page_count(page) > atomic_read(&page->_mapcount) + 1 + > > + (!PageAnon(page) || PageSwapCache(page))) > > goto isolate_fail; > > OK, we happened to notice this because we happened to trigger a > !PageSlab assertion. But if this page has been freed and reused for > slab, it could have been reused for *anything*? Perhaps it was reused > as a migratable page which we'll go ahead and migrate even though we no > longer should. There are various whacky parts of the kernel which > (ab)use surprising struct page fields in surprising ways - how do we > know it isn't one of those which now happens to look like a migratable > page? Here we just optimistically skip as much unwanted pages as possible. This code rechecks PageLRU and other tests later, under lru_lock. lru_lock blocks freeing path which should acquire it to remove from lru. > > I also worry about the next test: > > /* > * Only allow to migrate anonymous pages in GFP_NOFS context > * because those do not depend on fs locks. > */ > if (!(cc->gfp_mask & __GFP_FS) && page_mapping(page)) > goto isolate_fail; > > This page isn't PageLocked(), is it? It could be a recycled page which > is will be getting its ->mapping set one nanosecond hence. Yes, it's racy. I don't see how compaction rechecks this later. So it could try to unmap and migrate file page if race here with recycle. Probably nobody starts direct-compaction without GFP_FS. > > > > /* > > @@ -975,6 +979,11 @@ isolate_migratepages_block(struct compact_control *cc, unsigned long low_pfn, > > low_pfn += compound_nr(page) - 1; > > goto isolate_fail; > > } > > + > > + /* Recheck page extra references under lock */ > > + if (page_count(page) > page_mapcount(page) + > > + (!PageAnon(page) || PageSwapCache(page))) > > + goto isolate_fail; > > } > > > > lruvec = mem_cgroup_page_lruvec(page, pgdat); > > >