On 2025-08-16 16:25:44 [+0200], Florian Westphal wrote:
> Sebastian Andrzej Siewior <bigeasy@xxxxxxxxxxxxx> wrote:
> > @@ -1170,20 +1170,18 @@ nft_pipapo_avx2_lookup(const struct net *net, const struct nft_set *set,
> > }
> >
> > m = rcu_dereference(priv->match);
> > -
> > + scratch = *raw_cpu_ptr(m->scratch);
> > + if (unlikely(!scratch)) {
> > + local_bh_enable();
> > + return false;
>
> The function has been changed upstream to return a pointer.
Sorry. Missed that and gcc didn't do much of complaining.
> > + }
> > + __local_lock_nested_bh(&scratch->bh_lock);
> > /* Note that we don't need a valid MXCSR state for any of the
> > * operations we use here, so pass 0 as mask and spare a LDMXCSR
> > * instruction.
> > */
> > kernel_fpu_begin_mask(0);
>
> Not sure this is correct. If RT allows to migrate in BH before
> the local lock is taken, then the if (unlikely(!irq_fpu_usable()))
> check needs to be done after the local lock was taken, no?
Looking at this again, that irq_fpu_usable() itself looks slightly
placed at the wrong spot. It should be
| if (!irq_fpu_usable())
| return fallback_variant();
| local_bh_disable();
The reason is that if you expect calls from hardirq context then you
shouldn't disable BH. Lockdep enabled kernels should complain in this
case. But then this is networking and I would expect everything here
should be injected via NAPI/ softirq (either via driver's NAPI or
backlog's).
But back to your question: This is usually called from softirq so the
thread can't migrate to another CPU. If this would have been called from
process context then it could migrate to another CPU. However everything
that would block the FPU needs to disable BH (preemption on RT) so that
a tasks does not migrate there while the FPU is blocked.
> I will place a pending pipapo change in the nf-next:testing
> branch shortly in case you need to resend.
>
> If its fine as-is, I can also rebase the pending pipapo_avx2 patches.
>
> Let me know.
I've just started rebasing this patch on top of the testing branch. I
see that you moved that code a bit around. I can't acquire the
local_lock_t within kernel_fpu_begin(). I would need to move this back
to pipapo_get_avx2().
Sebastian
