Re: [PATCH splitout] mm: memory-failure: serialize TestSetPageHWPoison with zone->lock

["David Hildenbrand (Arm)" <david@kernel.org>]

On 6/16/26 13:40, Miaohe Lin wrote:
> On 2026/6/16 14:56, David Hildenbrand (Arm) wrote:
>>>
>>> These non-atomics are defined and used because they want to avoid atomic ops overhead?
>>> So I'm afraid using rcu read lock in these places would lead to unexpected overhead.
>>
>> It should be cheaper than atomics IIUC. Further, I assume that some pages could
>> batch over multiple such operations (esp. page freeing path when we process tail
>> pages).
>>
>> With !CONFIG_PREEMPT_RCU it's simply preempt_disable()/preempt_enable(), which
>> is either a NOP or just adjusting the preempt counter of the current thread. Cheap.
>>
>> With CONFIG_PREEMPT_RCU we mostly increment current->rcu_read_lock_nesting. But
>> there might be a function call involved (did not look into the details). So that
>> variant should be slightly more expensive.
> 
> I scanned the code and found rcu_read_unlock_special might be called in some cases.
> Some expensive ops, e.g. irq_work_queue_on, might be called in some corner cases.
> So the overhead of rcu read lock might be fluctuating.

Right. Usually rcu_read_lock+unlock is supposed to be very lightweight, but that
might not be completely the case with that PREEMPT_RCU thingy ...

> 
>>
>> We'd have to measure what an addition rcu read lock would cost in there. that
>> should be fairly easy to benchmark.
> 
> Sure. We can do that if needed.
> 
>>
>>>
>>> I think this is a good idea, although there are some remaining issues.
>>> But such race should be really rare, is it worth all this effort? Could we
>>> simply aim to resolve, not to be flawless? I.e. could we simply check
>>> and re-set the hwpoison flag at the end of memory_failure handling to
>>> simply avoid losing hwpoison flag as a best-effort attempt? Would it be
>>> acceptable?
>>
>> Hacky. Sufficient for the hypervisor to suspend the nonatomic-setting CPU at the
>> wrong time to still trigger the same behavior.
> 
> Right. hypervisor could make the issue easier to trigger...
> 
>>
>> I think, either we fix it properly, or we redesign hwpoison handling to deal
>> with setting/clearing becoming stale at some random point in the future.
> 
> I think your proposal, although there are still some issues to be resolved, is
> nevertheless a good solution. We could also wait and see if anyone comes up with
> a better one.

I wouldn't call it "good" ... it's the only thing I was easily able to come up
with :)

The only alternative would be moving the hwpoison bit out of page->flags,
storing it in a sparse bitmap or sth. like that. It would be a bigger rework and
I am sure there are issues with that as well.

-- 
Cheers,

David