Re: [PATCH] sfc: Use acquire/release for irq_soft_enabled

[Edward Cree]

On 03/06/2026 19:20, Jakub Kicinski wrote:
> On Thu, 28 May 2026 17:28:38 +0800 Gui-Dong Han wrote:
>> Use release stores when updating the gate and acquire loads in interrupt
>> handlers before touching channels. Keep the existing smp_wmb() after gate
>> updates, preserving the current ordering with event queue start and stop.
> 
> Why are you keeping the smp_wmb() tho? You don't have to repeat what
> the patch does. The goal of the commit message is to explain the why.
> 
> Ed, WDYT?

I think... that my head hurts from trying to understand this.
What I particularly don't understand is why it wouldn't suffice to just
 put smp_rmb() after every read of irq_soft_enabled, pairing with the
 existing smp_wmb().
And, conversely, why any of this works at all — what stops an interrupt
 handler passing the gate, because the write to irq_soft_enabled has
 propagated to its CPU, but the writes to channel pointers haven't?
 Naïvely I'd expect the smp_wmb() in efx_soft_enable_interrupts() to
 come _before_ the write to irq_soft_enabled.
(The smp_wmb() in efx_soft_disable_interrupts(), I'm not sure exactly
 what that pairs with, since the ordering that matters there is between
 the irq_soft_enabled=false write and the synchronize_irq().)

Anyway, this code predates my involvement with sfc and I've never had
 to touch it, so frankly I don't know any more here than anyone else,
 and if a memory-models expert reviews it and says 'this is needed and
 correct', that would mean more than a review tag from me.
That's not a cop-out "I won't review this", rather I'm saying that if
 Gui-Dong can't convince me because I'm too dumb, but can convince you,
 then you don't need to wait for me to show up and add my tag.

-ed