Received-SPF: softfail (google.com: domain of transitioning linux-kernel-owner@vger.kernel.org does not designate 23.128.96.19 as permitted sender) client-ip=23.128.96.19;
MIME-Version: 1.0
References: <1649704172-13181-1-git-send-email-quic_faiyazm@quicinc.com>
 <YlW2TO0O8qDHpkGW@kernel.org> <7b18bea8-b996-601d-f490-cb8aadfffa1b@quicinc.com>
In-Reply-To: <7b18bea8-b996-601d-f490-cb8aadfffa1b@quicinc.com>
From:   Rob Herring <robh@kernel.org>
Date:   Thu, 5 May 2022 10:31:05 -0500
Message-ID: <CAL_JsqKc07YZ=b5mKcw1+3Gtu+Le2H-gezAdnthh9JNyRfdngg@mail.gmail.com>
Subject: Re: [PATCH] mm: memblock: avoid to create memmap for memblock nomap regions
To:     Faiyaz Mohammed <quic_faiyazm@quicinc.com>
Cc:     Mike Rapoport <rppt@kernel.org>, quic_vjitta@quicinc.com,
        KarimAllah Ahmed <karahmed@amazon.de>,
        Quentin Perret <qperret@google.com>,
        Andrew Morton <akpm@linux-foundation.org>,
        linux-mm <linux-mm@kvack.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        Frank Rowand <frowand.list@gmail.com>,
        devicetree@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Thu, May 5, 2022 at 10:17 AM Faiyaz Mohammed
<quic_faiyazm@quicinc.com> wrote:
>
>
> On 4/12/2022 10:56 PM, Mike Rapoport wrote:
> > On Tue, Apr 12, 2022 at 12:39:32AM +0530, Faiyaz Mohammed wrote:
> >> This 'commit 86588296acbf ("fdt: Properly handle "no-map" field in the
> >> memory region")' is keeping the no-map regions in memblock.memory with
> >> MEMBLOCK_NOMAP flag set to use no-map memory for EFI using memblock api's,
> >> but during the initialization sparse_init mark all memblock.memory as
> >> present using for_each_mem_pfn_range, which is creating the memmap for
> >> no-map memblock regions. To avoid it skiping the memblock.memory regions
> >> set with MEMBLOCK_NOMAP set and with this change we will be able to save
> >> ~11MB memory for ~612MB carve out.
> > The MEMBLOCK_NOMAP is very fragile and caused a lot of issues already. I
> > really don't like the idea if adding more implicit assumptions about how
> > NOMAP memory may or may not be used in a generic iterator function.
>
> Sorry for delayed response.
> Yes, it is possible that implicit assumption can create
> misunderstanding. How about adding command line option and control the
> no-map region in fdt.c driver, to decide whether to keep "no-map" region
> with NOMAP flag or remove?. Something like below

No. That just added another dimension to the test matrix. Having
things from multiple sources is always a mess.

>
> --- a/drivers/of/fdt.c
> +++ b/drivers/of/fdt.c
> @@ -1180,8 +1180,10 @@ int __init __weak
> early_init_dt_reserve_memory_arch(phys_addr_t base,
>                  */
>                 if (memblock_is_region_reserved(base, size))
>                         return -EBUSY;
> -
> -               return memblock_mark_nomap(base, size);
> +               if (remove_nomap_region)
> +                       return memblock_remove(base, size);
> +               else
> +                       return memblock_mark_nomap(base, size);
> Thanks and regards,
> Mohammed Faiyaz
>