On Wed, Oct 18, 2023 at 7:08 AM Matthew Wilcox <willy@infradead.org> wrote:

> On Tue, Oct 17, 2023 at 05:55:48PM -0700, Sourav Panda wrote:
> > +             mod_node_early_perpage_metadata(pgdat->node_id,
> > +                                             PAGE_ALIGN(size) >>
> PAGE_SHIFT);
>
> What a curious way of writing DIV_ROUND_UP(size, PAGE_SIZE)
> (throughout).  A quick grep shows about 230 DIV_ROUND_UP and 110 of what
> you wrote, so it's not something you invented, but DIV_ROUND_UP is
> clearer.
>
> > @@ -303,18 +308,25 @@ static int __meminit
> init_section_page_ext(unsigned long pfn, int nid)
> >
> >  static void free_page_ext(void *addr)
> >  {
> > +     size_t table_size;
> > +     struct page *page;
> > +
> > +     table_size = page_ext_size * PAGES_PER_SECTION;
> > +
> >       if (is_vmalloc_addr(addr)) {
> > +             page = vmalloc_to_page(addr);
> >               vfree(addr);
> >       } else {
> > -             struct page *page = virt_to_page(addr);
> > -             size_t table_size;
> > -
> > -             table_size = page_ext_size * PAGES_PER_SECTION;
> > +             page = virt_to_page(addr);
> >
> >               BUG_ON(PageReserved(page));
> >               kmemleak_free(addr);
> >               free_pages_exact(addr, table_size);
> >       }
> > +
> > +     __mod_node_page_state(page_pgdat(page), NR_PAGE_METADATA,
> > +                           -1L * (PAGE_ALIGN(table_size) >>
> PAGE_SHIFT));
>
> This troubles me.  We're freeing the memory and then dereferencing
> the page that was freed.  I know that struct pages don't go away when
> they're freed, and they don't change which node they're allocated to,
> but it feels wrong.  I'd be happier if the page_pgdat() were extracted
> prior to freeing the memory.
>


Thank you Matthew Wilcox for pointing this out. I will extract page_pgdat()
prior to freeing the memory in v3.

With regards,
Sourav Panda