Re: [PATCH v5 7/9] drivers/base/memory: count inherited poisoned frames into the block

[Breno Leitao <leitao@debian.org>]

Hello David,

First of all, thanks for your time looking at this patchset.

On Wed, Sep 16, 2026 at 08:48:56AM +0200, David Hildenbrand (Arm) wrote:
> On 9/15/26 14:53, Breno Leitao wrote:
> > +static void memblk_nr_poison_init(struct memory_block *mem)
> > +{
> > +	unsigned long pfn = section_nr_to_pfn(mem->start_section_nr);
> > +	unsigned long nr_pages = PAGES_PER_SECTION * sections_per_block;
> > +	unsigned long i, nr_poison = 0;
> > +
> > +	/* A hotplugged block is created before its pages are online. */
> > +	if (mem->state != MEM_ONLINE)
> > +		return;
> > +
> > +	if (!range_contains_poisoned_memory(PFN_PHYS(pfn),
> > +					    nr_pages << PAGE_SHIFT))
> > +		return;
> > +
> > +	for (i = 0; i < nr_pages; i++) {
> > +		struct page *page = pfn_to_online_page(pfn + i);
> > +
> > +		if (page && PageHWPoison(page))
> > +			nr_poison++;
> > +	}
> 
> That just slows down boot unnecessarily on 99.9999999999999999999% of all
> systems out there.

hmmm, I am not sure I see it that way.

The loop only runs for a block the bitmap marks. On a machine with nothing
recorded the bitmap is all zeros, the range_contains_poisoned_memory() check
right above it returns false, and the loop never executes.

What every boot does pay is that check, once per block. The stub installs
the table whether or not anything was ever recorded in it, so this is not a
NULL test: it is two 64-bit divisions by the unit size plus a
find_next_bit() over the single word a 128M block covers at one bit per 2M.

The real cost (that "for loop above"), comes when you kexec (not on cold
boot -- given the bitmap is empty), and you are trying to init
a memory block that has poisoned pages into it. Which seems the right
trade-off, no?

That said, can we do better? Yes. The silly win is to let the table say
whether anything was ever recorded in it, something like a
linux_efi_poisoned_memory->empty that the first recorded frame clears,
and return on that before the bitmap is reached at all.

Is this what you are looking for, or something more drastic?

Thanks for your review,
--breno