On Mon, Jun 10, 2024 at 02:54:57PM +0000, Wei Yang wrote:
> On Mon, Jun 10, 2024 at 09:40:33AM +0300, Mike Rapoport wrote:
> >On Wed, Jun 05, 2024 at 01:07:42AM +0000, Wei Yang wrote:
> >> Function deferred_[init|free]_pages are only used in
> >> deferred_init_maxorder(), which makes sure the range to init/free is
> >> within MAX_ORDER_NR_PAGES size.
> >>
> >> With this knowledge, we can simplify these two functions. Since
> >>
> >> * only the first pfn could be IS_MAX_ORDER_ALIGNED()
> >
> >No, the first pfn is not necessarily IS_MAX_ORDER_ALIGNED(). Start pfn is a
> >beginning of a region in memblock.memory, and there's no guarantee on it's
> >alignment.
> >
>
> Yes, I mean only the first pfn is possible to be IS_MAX_ORDER_ALIGNED(), not
> must be IS_MAX_ORDER_ALIGNED().
>
> The range passed to deferred_[init|free]_pages must be within one
> MAX_ORDER_NR_PAGES. If the first pfn is not IS_MAX_ORDER_ALIGNED(), others
> could not be IS_MAX_ORDER_ALIGNED().
>
> Currently these two functions would iterate all pfn, and check
> IS_MAX_ORDER_ALIGNED() to break init|free on each MAX_ORDER_NR_PAGES. But this
> is only possible and necessary on the first pfn.
Thinking more about it, it looks to me that deferred_pfn_valid() cannot
ever return false.
The ranges that are passed to deferred_{free,init}_pages are always from
memblock.memory and we allocate the memory map to cover memblock.memory
with at least MAX_ORDER_NR_PAGES alignment.
So I don't see how pfn_valid() in deferred_pfn_valid() may ever return
false.
> >> Signed-off-by: Wei Yang <richard.weiyang@xxxxxxxxx>
> >> CC: Kirill A. Shutemov <kirill.shutemov@xxxxxxxxxxxxxxx>
> >> CC: Mike Rapoport (IBM) <rppt@xxxxxxxxxx>
> >> CC: David Hildenbrand <david@xxxxxxxxxx>
> >>
> >> ---
> >> But my question is why we just test pfn_valid for the
> >> IS_MAX_ORDER_ALIGNED pfn? I thought we should test pfn_valid for each
> >> pfn until the first one in MAX_ORDER pages. Do I miss something?
> >> ---
> >> mm/mm_init.c | 43 ++++++++++++++-----------------------------
> >> 1 file changed, 14 insertions(+), 29 deletions(-)
> >>
> >> diff --git a/mm/mm_init.c b/mm/mm_init.c
> >> index bbaf3a2c1cfd..6a4adf9e7d9a 100644
> >> --- a/mm/mm_init.c
> >> +++ b/mm/mm_init.c
> >> @@ -1970,21 +1970,10 @@ static inline bool __init deferred_pfn_valid(unsigned long pfn)
> >> static void __init deferred_free_pages(unsigned long pfn,
> >> unsigned long end_pfn)
> >> {
> >> - unsigned long nr_free = 0;
> >> -
> >> - for (; pfn < end_pfn; pfn++) {
> >> - if (!deferred_pfn_valid(pfn)) {
> >> - deferred_free_range(pfn - nr_free, nr_free);
> >> - nr_free = 0;
> >> - } else if (IS_MAX_ORDER_ALIGNED(pfn)) {
> >> - deferred_free_range(pfn - nr_free, nr_free);
> >> - nr_free = 1;
> >> - } else {
> >> - nr_free++;
> >> - }
> >> - }
> >> - /* Free the last block of pages to allocator */
> >> - deferred_free_range(pfn - nr_free, nr_free);
> >> + if (!deferred_pfn_valid(pfn))
> >> + pfn++;
> >> +
> >> + deferred_free_range(pfn, end_pfn - pfn);
> >> }
> >>
> >> /*
> >> @@ -1992,27 +1981,23 @@ static void __init deferred_free_pages(unsigned long pfn,
> >> * by performing it only once every MAX_ORDER_NR_PAGES.
> >> * Return number of pages initialized.
> >> */
> >> -static unsigned long __init deferred_init_pages(struct zone *zone,
> >> - unsigned long pfn,
> >> - unsigned long end_pfn)
> >> +static unsigned long __init deferred_init_pages(struct zone *zone,
> >> + unsigned long pfn,
> >> + unsigned long end_pfn)
> >> {
> >> int nid = zone_to_nid(zone);
> >> unsigned long nr_pages = 0;
> >> int zid = zone_idx(zone);
> >> struct page *page = NULL;
> >>
> >> - for (; pfn < end_pfn; pfn++) {
> >> - if (!deferred_pfn_valid(pfn)) {
> >> - page = NULL;
> >> - continue;
> >> - } else if (!page || IS_MAX_ORDER_ALIGNED(pfn)) {
> >> - page = pfn_to_page(pfn);
> >> - } else {
> >> - page++;
> >> - }
> >> + if (!deferred_pfn_valid(pfn))
> >> + pfn++;
> >> +
> >> + page = pfn_to_page(pfn);
> >> + nr_pages = end_pfn - pfn;
> >> +
> >> + for (; pfn < end_pfn; pfn++, page++)
> >> __init_single_page(page, pfn, zid, nid);
> >> - nr_pages++;
> >> - }
> >> return nr_pages;
> >> }
> >>
> >> --
> >> 2.34.1
> >>
> >
> >--
> >Sincerely yours,
> >Mike.
>
> --
> Wei Yang
> Help you, Help me
--
Sincerely yours,
Mike.
