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

[David Hildenbrand (Arm)]

On 9/17/26 15:01, Breno Leitao wrote:
> On Wed, Sep 16, 2026 at 04:51:40PM +0200, David Hildenbrand (Arm) wrote:
>> On 9/16/26 11:35, Breno Leitao wrote:
>>>
>>> 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?
>>
>> Ah, that magical "range_contains_poisoned_memory" does a bitmap scan?
>>
>> I'm sorry, but that is absolutely confusing.
>>
>> There is no way someone will figure out that range_contains_poisoned_memory()
>> queries some efi specific bitmap that won't even be able to represent any memory
>> outside of it's range.
>>
>> I don't really have time to give a better solution, but starting with the
>> naming, range_contains_poisoned_memory() is just absolutely misleading.
> 
> Fair point, I'll clean up the naming in the next revision.
> 
> I'll also add that ->empty field, which should help locate this bit
> faster and may let us skip the bitmap query entirely on the happy path.
> 
> Anything else you'd like addressed?
> 
> Good to know this moved the needle from "David hates this feature" to
> "David only hates the naming" -- I'll take that as progress.  :-P

;)

I think the crucial part is to find a way to cleanly distinguish our source of
information, and also how the source does only apply to some memory.

Regarding this patch here, I'd assume it's sufficient.

But I do wonder why we are walking pages when we have a bitmap to walk/process
at hand?

-- 
Cheers,

David