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=-3.8 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,SPF_HELO_NONE, SPF_PASS autolearn=no 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 49457C433DB for ; Wed, 6 Jan 2021 20:03:05 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id B2CBF22BEA for ; Wed, 6 Jan 2021 20:03:04 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org B2CBF22BEA Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=intel.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=owner-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix) id 276DE6B02CE; Wed, 6 Jan 2021 15:03:04 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 1FE9E6B02D0; Wed, 6 Jan 2021 15:03:04 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 115516B02D1; Wed, 6 Jan 2021 15:03:04 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0005.hostedemail.com [216.40.44.5]) by kanga.kvack.org (Postfix) with ESMTP id E95DD6B02CE for ; Wed, 6 Jan 2021 15:03:03 -0500 (EST) Received: from smtpin10.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay02.hostedemail.com (Postfix) with ESMTP id B41843637 for ; Wed, 6 Jan 2021 20:03:03 +0000 (UTC) X-FDA: 77676423846.10.wheel47_050f0d3274e4 Received: from filter.hostedemail.com (10.5.16.251.rfc1918.com [10.5.16.251]) by smtpin10.hostedemail.com (Postfix) with ESMTP id 965B716A0D2 for ; Wed, 6 Jan 2021 20:03:03 +0000 (UTC) X-HE-Tag: wheel47_050f0d3274e4 X-Filterd-Recvd-Size: 7927 Received: from mail-ej1-f52.google.com (mail-ej1-f52.google.com [209.85.218.52]) by imf49.hostedemail.com (Postfix) with ESMTP for ; Wed, 6 Jan 2021 20:03:02 +0000 (UTC) Received: by mail-ej1-f52.google.com with SMTP id w1so6605369ejf.11 for ; Wed, 06 Jan 2021 12:03:02 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=intel-com.20150623.gappssmtp.com; s=20150623; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=Qo74SNxBRzE7jEcHLiGlvWl2XU+3/X3BlAwXU5Tlukc=; b=v6wWef2R9dxJyWPfJ27SmbgkvLAgJTLJrR8TGAZY0TVKm5mjjrpyGwFC1+p+wEjk3K bx80Q1irXT/aw/6p0iFQGF9LdAzwGpc+WHZ/uV+oFrVOOpPxImw8wSy0gCakoYhmLYRj vu+uiZW2GjsF2XsB09pGc9nHgVUd/9aHwAEw42JaGyF7p1TAAXFCmp9rx0epW1387AaS 0bo2Zldk52ppJRBpy2IUeDFr3DPE6XOEkrOFrJzP60LqeffanhcYIvWtE7PdKyR0rd3b AGJGT8viZOu6JQOD4r/7UtXTooi48AHGT6o2SzubUEIuRh+Ma7hyk71x3dktuEQHol6t rUHQ== 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=Qo74SNxBRzE7jEcHLiGlvWl2XU+3/X3BlAwXU5Tlukc=; b=pJVLqBh/QdZLm/7fuaaFTBLES4uuThP5texblrpGMi0wMVnXl4sPni9Czq2r2hq6OU L/sX9ZiVc9qk5+2S64GkslGxlQei0vtQufwoO+T1DasVIL+XR65EOMePrOIA99SOxLYJ E2+iINNXWtcyhSIaYzMWyPsh42nhb5Ya355R6sXBIFu2U8sE4OGjgRx6jNx8D29cL/ku /VRIpL0YDCNN1NWRkC1N0nBgUYTikrV6vC1VXcIQU0Ncgx3stIrmguqB4tyASGf9dWy9 Sk54hq8PL4wVwbPfk6I7jyxh81i//LNC/MwMWblDW1/U6eZHGQBcBcyao3NrlknwYqqR jurA== X-Gm-Message-State: AOAM530Xf76Rv6mUfvztQxb9LlLuI9YZJkHGciJIGoc0jgl9wb0wj5zV PypsouQlECHjzJ+xu/QduPHr6uxGNb1CEhZCDfIAaA== X-Google-Smtp-Source: ABdhPJz/eZOahwvUGIhOCo6IyZBDGJON3yP3sTwT45bJBjf/sI20Gv0PBTTRmCDg9BekHP/oVX9aYsXHKIcRN7kQ5wE= X-Received: by 2002:a17:906:2707:: with SMTP id z7mr4030704ejc.418.1609963381149; Wed, 06 Jan 2021 12:03:01 -0800 (PST) MIME-Version: 1.0 References: <160990599013.2430134.11556277600719835946.stgit@dwillia2-desk3.amr.corp.intel.com> <785b9095-eca4-8100-33ea-6ae84e02a92e@redhat.com> <20210106104255.GK13207@dhcp22.suse.cz> <7d7c5dc4-7784-5dcc-fc00-4fe99f0a4a90@redhat.com> In-Reply-To: <7d7c5dc4-7784-5dcc-fc00-4fe99f0a4a90@redhat.com> From: Dan Williams Date: Wed, 6 Jan 2021 12:02:49 -0800 Message-ID: Subject: Re: [PATCH] mm: Teach pfn_to_online_page() about ZONE_DEVICE section collisions To: David Hildenbrand Cc: Michal Hocko , Linux MM , Andrew Morton , Linux Kernel Mailing List Content-Type: text/plain; charset="UTF-8" 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 6, 2021 at 3:23 AM David Hildenbrand wrote: > > On 06.01.21 11:42, Michal Hocko wrote: > > On Wed 06-01-21 10:56:19, David Hildenbrand wrote: > > [...] > >> Note that this is not sufficient in the general case. I already > >> mentioned that we effectively override an already initialized memmap. > >> > >> --- > >> > >> [ SECTION ] > >> Before: > >> [ ZONE_NORMAL ][ Hole ] > >> > >> The hole has some node/zone (currently 0/0, discussions ongoing on how > >> to optimize that to e.g., ZONE_NORMAL in this example) and is > >> PG_reserved - looks like an ordinary memory hole. > >> > >> After memremap: > >> [ ZONE_NORMAL ][ ZONE_DEVICE ] > >> > >> The already initialized memmap was converted to ZONE_DEVICE. Your > >> slowpath will work. > >> > >> After memunmap (no poisioning): > >> [ ZONE_NORMAL ][ ZONE_DEVICE ] > >> > >> The slow path is no longer working. pfn_to_online_page() might return > >> something that is ZONE_DEVICE. > >> > >> After memunmap (poisioning): > >> [ ZONE_NORMAL ][ POISONED ] > >> > >> The slow path is no longer working. pfn_to_online_page() might return > >> something that will BUG_ON via page_to_nid() etc. > >> > >> --- > >> > >> Reason is that pfn_to_online_page() does no care about sub-sections. And > >> for now, it didn't had to. If there was an online section, it either was > >> > >> a) Completely present. The whole memmap is initialized to sane values. > >> b) Partially present. The whole memmap is initialized to sane values. > >> > >> memremap/memunmap messes with case b) > > > > I do not see we ever clear the newly added flag and my understanding is > > that the subsection removed would lead to get_dev_pagemap returning a > > NULL. Which would obviously need to be checked for pfn_to_online_page. > > Or do I miss anything and the above is not the case and we could still > > get false positives? > > See my example above ("After memunmap"). > > We're still in the slow pathg. pfn_to_online_page() will return a struct > page as get_dev_pagemap() is now NULL. > > Yet page_zone(page) will either > - BUG_ON (memmap was poisoned) > - return ZONE_DEVICE zone (memmap not poisoned when memunmapping) > > As I said, can be tackled by checking for pfn_section_valid() at least > on the slow path. Ideally also on the fast path. Good eye, I glazed over that the existing pfn_section_valid() check in pfn_valid() is obviated by early_section(). I'll respin with a standalone pfn_section_valid() gate in pfn_to_online_page(). > > > > >> Well have to further tweak pfn_to_online_page(). You'll have to also > >> check pfn_section_valid() *at least* on the slow path. Less-hacky would > >> be checking it also in the "somehwat-faster" path - that would cover > >> silently overriding a memmap that's visible via pfn_to_online_page(). > >> Might slow down things a bit. > >> > >> > >> Not completely opposed to this, but I would certainly still prefer just > >> avoiding this corner case completely instead of patching around it. Thanks! > > > > Well, I would love to have no surprises either. So far there was not > > actual argument why the pmem reserved space cannot be fully initialized. > > Yes, I'm still hoping Dan can clarify that. Complexity and effective utility (once pfn_to_online_page() is fixed) are the roadblocks in my mind. The altmap is there to allow for PMEM capacity to be used as memmap space, so there would need to be code to break that circular dependency and allocate a memmap for the metadata space from DRAM and the rest of the memmap space for the data capacity from pmem itself. That memmap-for-pmem-metadata will still represent offline pages. So once pfn_to_online_page() is fixed, what pfn-walker is going to be doing pfn_to_page() on PMEM metadata? Secondly, there is a PMEM namespace mode called "raw" that eschews DAX and 'struct page' for pmem and just behaves like a memory-backed block device. The end result is still that pfn walkers need to discover if a PMEM pfn has a page, so I don't see what "sometimes there's an memmap-for-pmem-metadata" buys us? > > > On the other hand making sure that pfn_to_online_page sounds like the > > right thing to do. And having an explicit check for zone device there in > > a slow path makes sense to me. > > As I said, I'd favor to simplify and just get rid of the special case, > instead of coming up with increasingly complex ways to deal with it. > pfn_to_online_page() used to be simple, essentially checking a single > flag was sufficient in most setups. I think the logic to throw away System RAM that might collide with PMEM and soft-reserved memory within a section is on the order of the same code complexity as the patch proposed here, no? Certainly the throw-away concept itself is easier to grasp, but I don't think that would be reflected in the code patch to achieve it... willing to be proved wrong with a patch.