Re: [PATCH] kho: align kho_scratch to MAX_ORDER_NR_PAGES pages

[Pasha Tatashin <pasha.tatashin@soleen.com>]

On 07-15 09:42, Mike Rapoport wrote:
> Hi Michal,
> 
> > I caught this crash:
> > BUG: unable to handle page fault for address: ff19164fffff8328
> > RIP: 0010:__free_one_page+0x1a1/0x6b0
> > Call Trace:
> >  <TASK>
> >  [<ffffffff913208bf>] free_one_page+0xaf/0x240
> >  [<ffffffff93973288>] deferred_free_pages+0xa8/0xd0
> >  [<ffffffff93971b4f>] deferred_init_memmap_chunk+0x10f/0x1b0
> >  [<ffffffff9396e265>] padata_mt_helper+0x65/0xa0
> >  [<ffffffff90fac402>] process_scheduled_works+0x202/0x410
> >  [<ffffffff90fae739>] worker_thread+0x1f9/0x2d0
> >  [<ffffffff90fb62fd>] kthread+0x27d/0x2f0
> >  [<ffffffff90fae540>] ? __pfx_worker_thread+0x10/0x10
> >  [<ffffffff90fb6080>] ? __pfx_kthread+0x10/0x10
> >  [<ffffffff90efdc55>] ret_from_fork+0x145/0x280
> >  [<ffffffff90fb6080>] ? __pfx_kthread+0x10/0x10
> >  [<ffffffff90e2e46a>] ret_from_fork_asm+0x1a/0x30
> >  </TASK>
> > 
> > deferred_init_memmap_chunk() works fine without KHO because free
> > regions will never be neighbors. However, with KHO, free memory
> > will be split into (free && scratch) and (free && !scratch).
> > KHO scratch is aligned to CMA_MIN_ALIGNMENT_BYTES (which on my setup
> > is equal to 1 << 9 pages) but buddy can look at the neighborhood of
> > MAX_ORDER_NR_PAGES pages (which is equal to 1 << 10 pages).
> > This sometimes crashes when one half of the neighborhood is being
> > initialized but the other part is yet to be initialized.
> > 
> > To fix this, let's just align KHO scratch to MAX_ORDER_NR_PAGES pages.
> > 
> > Fixes: c6073743d0c7 ("kho: make preserved pages compatible with deferred struct page init")
> > Signed-off-by: Michal Clapinski <mclapinski@google.com>
> >
> > diff --git a/kernel/liveupdate/kexec_handover.c b/kernel/liveupdate/kexec_handover.c
> > index 4834a809985ab..50928aaa33710 100644
> > --- a/kernel/liveupdate/kexec_handover.c
> > +++ b/kernel/liveupdate/kexec_handover.c
> > @@ -38,6 +38,16 @@
> >  #include "../kexec_internal.h"
> >  #include "kexec_handover_internal.h"
> >  
> > +/*
> > + * This is the minimal alignment required by deferred struct page init.
> > + * deferred_init_memmap_chunk frees memory to the buddy allocator, which looks
> > + * at the neighboring pages (up to MAX_PAGE_ORDER) to merge them.
> > + * If KHO scratch is not aligned to that value, buddy can access uninitialized
> > + * struct pages, which can cause a crash.
> > + */
> > +#define SCRATCH_ALIGNMENT_BYTES (1 << (PAGE_SHIFT + MAX_PAGE_ORDER))
> > +static_assert(SCRATCH_ALIGNMENT_BYTES >= CMA_MIN_ALIGNMENT_BYTES);
> 
> Maybe make it a variable and set it at runtime?
> We won't need to worry about different confugurations that override
> MAX_ORDER and potentail divergence of pageblock size from MAX_ORDER in

Do those architectures reduce MAX_ORDER when they override it? I would 
hope we could set the alignment once and make it large enough to work 
everywhere, rather than depending on dynamic changes even during boot...

> the future.
> 
> -- 
> Sincerely yours,
> Mike.
>