Re: [PATCH] page_ext: move up page_ext_init() to catch early page allocation if DEFERRED_STRUCT_PAGE_INIT is n

[Michal Hocko]

On Sat 20-08-22 09:02:57, lizhe.67@xxxxxxxxxxxxx wrote:
> On 2022-08-18 7:36 UTC, mhocko@xxxxxxxx wrote:
> >> From: Li Zhe <lizhe.67@xxxxxxxxxxxxx>
> >> 
> >> In 'commit 2f1ee0913ce5 ("Revert "mm: use early_pfn_to_nid in page_ext_init"")',
> >> we call page_ext_init() after page_alloc_init_late() to avoid some panic
> >> problem. It seems that we cannot track early page allocations in current
> >> kernel even if page structure has been initialized early.
> >> 
> >> This patch move up page_ext_init() to catch early page allocations when
> >> DEFERRED_STRUCT_PAGE_INIT is n. After this patch, we only need to turn
> >> DEFERRED_STRUCT_PAGE_INIT to n then we are able to analyze the early page
> >> allocations. This is useful especially when we find that the free memory
> >> value is not the same right after different kernel booting.
> >
> >is this actually useful in practice? I mean who is going to disable
> >DEFERRED_STRUCT_PAGE_INIT and recompile the kernel for debugging early
> >allocations?
> 
> Yes it is useful. We use this method to catch the difference of early
> page allocations between two kernel.

I was not questioning the functionality itself but the way how it is
achieved. Recompiling the kernel to achieve debuggability has proven to
be really a bad approach historically. Most people are using
pre-compiled kernels these days.

> > I do see how debugging those early allocations might be useful but that
> > would require a boot time option to be practical IMHO. Would it make
> > sense to add a early_page_ext parameter which would essentially disable
> > the deferred ipage initialization. That should be quite trivial to
> > achieve (just hook into defer_init AFAICS).
> 
> It is a good idea. A cmdline parameter is a flexible and dynamic method for
> us to decide whether to defer page's and page_ext's initilization. For
> comparison, this patch provides a static method to decide whether to defer
> page's and page_ext's initilization. They are not conflicting. My next
> work is trying to achieve your idea.

They are not conflicting but this patch adds ifdefs and additional code
that needs compile time testing with different options set. I.e. it adds
maintenance burden for something that can be achieved by better means.
So if you are ok to work on the runtime knob then I would propose to
drop this patch from the mm tree and replace it by a trivial patch to
allow early boot debugging by a cmd line parameter.
-- 
Michal Hocko
SUSE Labs