All the mail mirrored from lore.kernel.org
 help / color / mirror / Atom feed
From: DaeRo Lee <skseofh@gmail.com>
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>
Subject: Re: [PATCH v2] memblock: add no-map alloc functions
Date: Sat, 27 Apr 2024 19:24:23 +0900	[thread overview]
Message-ID: <CAATEi5=Z0qirM-fyGJL_UPcr7-iyCFtOW9d3XsdN50Tkhpm0iA@mail.gmail.com> (raw)
In-Reply-To: <Ziy8AsAGZyKCyXX_@kernel.org>

2024년 4월 27일 (토) 오후 5:50, Mike Rapoport <rppt@kernel.org>님이 작성:
>
> On Fri, Apr 19, 2024 at 10:59:52AM +0900, DaeRo Lee wrote:
> > 2024년 4월 19일 (금) 오전 10:46, DaeRo Lee <skseofh@gmail.com>님이 작성:
> > >
> > > 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 right.
> >
> > > -> 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
>
> NOMAP and memblock.reserved are semantically different, and at makes sense
> to have a "reserved nomap" node in fdt recorded in both memblock.memory and
> memblock.reserved.
>
> memblock.reserved represents the memory that is used by firmware or early
> kernel allocation, so reserved memory in fdt should be reserved in memblock
> as well. I believe it's an oversight that early_init_dt_reserve_memory()
> does not call memblock_reserve() for nomap memory.
>
> NOMAP is a property of a memory region that says that that region should
> 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.

Regards,.
DaeRo Lee

  reply	other threads:[~2024-04-27 10:24 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-04-16 12:06 [PATCH v2] memblock: add no-map alloc functions skseofh
2024-04-16 12:06 ` skseofh
2024-04-17  2:23   ` Wei Yang
2024-04-17  6:02   ` Mike Rapoport
2024-04-18 14:54     ` DaeRo Lee
2024-04-18 18:03       ` Mike Rapoport
2024-04-19  1:46         ` DaeRo Lee
2024-04-19  1:59           ` DaeRo Lee
2024-04-27  2:42             ` DaeRo Lee
2024-04-27  3:27               ` DaeRo Lee
2024-04-27  8:49             ` Mike Rapoport
2024-04-27 10:24               ` DaeRo Lee [this message]
2024-04-28  6:33                 ` Mike Rapoport
2024-04-28 10:36                   ` DaeRo Lee
2024-04-28 12:00                     ` Mike Rapoport
2024-04-28 12:52                       ` DaeRo Lee

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CAATEi5=Z0qirM-fyGJL_UPcr7-iyCFtOW9d3XsdN50Tkhpm0iA@mail.gmail.com' \
    --to=skseofh@gmail.com \
    --cc=akpm@linux-foundation.org \
    --cc=daero_le.lee@samsung.com \
    --cc=devicetree@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=robh@kernel.org \
    --cc=rppt@kernel.org \
    --cc=saravanak@google.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.