Re: [PATCH v5] kexec: keep the next kernel off hardware-poisoned pages

[Breno Leitao <leitao@debian.org>]

On Tue, Aug 11, 2026 at 01:36:25PM +0200, Pratyush Yadav wrote:
> On Tue, Aug 11 2026, Breno Leitao wrote:
> 
> > On Mon, Aug 10, 2026 at 07:32:41PM +0300, Mike Rapoport wrote:
> >> Hi Breno,
> >> 
> >> On Mon, Aug 10, 2026 at 06:32:04AM -0700, Breno Leitao wrote:
> [...]
> >> > +	for (pfn = PHYS_PFN(start); pfn <= end_pfn; pfn++) {
> >> > +
> >> > +		cond_resched();
> >> 
> >> cond_resched() for every pfn is too much, isn't it?
> >
> > It is what the other pfn walkers do: the kpageflags read loop in
> > fs/proc/page.c and read_page_owner() in mm/page_owner.c both call it
> > once per pfn. 
> >
> > But I honestly don't have a strong opinion here, though, happy to batch
> > it if you prefer. Would this one look better?:
> >
> > 	if (!(pfn % MAX_ORDER_NR_PAGES))
> > 		cond_resched();
> >
> 
> Now that we are looking at this, Sashiko also complains about a race
> with memory hotunplug.
> 
>     Can the page pointer become invalid here due to concurrent memory hotplug?
> 
>     The loop retrieves the struct page pointer using pfn_to_online_page() and
>     then yields the CPU with cond_resched(). Since this iteration runs without
>     holding get_online_mems(), could a concurrent memory hot-unplug event offline
>     and remove the memory section while the thread is sleeping?
> 
>     If the vmemmap backing the struct page is freed and its page tables torn
>     down, dereferencing the pointer in is_page_hwpoison(page) upon waking
>     could cause a use-after-free regression.
> 
> Kind of makes sense at first glance but I didn't go and look. Perhaps it
> is better to do the cond_resched() at the end of the loop?

Agree. I was calling cond_resched() earlier because I wanted to call it
before the "continue", but it is rather unusal. Let me move it at the
end of the loop.

Thanks for the review,
--breno