Received-SPF: pass (google.com: domain of linux-kernel+bounces-161374-linux.lists.archive=gmail.com@vger.kernel.org designates 139.178.88.99 as permitted sender) client-ip=139.178.88.99;
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> <CAATEi5kFt8iUeWSkrj_bVTyPO_tfQzG77D719P5dLsr2j6Zkzw@mail.gmail.com>
 <CAATEi5ksY-v7-LEqNZWFV5hsHiegNEtrh4LpMWOQ=vT7hC0Rng@mail.gmail.com>
 <Ziy8AsAGZyKCyXX_@kernel.org> <CAATEi5=Z0qirM-fyGJL_UPcr7-iyCFtOW9d3XsdN50Tkhpm0iA@mail.gmail.com>
 <Zi3twYLGvhtJa9Yh@kernel.org>
In-Reply-To: <Zi3twYLGvhtJa9Yh@kernel.org>
From: DaeRo Lee <skseofh@gmail.com>
Date: Sun, 28 Apr 2024 19:36:40 +0900
Message-ID: <CAATEi5nOQE7xi5ztV0BFO6MRqSGwUPT4V9dqpMT+p4r7iZQwRQ@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 28=EC=9D=BC (=EC=9D=BC) =EC=98=A4=ED=9B=84 3:35, M=
ike Rapoport <rppt@kernel.org>=EB=8B=98=EC=9D=B4 =EC=9E=91=EC=84=B1:
>
> On Sat, Apr 27, 2024 at 07:24:23PM +0900, DaeRo Lee wrote:
> > 2024=EB=85=84 4=EC=9B=94 27=EC=9D=BC (=ED=86=A0) =EC=98=A4=ED=9B=84 5:5=
0, Mike Rapoport <rppt@kernel.org>=EB=8B=98=EC=9D=B4 =EC=9E=91=EC=84=B1:
> > >
> > > On Fri, Apr 19, 2024 at 10:59:52AM +0900, DaeRo Lee wrote:
> > > > 2024=EB=85=84 4=EC=9B=94 19=EC=9D=BC (=EA=B8=88) =EC=98=A4=EC=A0=84=
 10:46, DaeRo Lee <skseofh@gmail.com>=EB=8B=98=EC=9D=B4 =EC=9E=91=EC=84=B1:
> > > > >
> > > > > In memmap_init_reserved_pages, we mark memblock.reserved as
> > > > > PageReserved first and mark the memblock.reserved with nomap flag
> > > > > also.
> > > > Sorry. This is my mistake. 'memblock.memory with nomap flag' is rig=
ht.
> > > >
> > > > > -> 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 t=
he
> > > > > 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 valu=
e
> > > > > 'err' is not zero(which is '-ENOMEM' from memblock_isolate_range)=
, we
> > > > > free the region.
> > > > > - 'nomap' is true -> memblock_mark_nomap : success -> not free th=
e region
> > > > >
> > > > > : fail -> free the region
> > > > > And it can be said that we add the region to the memblock.reserve=
d
> > > > > using memblock_phys_alloc_range and if the region is nomap, then =
we
> > > > > can free the region from memblock.reserved. But is it necessary t=
o add
> > > > > it to memblock.reserved? We just need the region in memblock.memo=
ry 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.reserve=
d
> > >
> > > NOMAP and memblock.reserved are semantically different, and at makes =
sense
> > > to have a "reserved nomap" node in fdt recorded in both memblock.memo=
ry and
> > > memblock.reserved.
> > >
> > > memblock.reserved represents the memory that is used by firmware or e=
arly
> > > kernel allocation, so reserved memory in fdt should be reserved in me=
mblock
> > > as well. I believe it's an oversight that early_init_dt_reserve_memor=
y()
> > > does not call memblock_reserve() for nomap memory.
> > >
> > > NOMAP is a property of a memory region that says that that region sho=
uld
> > > not be mapped in the linear map, it's not necessarily in use.
> >
> > I agree that the NOMAP region should be added to memblock.reserved.
> >
> > So, I think we need to clean-up memmap_init_reserved_pages, because in
> > this function we call reserve_bootmem_region for memblock.reserved and
> > memblock.memory with nomap. We don't need to call
> > reserve_bootmem_region for nomap.
>
> Read the comment about memblock_mark_nomap()
I read the comment about memblock_mark_nomap() and understood that
regions with nomap flags should be treated as PageReserved.
But, if we add this nomap region to memblock.reserved, the region with
nomap flag will be processed in the first for-loop in
memmap_init_reserved_pages.

Am I thinking wrong?

Regards,
DaeRo Lee