Re: [PATCH 3/6] xen/softirq: Rework arch_skip_send_event_check() into arch_pend_softirq()

[Jan Beulich <jbeulich@xxxxxxxx>]

On 03.07.2025 12:36, Andrew Cooper wrote:
> On 03/07/2025 9:11 am, Jan Beulich wrote:
>> On 02.07.2025 16:41, Andrew Cooper wrote:
>>> x86 is the only architecture wanting an optimisation here, but the
>>> test_and_set_bit() is a store into the monitored line
>> Which is intentional aiui, while this reads as if this was part of the issue.
> 
> I don't understand what you're trying to say.

What I was trying to say is the way you wrote it to me it read as if you're
describing two issues: There wrongly being a store into the monitored line,
and ...

> It is racy, and that's why we're changing it.
> 
>>> and is racy with determining whether an IPI can be skipped.

... there being a race.

>> Racy here as in limiting the effect of the optimization, but not affecting
>> correctness aiui: If the woken CPU managed to clear the bit already, we'd
>> needlessly IPI it.
> 
> Correct.
> 
>>  This could also do with saying.
> 
> What about this?
> 
> x86 is the only architecture wanting an optimisation here, but the
> test_and_set_bit() is a store into the monitored line (i.e. will wake up
> the target) and, prior to the removal of the broken IPI-elision
> algorithm, was racy, causing unnecessary IPIs to be sent.
> 
> To do this in a race-free way, the store to the monited line needs to
> also sample the status of the target in one atomic action.  Implement a
> new arch helper with different semantics; to make the softirq pending
> and decide about IPIs together.  For now, implement the default helper. 
> It will be overridden by x86 in a subsequent change.

Better, yes. Thanks.

Jan