Received: by 10.213.65.68 with SMTP id h4csp17446imn; Thu, 15 Mar 2018 08:16:33 -0700 (PDT) X-Google-Smtp-Source: AG47ELtMsSZWy0DQ7tn1vYo/o7SlswKg/6KiHZZI0X+ejNxr/yhNeZO+hxZ9KweiyEeaMBGT5o/D X-Received: by 2002:a17:902:51e7:: with SMTP id y94-v6mr518768plh.214.1521126993080; Thu, 15 Mar 2018 08:16:33 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1521126993; cv=none; d=google.com; s=arc-20160816; b=PCP0WOCz8UGz/4jsAxHXIxzVJcjsIey0cSzHLdWbY8AmkJpW6ExwnhDq61G1sQWq60 SOwGnQDMqhCIE3Qlb0kP6kjR2/WKrZNrhdKMq4qH5m8+MiLCJigzB82SfQpA4gKqcuuL T5tvZsnWPAhfpkKQ0qrvFCaArxwNz4+4TY7blN7AGzw8nx5jfJxNcGiuxQMVx/dufxT3 VwMoZ37H3rDAvxyN22yDZoJWtPBnML7zMu/OnsZTuLKGCuRYIojdxgOYt73sIT5fADPF rzc1D+0IXn2fcsI4o1HA3HW5w9AdlKypW/Pjpof5EfbkHdHu2wZQ3aWoG/mLwTsO9kB1 jmLA== 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 :references:in-reply-to:mime-version:arc-authentication-results; bh=846uljRPO0x5gnY6P10aahrS+NKfs9htPlgCiSIBED4=; b=vwoW5//YImVY8bsb0cK69IRpno52vw+sYE7oKYbGSjmFCm8XRwKC14btS3oe48rJku BP0EgjoF8e+3PoQ+dtCv5b5TtBhJiC1m/i7wR5O+kMtPKM3EL3u58F+VSwU+B7llmpft JnYAvKZdX60Wa5lEix8pZCLsSU8SjcgkJyWRqDjZtvyFBA/pm03n8gEY4wMRKzj0+p9T TaFgBx1WMkCJHerGAXWCzwBxwIwht2ieMge3ej53Pph/++kxI6+Fr4l/9LZGdlQIoOYu xX0VHo6ZQWQju7tNcUfe19ILxFynbFcgOfrA367DqTzzJ/IJINzYLKRttiaUM5QbH4N2 xLEA== ARC-Authentication-Results: i=1; mx.google.com; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=fail (p=NONE sp=NONE dis=NONE) header.from=redhat.com Return-Path: Received: from vger.kernel.org (vger.kernel.org. [209.132.180.67]) by mx.google.com with ESMTP id d7-v6si4163486plm.539.2018.03.15.08.16.18; Thu, 15 Mar 2018 08:16:33 -0700 (PDT) Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67; Authentication-Results: mx.google.com; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=fail (p=NONE sp=NONE dis=NONE) header.from=redhat.com Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752712AbeCOPMW (ORCPT + 99 others); Thu, 15 Mar 2018 11:12:22 -0400 Received: from mail-oi0-f65.google.com ([209.85.218.65]:32852 "EHLO mail-oi0-f65.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751502AbeCOPMV (ORCPT ); Thu, 15 Mar 2018 11:12:21 -0400 Received: by mail-oi0-f65.google.com with SMTP id e9so5995104oii.0 for ; Thu, 15 Mar 2018 08:12:21 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc; bh=846uljRPO0x5gnY6P10aahrS+NKfs9htPlgCiSIBED4=; b=hxg0NIAeLO/d49USLU5o0bfswtLAQbKjqeDjSLojWsERrgeVs7tZrhNKQLaUkI3e2c 14Lcwis93y62B7gs5c4SW8mC+C3wVuQoCSl2s4K+N2fTqAj6FaShlWJaf0KCLDgRnAEO iJzmUBCo11GGGjwJu28FgwaIOwof/Puj0nNl8RLMhhW65rXvhhQbrUhNdVach0+ogtXw w/SV9Tl9ecrBmX9W0zWka0G+s1wUBHMJRIeQKYXXyARVeKeeH614xfZ2EYejq8E5jyTA 9a5ONSIT7PQD9Itu+Xj97C7toMQwRIcBaX9fT//OKMgQ1oB4rGhskAXDKun7WM1gdzEv i8fw== X-Gm-Message-State: AElRT7FjlglSAJnuJwtwadRsO4nKzBD9fC7zqjGxOFpKZf6KwBkePhhI 95oR9mc4YInDOr2LgKF5CJMjcG0dkoXLQIwgJatGAw== X-Received: by 10.202.236.130 with SMTP id k124mr5270171oih.33.1521126740845; Thu, 15 Mar 2018 08:12:20 -0700 (PDT) MIME-Version: 1.0 Received: by 2002:a9d:39f6:0:0:0:0:0 with HTTP; Thu, 15 Mar 2018 08:12:20 -0700 (PDT) In-Reply-To: References: <20180314192937.12888-1-ard.biesheuvel@linaro.org> From: Daniel Vacek Date: Thu, 15 Mar 2018 16:12:20 +0100 Message-ID: Subject: Re: [PATCH v2] Revert "mm/page_alloc: fix memmap_init_zone pageblock alignment" To: Ard Biesheuvel Cc: linux-arm-kernel , open list , Mark Rutland , Will Deacon , Catalin Marinas , Marc Zyngier , Mel Gorman , Michal Hocko , Paul Burton , Pavel Tatashin , Vlastimil Babka , Andrew Morton , Linus Torvalds 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 Thu, Mar 15, 2018 at 8:45 AM, Ard Biesheuvel wrote: > On 15 March 2018 at 07:44, Daniel Vacek wrote: >> On Thu, Mar 15, 2018 at 8:36 AM, Ard Biesheuvel >> wrote: >>> On 15 March 2018 at 02:23, Daniel Vacek wrote: >>>> On Wed, Mar 14, 2018 at 8:29 PM, Ard Biesheuvel >>>> wrote: >>>>> This reverts commit 864b75f9d6b0100bb24fdd9a20d156e7cda9b5ae. >>>>> >>>>> Commit 864b75f9d6b0 ("mm/page_alloc: fix memmap_init_zone pageblock >>>>> alignment") modified the logic in memmap_init_zone() to initialize >>>>> struct pages associated with invalid PFNs, to appease a VM_BUG_ON() >>>>> in move_freepages(), which is redundant by its own admission, and >>>>> dereferences struct page fields to obtain the zone without checking >>>>> whether the struct pages in question are valid to begin with. >>>>> >>>>> Commit 864b75f9d6b0 only makes it worse, since the rounding it does >>>>> may cause pfn assume the same value it had in a prior iteration of >>>>> the loop, resulting in an infinite loop and a hang very early in the >>>>> boot. Also, since it doesn't perform the same rounding on start_pfn >>>>> itself but only on intermediate values following an invalid PFN, we >>>>> may still hit the same VM_BUG_ON() as before. >>>>> >>>>> So instead, let's fix this at the core, and ensure that the BUG >>>>> check doesn't dereference struct page fields of invalid pages. >>>>> >>>>> Fixes: 864b75f9d6b0 ("mm/page_alloc: fix memmap_init_zone pageblock alignment") >>>>> Cc: Daniel Vacek >>>>> Cc: Mel Gorman >>>>> Cc: Michal Hocko >>>>> Cc: Paul Burton >>>>> Cc: Pavel Tatashin >>>>> Cc: Vlastimil Babka >>>>> Cc: Andrew Morton >>>>> Cc: Linus Torvalds >>>>> Signed-off-by: Ard Biesheuvel >>>>> --- >>>>> mm/page_alloc.c | 13 +++++-------- >>>>> 1 file changed, 5 insertions(+), 8 deletions(-) >>>>> >>>>> diff --git a/mm/page_alloc.c b/mm/page_alloc.c >>>>> index 3d974cb2a1a1..635d7dd29d7f 100644 >>>>> --- a/mm/page_alloc.c >>>>> +++ b/mm/page_alloc.c >>>>> @@ -1910,7 +1910,9 @@ static int move_freepages(struct zone *zone, >>>>> * Remove at a later date when no bug reports exist related to >>>>> * grouping pages by mobility >>>>> */ >>>>> - VM_BUG_ON(page_zone(start_page) != page_zone(end_page)); >>>>> + VM_BUG_ON(pfn_valid(page_to_pfn(start_page)) && >>>>> + pfn_valid(page_to_pfn(end_page)) && >>>>> + page_zone(start_page) != page_zone(end_page)); >>>> >>>> Hi, I am on vacation this week and I didn't have a chance to test this >>>> yet but I am not sure this is correct. Generic pfn_valid() unlike the >>>> arm{,64} arch specific versions returns true for all pfns in a section >>>> if there is at least some memory mapped in that section. So I doubt >>>> this prevents the crash I was targeting. I believe pfn_valid() does >>>> not change a thing here :( >>>> >>> >>> If this is the case, memblock_next_valid_pfn() is broken since it >>> skips valid PFNs, and we should be fixing that instead. >> >> How do you define valid pfn? Maybe the generic version of pfn_valid() >> should be fixed??? >> > > memblock_next_valid_pfn() skips PFNs for which pfn_valid() returns > true. That is clearly a bug. So pfn_valid() does not mean this frame is usable memory? OK, in that case we need to fix or revert memblock_next_valid_pfn(). That works for me. --nX