All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH batadv-next v2] batman-adv: tt: batch BH re-enable during TT hash walks
@ 2026-11-07 15:37 Ömer Mete Kaya
  2026-11-07 16:09 ` Sven Eckelmann
  0 siblings, 1 reply; 2+ messages in thread
From: Ömer Mete Kaya @ 2026-11-07 15:37 UTC (permalink / raw)
  To: sven; +Cc: b.a.t.m.a.n, Ömer Mete Kaya, syzbot+61b202fdc410840f0198

Each spin_unlock_bh() re-enables bottom halves and, when softirqs are
pending, processes them inline before returning. During a 1024-bucket
TT hash walk under network load, the purge worker can be repeatedly
interrupted by inline softirq processing at every bucket boundary.

Replace per-bucket spin_lock_bh()/spin_unlock_bh() with an outer
local_bh_disable(), inner spin_lock()/spin_unlock(), and periodic
re-enable points via batadv_hash_bh_resched(). The helper re-enables
BH and calls cond_resched() every BATADV_HASH_BH_BUDGET (64) buckets.
The total softirq work is unchanged, but BH state transitions drop
from 1024 to ~16 per walk, reducing how often the purge loop is
interrupted.

The cond_resched() provides a voluntary preemption point on
non-PREEMPT kernels. Functions called under tt.commit_lock cannot
yield because the caller holds a spinlock; these paths run only
during TT commit and the hash walk is the only significant work
under that lock.

The five converted functions:

  batadv_tt_local_purge():
    Accepts a new @may_sleep parameter. From workqueue context
    (batadv_tt_purge), may_sleep=true allows periodic yielding.
    From batadv_tt_local_resize_to_mtu(), may_sleep=false because
    the caller holds tt.commit_lock.

  batadv_tt_global_del_orig():
    Called from workqueue context; yields every 64 buckets. The
    local_bh_enable() is placed before clear_bit() so that pending
    softirqs run while no per-bucket lock is held.

  batadv_tt_global_purge():
    Called from workqueue context; yields every 64 buckets.

  batadv_tt_local_transition_new():
  batadv_tt_local_purge_pending_clients():
    Called under tt.commit_lock. Convert to outer local_bh_disable()/
    local_bh_enable() with inner spin_lock()/spin_unlock() but do
    not use batadv_hash_bh_resched().

On PREEMPT_RT, local_bh_disable() maps to migrate_disable() and
softirqs run in dedicated kthreads, so the batching has no effect
on softirq latency but the reduced BH transition overhead still
applies.

Reported-by: syzbot+61b202fdc410840f0198@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug\?extid\=61b202fdc410840f0198
Signed-off-by: Ömer Mete Kaya <omermetekaya0@gmail.com>
---
Changes in v2:
  - Rewrite commit message to explain the patch better
  - Extract common helper batadv_hash_bh_resched() with
    BATADV_HASH_BH_BUDGET constant
  - Convert all five hash-walking functions:
    batadv_tt_global_del_orig(), batadv_tt_local_transition_new(),
    batadv_tt_local_purge_pending_clients()
  - Add @may_sleep kernel-doc for batadv_tt_local_purge()
  - Add #include <linux/sched.h>
  - Drop Fixes: tag; target batadv-next
  - Rebase onto current batadv-next

 net/batman-adv/hash.h              | 30 +++++++++++++++++
 net/batman-adv/translation-table.c | 52 ++++++++++++++++++++++--------
 2 files changed, 69 insertions(+), 13 deletions(-)

diff --git a/net/batman-adv/hash.h b/net/batman-adv/hash.h
index 5d9a5ab..cf3ef70 100644
--- a/net/batman-adv/hash.h
+++ b/net/batman-adv/hash.h
@@ -10,10 +10,12 @@
 #include "main.h"

 #include <linux/atomic.h>
+#include <linux/bottom_half.h>
 #include <linux/compiler.h>
 #include <linux/list.h>
 #include <linux/lockdep.h>
 #include <linux/rculist.h>
+#include <linux/sched.h>
 #include <linux/spinlock.h>
 #include <linux/stddef.h>
 #include <linux/types.h>
@@ -167,4 +169,32 @@ static inline void *batadv_hash_remove(struct batadv_hashtable *hash,
 	return data_save;
 }

+/* work which can be done in a hash walk before BH gets re-enabled */
+#define BATADV_HASH_BH_BUDGET 64
+
+/**
+ * batadv_hash_bh_resched() - re-enable BH after the work budget is used up
+ * @buckets_done: buckets processed since BH was disabled, reset when BH is
+ *  re-enabled
+ *
+ * A walk over all buckets of a hash disables BH only once instead of
+ * re-enabling it after each bucket. Otherwise, pending softirqs would be
+ * processed inline after each bucket. When @buckets_done reached
+ * BATADV_HASH_BH_BUDGET, BH is re-enabled to run the pending softirqs and to
+ * give other tasks a chance to run.
+ *
+ * Must be called with BH disabled, without any spinlock held and from a
+ * context which is allowed to sleep.
+ */
+static inline void batadv_hash_bh_resched(u32 *buckets_done)
+{
+	if (*buckets_done < BATADV_HASH_BH_BUDGET)
+		return;
+
+	local_bh_enable();
+	cond_resched();
+	local_bh_disable();
+	*buckets_done = 0;
+}
+
 #endif /* _NET_BATMAN_ADV_HASH_H_ */
diff --git a/net/batman-adv/translation-table.c b/net/batman-adv/translation-table.c
index b9407ad..db93750 100644
--- a/net/batman-adv/translation-table.c
+++ b/net/batman-adv/translation-table.c
@@ -36,6 +36,7 @@
 #include <linux/overflow.h>
 #include <linux/rculist.h>
 #include <linux/rcupdate.h>
+#include <linux/sched.h>
 #include <linux/skbuff.h>
 #include <linux/slab.h>
 #include <linux/spinlock.h>
@@ -1892,23 +1893,32 @@ static void batadv_tt_local_purge_list(struct batadv_priv *bat_priv,
  * @bat_priv: the bat priv with all the mesh interface information
  * @timeout: parameter deciding whether a given tt local entry is considered
  *  inactive or not
+ * @may_sleep: if true, periodically re-enable softirqs and yield the CPU.
+ *  Must be false when called with spinlocks held.
  */
 static void batadv_tt_local_purge(struct batadv_priv *bat_priv,
-				  int timeout)
+				  int timeout, bool may_sleep)
 {
 	spinlock_t *list_lock; /* protects write access to the hash lists */
 	struct batadv_hashtable *hash = bat_priv->tt.local_hash;
 	struct hlist_head *head;
+	u32 buckets_done = 0;
 	u32 i;

+	local_bh_disable();
 	for (i = 0; i < hash->size; i++) {
 		head = &hash->table[i];
 		list_lock = &hash->list_locks[i];

-		spin_lock_bh(list_lock);
+		spin_lock(list_lock);
 		batadv_tt_local_purge_list(bat_priv, head, timeout);
-		spin_unlock_bh(list_lock);
+		spin_unlock(list_lock);
+		buckets_done++;
+
+		if (may_sleep)
+			batadv_hash_bh_resched(&buckets_done);
 	}
+	local_bh_enable();
 }

 /**
@@ -2882,16 +2892,18 @@ void batadv_tt_global_del_orig(struct batadv_priv *bat_priv,
 	struct hlist_node *safe;
 	struct hlist_head *head;
 	unsigned short vid;
+	u32 buckets_done = 0;
 	u32 i;

 	if (!hash)
 		return;

+	local_bh_disable();
 	for (i = 0; i < hash->size; i++) {
 		head = &hash->table[i];
 		list_lock = &hash->list_locks[i];

-		spin_lock_bh(list_lock);
+		spin_lock(list_lock);
 		hlist_for_each_entry_safe(tt_common_entry, safe,
 					  head, hash_entry) {
 			/* remove only matching entries */
@@ -2916,8 +2928,12 @@ void batadv_tt_global_del_orig(struct batadv_priv *bat_priv,
 				batadv_tt_global_entry_put(tt_global);
 			}
 		}
-		spin_unlock_bh(list_lock);
+		spin_unlock(list_lock);
+		buckets_done++;
+
+		batadv_hash_bh_resched(&buckets_done);
 	}
+	local_bh_enable();
 	clear_bit(BATADV_ORIG_CAPA_HAS_TT, &orig_node->capa_initialized);
 }

@@ -2970,13 +2986,15 @@ static void batadv_tt_global_purge(struct batadv_priv *bat_priv)
 	struct hlist_node *node_tmp;
 	struct hlist_head *head;
 	char *msg = NULL;
+	u32 buckets_done = 0;
 	u32 i;

+	local_bh_disable();
 	for (i = 0; i < hash->size; i++) {
 		head = &hash->table[i];
 		list_lock = &hash->list_locks[i];

-		spin_lock_bh(list_lock);
+		spin_lock(list_lock);
 		hlist_for_each_entry_safe(tt_common, node_tmp, head,
 					  hash_entry) {
 			tt_global = container_of(tt_common,
@@ -2996,8 +3014,12 @@ static void batadv_tt_global_purge(struct batadv_priv *bat_priv)

 			batadv_tt_global_entry_put(tt_global);
 		}
-		spin_unlock_bh(list_lock);
+		spin_unlock(list_lock);
+		buckets_done++;
+
+		batadv_hash_bh_resched(&buckets_done);
 	}
+	local_bh_enable();
 }

 /**
@@ -4402,7 +4424,7 @@ static void batadv_tt_purge(struct work_struct *work)
 	priv_tt = container_of(delayed_work, struct batadv_priv_tt, work);
 	bat_priv = container_of(priv_tt, struct batadv_priv, tt);

-	batadv_tt_local_purge(bat_priv, BATADV_TT_LOCAL_TIMEOUT);
+	batadv_tt_local_purge(bat_priv, BATADV_TT_LOCAL_TIMEOUT, true);
 	batadv_tt_global_purge(bat_priv);
 	batadv_tt_req_purge(bat_priv);
 	batadv_tt_roam_purge(bat_priv);
@@ -4449,11 +4471,12 @@ static void batadv_tt_local_transition_new(struct batadv_priv *bat_priv)
 	if (!hash)
 		return;

+	local_bh_disable();
 	for (i = 0; i < hash->size; i++) {
 		head = &hash->table[i];
 		list_lock = &hash->list_locks[i];

-		spin_lock_bh(list_lock);
+		spin_lock(list_lock);
 		hlist_for_each_entry(tt_common_entry, head, hash_entry) {
 			bool cont = false;

@@ -4472,8 +4495,9 @@ static void batadv_tt_local_transition_new(struct batadv_priv *bat_priv)
 			batadv_tt_local_size_inc(bat_priv,
 						 tt_common_entry->key.vid);
 		}
-		spin_unlock_bh(list_lock);
+		spin_unlock(list_lock);
 	}
+	local_bh_enable();
 }

 /**
@@ -4497,11 +4521,12 @@ static void batadv_tt_local_purge_pending_clients(struct batadv_priv *bat_priv)
 	if (!hash)
 		return;

+	local_bh_disable();
 	for (i = 0; i < hash->size; i++) {
 		head = &hash->table[i];
 		list_lock = &hash->list_locks[i];

-		spin_lock_bh(list_lock);
+		spin_lock(list_lock);
 		hlist_for_each_entry_safe(tt_common, node_tmp, head,
 					  hash_entry) {
 			bool cont = false;
@@ -4536,8 +4561,9 @@ static void batadv_tt_local_purge_pending_clients(struct batadv_priv *bat_priv)

 			batadv_tt_local_entry_put(tt_local);
 		}
-		spin_unlock_bh(list_lock);
+		spin_unlock(list_lock);
 	}
+	local_bh_enable();
 }

 /**
@@ -4839,7 +4865,7 @@ void batadv_tt_local_resize_to_mtu(struct net_device *mesh_iface)
 		if (size_max >= table_size)
 			break;

-		batadv_tt_local_purge(bat_priv, timeout);
+		batadv_tt_local_purge(bat_priv, timeout, false);
 		batadv_tt_local_purge_pending_clients(bat_priv);

 		timeout /= 2;
--
2.56.0


^ permalink raw reply related	[flat|nested] 2+ messages in thread

* Re: [PATCH batadv-next v2] batman-adv: tt: batch BH re-enable during TT hash walks
  2026-11-07 15:37 [PATCH batadv-next v2] batman-adv: tt: batch BH re-enable during TT hash walks Ömer Mete Kaya
@ 2026-11-07 16:09 ` Sven Eckelmann
  0 siblings, 0 replies; 2+ messages in thread
From: Sven Eckelmann @ 2026-11-07 16:09 UTC (permalink / raw)
  To: Ömer Mete Kaya, syzbot+61b202fdc410840f0198; +Cc: b.a.t.m.a.n

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

On Saturday, 7 November 2026 16:37:25 CET Ömer Mete Kaya wrote:
> Closes: https://syzkaller.appspot.com/bug\?extid\=61b202fdc410840f0198

This link doesn't seem to work.

On Saturday, 7 November 2026 16:37:25 CET Ömer Mete Kaya wrote:
>         unsigned short vid;
> +       u32 buckets_done = 0;

Breaks the reverse x-mas tree order of variable declarations.

(there are multiple occurrences of this problem)

>   batadv_tt_global_del_orig():
>     Called from workqueue context; yields every 64 buckets. The
>     local_bh_enable() is placed before clear_bit() so that pending
>     softirqs run while no per-bucket lock is held.
> 

Are you sure? It is called from so many different places. Including this one 
(which clearly doesn't allow you to sleep due to the spinlock around the 
call):

void batadv_purge_orig_ref(struct batadv_priv *bat_priv)
{
[...]
	/* for all origins... */
	for (i = 0; i < hash->size; i++) {
		head = &hash->table[i];
		if (hlist_empty(head))
			continue;
		list_lock = &hash->list_locks[i];

		spin_lock_bh(list_lock);
[....]
				batadv_tt_global_del_orig(orig_node->bat_priv,
							  orig_node, -1,
							  "originator timed out");
[...]
		spin_unlock_bh(list_lock);
	}

	batadv_gw_election(bat_priv);
}

Regards,
	Sven

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

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2026-11-07 16:09 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-11-07 15:37 [PATCH batadv-next v2] batman-adv: tt: batch BH re-enable during TT hash walks Ömer Mete Kaya
2026-11-07 16:09 ` Sven Eckelmann

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.