Received-SPF: pass (google.com: domain of linux-kernel+bounces-150911-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45e3:2400::1 as permitted sender) client-ip=2604:1380:45e3:2400::1;
Precedence: bulk
MIME-Version: 1.0
References: <linux-mm@kvack.org> <20240416120635.361838-1-skseofh@gmail.com>
 <20240416120635.361838-2-skseofh@gmail.com> <Zh9l_LpThq9aFUR7@kernel.org>
 <CAATEi5kywwC2yUaYjgs+Gm=4HM5o=KHTqH1ALKJijWE_gge0=g@mail.gmail.com> <ZiFgYWydIwvnpIIY@kernel.org>
In-Reply-To: <ZiFgYWydIwvnpIIY@kernel.org>
From: DaeRo Lee <skseofh@gmail.com>
Date: Fri, 19 Apr 2024 10:46:10 +0900
Message-ID: <CAATEi5kFt8iUeWSkrj_bVTyPO_tfQzG77D719P5dLsr2j6Zkzw@mail.gmail.com>
Subject: Re: [PATCH v2] memblock: add no-map alloc functions
To: Mike Rapoport <rppt@kernel.org>
Cc: robh@kernel.org, saravanak@google.com, akpm@linux-foundation.org, 
	devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, linux-mm@kvack.org, 
	Daero Lee <daero_le.lee@samsung.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

2024=EB=85=84 4=EC=9B=94 19=EC=9D=BC (=EA=B8=88) =EC=98=A4=EC=A0=84 3:04, M=
ike Rapoport <rppt@kernel.org>=EB=8B=98=EC=9D=B4 =EC=9E=91=EC=84=B1:
>
> On Thu, Apr 18, 2024 at 11:54:15PM +0900, DaeRo Lee wrote:
> > 2024=EB=85=84 4=EC=9B=94 17=EC=9D=BC (=EC=88=98) =EC=98=A4=ED=9B=84 3:0=
3, Mike Rapoport <rppt@kernel.org>=EB=8B=98=EC=9D=B4 =EC=9E=91=EC=84=B1:
> > >
> > > On Tue, Apr 16, 2024 at 09:06:35PM +0900, skseofh@gmail.com wrote:
> > > > From: Daero Lee <daero_le.lee@samsung.com>
> > > >
> > > > Like reserved-memory with the 'no-map' property and only 'size' pro=
perty
> > > > (w/o 'reg' property), there are memory regions need to be allocated=
 in
> > > > memblock.memory marked with the MEMBLOCK_NOMAP flag, but should not=
 be
> > > > allocated in memblock.reserved.
> > >
> > > This still does not explain why you need such regions.
> > >
> > > As Wei Yang explained, memblock does not allocate memory from
> > > memblock.reserved. The memblock.reserved array represents memory that=
 is in
> > > use by firmware or by early kernel allocations and cannot be freed to=
 page
> > > allocator.
> > Thank you for your comments. I used the wrong word.
> > When I use 'allocate', I mean that the region 'adds' to the memblock.re=
served.
> >
> > >
> > > If you have a region that's _NOMAP in memblock.memory and is absent i=
n
> > > memblock.reserved it will not be mapped by the kernel page tables, bu=
t it
> > > will be considered as free memory by the core mm.
> > >
> > > Is this really what you want?
> > If my understanding is right, before freeing (memory && !reserved)
> > area, we marked the memblock.reserved regions and memblock.memory
> > regions with no-map flag. And when we free (memory && !reserved) area,
> > we skip the memblock.memory regions with no-map(see
> > should_skip_region). So, I think that the memory regions with no-map
> > flag will not be considered as free memory.
>
> You are right here.
>
> But I still don't understand *why* do you want to change the way
> early_init_dt_alloc_reserved_memory_arch() works.

In memmap_init_reserved_pages, we mark memblock.reserved as
PageReserved first and mark the memblock.reserved with nomap flag
also.
-> Isn't this duplicated work? (If we add no-map region to
memblock.reserved 'and' mark in memblock.memory..)
So, I think that for the no-map region, we don't need to add to the
memblock.reserved.
This is what we do now in early_init_dt_reserve_memory. the nomap
region is not added to the memblock.reserved.

In early_init_dt_alloc_reserved_memory_arch, if 'nomap' is true, we
mark the memblock.memory region as _NOMAP. And if the return value
'err' is not zero(which is '-ENOMEM' from memblock_isolate_range), we
free the region.
- 'nomap' is true -> memblock_mark_nomap : success -> not free the region

: fail -> free the region
And it can be said that we add the region to the memblock.reserved
using memblock_phys_alloc_range and if the region is nomap, then we
can free the region from memblock.reserved. But is it necessary to add
it to memblock.reserved? We just need the region in memblock.memory to
mark nomap.

So, here is what I think:
- reserved-memory w/ nomap region -> mark only to memblock.memory
- reserved-memory w/o nomap region -> add to the memblock.reserved

Regards,
DaeRo Lee