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
