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=-6.6 required=3.0 tests=DKIM_SIGNED,DKIM_VALID, DKIM_VALID_AU,FREEMAIL_FORGED_FROMDOMAIN,FREEMAIL_FROM, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI,SIGNED_OFF_BY, SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=unavailable 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 698C0C3524D for ; Mon, 3 Feb 2020 23:17:44 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id F3C1D20838 for ; Mon, 3 Feb 2020 23:17:43 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="SnOMM6Gj" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org F3C1D20838 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=gmail.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=owner-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix) id A38756B0005; Mon, 3 Feb 2020 18:17:43 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 9E9E66B0007; Mon, 3 Feb 2020 18:17:43 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 8FEFC6B0008; Mon, 3 Feb 2020 18:17:43 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0120.hostedemail.com [216.40.44.120]) by kanga.kvack.org (Postfix) with ESMTP id 73FF56B0005 for ; Mon, 3 Feb 2020 18:17:43 -0500 (EST) Received: from smtpin23.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay01.hostedemail.com (Postfix) with ESMTP id 09D48180AD802 for ; Mon, 3 Feb 2020 23:17:43 +0000 (UTC) X-FDA: 76450380006.23.quiet33_7d8909121fe45 X-HE-Tag: quiet33_7d8909121fe45 X-Filterd-Recvd-Size: 6879 Received: from mail-io1-f66.google.com (mail-io1-f66.google.com [209.85.166.66]) by imf19.hostedemail.com (Postfix) with ESMTP for ; Mon, 3 Feb 2020 23:17:42 +0000 (UTC) Received: by mail-io1-f66.google.com with SMTP id m25so18747306ioo.8 for ; Mon, 03 Feb 2020 15:17:42 -0800 (PST) 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:content-transfer-encoding; bh=+tDvtzVGg2KSq7T5luQWCSg6VFxYPHqxgUAO4e7oxrs=; b=SnOMM6GjX6albBnFz8qLixcUOyXC8NzNWszG/RXLPpxwBZwdulnjgWIV7sUDv3dm1h 0vcdUuSS+bYM5GCByB42Vz1YnGDXJukXxHIy4FdfV+8MWXO6lwD1G7pwh7p/5c6Zzf2V 85pIMMPa7wtQdF0RMzn5M7yJDEfREvbumwZD1NoyyH0CLiRHg7stvIF6r+Dukr+4zPtH lB/pVI4cbP9dseg8oAaH9qbm6rFiPK52oYXjPvlIzcSoLgYc7wdcL8/1yNicnC5H+dzb BjMRBXfu9UEqfwHxBWYXhKEgWaLMVcdWNLKk940V3LKOIGxRKRszsTZdFloZOMKOUx+4 yL0g== 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:content-transfer-encoding; bh=+tDvtzVGg2KSq7T5luQWCSg6VFxYPHqxgUAO4e7oxrs=; b=Hg2ASRFzeezWZg85MBgfydafl6x4IEZxjdx1cWZPp3q2U00murBB2QbNrTDYex21/0 owc3V7z1h7WPDWwjsGPsAr+g8B1izAOERWMIfJTuQD996VnO7HaOtlf/miqsQPUDR8oG /HUQp0zVH5mb82jsDnKLrXFwU1Np1pH17I02KAptuQHLJZbDA0UFum+d0kPIAaNpwzHo KuHlaWib4tWr0wk7WvZ/ENFmwx8RyiS6hgqhUa1Jgy7SCoC+hOsZyD9gk5qMz4O4tyik OGsHuj+FtkyUvk6gebaDjJsBv66/I8SVb+vvnpCdsqxvXYYxSrCdVKepcPKfgnlCK74S AhtQ== X-Gm-Message-State: APjAAAXB+YVqxIFzI0ELv29pnwYM0wovg1YaoyXD31wXP0+YgubKlx3r neGz0+bYe2/WrtvnYZDuGu2epH6JnXSCFFrXl/I= X-Google-Smtp-Source: APXvYqxpRmooXuIi+SOWAl9WIAxQngPh9uM/6vZFn9xm/X9aWdsztOqz3x5u9XHoj/TYdn4FUaGWCtTP8N6OxnIZuHU= X-Received: by 2002:a6b:6e06:: with SMTP id d6mr20095773ioh.95.1580771861858; Mon, 03 Feb 2020 15:17:41 -0800 (PST) MIME-Version: 1.0 References: <1583F4CF-6CD8-4AB6-A2F6-60E6AEE5D5B2@redhat.com> In-Reply-To: <1583F4CF-6CD8-4AB6-A2F6-60E6AEE5D5B2@redhat.com> From: Alexander Duyck Date: Mon, 3 Feb 2020 15:17:30 -0800 Message-ID: Subject: Re: [PATCH v1 1/2] mm/page_alloc: fix and rework pfn handling in memmap_init_zone() To: David Hildenbrand Cc: LKML , linux-mm , Pavel Tatashin , Andrew Morton , Michal Hocko , Oscar Salvador , "Kirill A . Shutemov" Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable 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 Mon, Feb 3, 2020 at 1:44 PM David Hildenbrand wrote: > > > > > Am 03.02.2020 um 22:35 schrieb Alexander Duyck : > > > > =EF=BB=BFOn Mon, Jan 13, 2020 at 6:40 AM David Hildenbrand wrote: > >> > >> Let's update the pfn manually whenever we continue the loop. This make= s > >> the code easier to read but also less error prone (and we can directly > >> fix one issue). > >> > >> When overlap_memmap_init() returns true, pfn is updated to > >> "memblock_region_memory_end_pfn(r)". So it already points at the *next= * > >> pfn to process. Incrementing the pfn another time is wrong, we might > >> leave one uninitialized. I spotted this by inspecting the code, so I h= ave > >> no idea if this is relevant in practise (with kernelcore=3Dmirror). > >> > >> Fixes: a9a9e77fbf27 ("mm: move mirrored memory specific code outside o= f memmap_init_zone") > >> Cc: Pavel Tatashin > >> Cc: Andrew Morton > >> Cc: Michal Hocko > >> Cc: Oscar Salvador > >> Cc: Kirill A. Shutemov > >> Signed-off-by: David Hildenbrand > >> --- > >> mm/page_alloc.c | 9 ++++++--- > >> 1 file changed, 6 insertions(+), 3 deletions(-) > >> > >> diff --git a/mm/page_alloc.c b/mm/page_alloc.c > >> index a41bd7341de1..a92791512077 100644 > >> --- a/mm/page_alloc.c > >> +++ b/mm/page_alloc.c > >> @@ -5905,18 +5905,20 @@ void __meminit memmap_init_zone(unsigned long = size, int nid, unsigned long zone, > >> } > >> #endif > >> > >> - for (pfn =3D start_pfn; pfn < end_pfn; pfn++) { > >> + for (pfn =3D start_pfn; pfn < end_pfn; ) { > >> /* > >> * There can be holes in boot-time mem_map[]s handed to= this > >> * function. They do not exist on hotplugged memory. > >> */ > >> if (context =3D=3D MEMMAP_EARLY) { > >> if (!early_pfn_valid(pfn)) { > >> - pfn =3D next_pfn(pfn) - 1; > >> + pfn =3D next_pfn(pfn); > >> continue; > >> } > >> - if (!early_pfn_in_nid(pfn, nid)) > >> + if (!early_pfn_in_nid(pfn, nid)) { > >> + pfn++; > >> continue; > >> + } > >> if (overlap_memmap_init(zone, &pfn)) > >> continue; > >> if (defer_init(nid, pfn, end_pfn)) > > > > I'm pretty sure this is a bit broken. The overlap_memmap_init is going > > to return memblock_region_memory_end_pfn instead of the start of the > > next region. I think that is going to stick you in a mirrored region > > without advancing in that case. You would also need to have that case > > do a pfn++ before the continue; > > Thanks for having a look. > > Did you read the description regarding this change? Actually I hadn't read it all that closely, so my bad on that. The part that had caught my attention though was that memblock_region_memory_end is using PFN_DOWN to identify the end of the memory region, Given that we probably shouldn't be messing with the PFNs that may contain any of this memory it might make more sense to use memblock_region_reserved_end_pfn which uses PFN_UP so that we exclude all memory that is in the mirrored region just in case something doesn't end on a PFN aligned boundary. If we know that the mirrored region is going to always be page size aligned then I guess you are good to go. That was the only thing I wasn't sure about. Reviewed-by: Alexander Duyck