Re: [PATCH net] batman-adv: reduce softirq drain in tt_purge loops

[Sven Eckelmann <sven@narfation.org>]

[-- Attachment #1: Type: text/plain, Size: 2569 bytes --]

On Saturday, 31 October 2026 01:02:27 CET Ömer Mete Kaya wrote:
> I think keeping the batching approach is the better option since
> cond_resched() alone does not reduce the number of inline softirq
> drains. But if you prefer the simpler per-bucket spin_lock_bh()
> with only cond_resched(), I will go that route for v2.

My understanding seems to be completely different. There is are two major 
changes - one is the actual optimization and then a second one which only 
applied in some situations:

I need an explanation in the commit message which tells us what the 
problematic situation is and why the approach of an outer local_bh_disable() 
is the correct one.

But since you have also an cond_resched() in the middle, I need also an 
explanation why this is required/good idea. And why it is not a problem to not 
have cond_resched in some contexts. I am not talking about the technical 
problem (commit_lock - which still needs to be mentioned) but why it doesn't
cause problems that there is no cond_resched there.

Not to forget about an explanation why the time for the worker doesn't stay 
stable when it has a lot of softirq work which is done anyway on the BH re-
enable points. The number of these BH re-enable points is lower but the total 
amount of work it needs to do stays the same - (without this explanation) you 
could therefore argue that just more softirq work is done per BH re-enable 
point and therefore the total amount of time spend by the worker (with the 
softirq time) stays the same. As result, we would still see the splat (see
syzkaller report).


> > This looks like a workaround for a problem which isn't described very well. 
> > Are there any precedents in the Linux kernel?
> > 
> > I know of things like ctnetlink_dump_table() with the large local_bh_disable() 
> > and inner spinlocks. But I know of nothing with the cond_resched. But since I 
> > don't know why it was added in the first place, I would a better explanation 
> > of the problem and the reasoning why it was tried to be fixed like you did.
> 
> ctnetlink_dump_table() in net/netfilter/nf_conntrack_netlink.c uses
> an outer local_bh_disable() with inner spin_lock()/
> spin_unlock() calls on per-bucket locks.

As said in my message: no cond_resched in ctnetlink_dump_table()

> nf_ct_iterate_cleanup() also uses cond_resched() while walking the
> conntrack hash, although it re-enables BHs at a different granularity.

No outer local_bh_disable() and only a mutex around the main loop.

Kind regards,
	Sven

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 228 bytes --]