Re: [PATCH nf-next v2 3/3] netfilter: nft_set_pipapo: Use nested-BH locking for nft_pipapo_scratch

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 2025-08-18 11:24:07 [+0200], Florian Westphal wrote:
> Sebastian Andrzej Siewior <bigeasy@xxxxxxxxxxxxx> wrote:
> > > 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).
> 
> This is never called from hardirq context, its always process context
> or softirq.  Are you saying that the irq_fpu_usable() is bogus
> and can be axed?

Process context could use the FPU (say btrfs for crc32 checking) and
then in softirq you would have to check if you are allowed to used them.
This was the original requirement for softirq usage.

x86 does disable BH in kernel_fpu_begin() before the FPU can be used.
This "new" development required while the check was added. The
fpu-begin() change has been added around v5.2-rc1 while the FPU
registers are restored on return to user land (in order to make kernel
fpu begin/ end cheaper). 

Since this is x86 only you could drop it without breaking anything as of
today. I don't think this will change but I also don't feel like I
should advice dropping it. This does not look performance critical and
all users call it first.

> > 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().
> 
> Sure, I don't see why it can't be moved back to pipapo_get_avx2().
> 
> You can also just resubmit vs. nf-next main and I can rebase my
> patches on top of the local_lock changes, up to you.

I'm half way done ;)

Sebastian




[Index of Archives]     [Netfitler Users]     [Berkeley Packet Filter]     [LARTC]     [Bugtraq]     [Yosemite Forum]

  Powered by Linux