batman-adv: Remove atomic usage for tt.local_changes

The tt.local_changes atomic is either written with tt.changes_list_lock
or close to it (see batadv_tt_local_event()). Thus the performance gain
using an atomic was limited (or because of atomic_read() impact even
negative). Using atomic also comes with the need to be wary of potential
negative tt.local_changes value.

Simplify the tt.local_changes usage by removing the atomic property and
modifying it only with tt.changes_list_lock held.

Signed-off-by: Remi Pommarel <repk@triplefau.lt>
Signed-off-by: Sven Eckelmann <sven@narfation.org>
Signed-off-by: Simon Wunderlich <sw@simonwunderlich.de>

diff --git a/net/batman-adv/soft-interface.c b/net/batman-adv/soft-interface.c
index 2758aba47a2f25961725dbac45f0133f9251f858..5666c268cead28c54a406a30d5e610a2f4654a01 100644 (file)
--- a/net/batman-adv/soft-interface.c
+++ b/net/batman-adv/soft-interface.c
@@ -783,13 +783,13 @@ static int batadv_softif_init_late(struct net_device *dev)
        atomic_set(&bat_priv->mesh_state, BATADV_MESH_INACTIVE);
        atomic_set(&bat_priv->bcast_seqno, 1);
        atomic_set(&bat_priv->tt.vn, 0);
-       atomic_set(&bat_priv->tt.local_changes, 0);
        atomic_set(&bat_priv->tt.ogm_append_cnt, 0);
 #ifdef CONFIG_BATMAN_ADV_BLA
        atomic_set(&bat_priv->bla.num_requests, 0);
 #endif
        atomic_set(&bat_priv->tp_num, 0);
 
+       WRITE_ONCE(bat_priv->tt.local_changes, 0);
        bat_priv->tt.last_changeset = NULL;
        bat_priv->tt.last_changeset_len = 0;
        bat_priv->isolation_mark = 0;

diff --git a/net/batman-adv/translation-table.c b/net/batman-adv/translation-table.c
index b44c382226a11b63984f294445b9081828fed1d3..6e0345b91ece2168024a639827c312f9901f7b1a 100644 (file)
--- a/net/batman-adv/translation-table.c
+++ b/net/batman-adv/translation-table.c
@@ -423,8 +423,8 @@ static void batadv_tt_local_event(struct batadv_priv *bat_priv,
        struct batadv_tt_change_node *tt_change_node, *entry, *safe;
        struct batadv_tt_common_entry *common = &tt_local_entry->common;
        u8 flags = common->flags | event_flags;
-       bool event_removed = false;
        bool del_op_requested, del_op_entry;
+       size_t changes;
 
        tt_change_node = kmem_cache_alloc(batadv_tt_change_cache, GFP_ATOMIC);
        if (!tt_change_node)
@@ -440,6 +440,7 @@ static void batadv_tt_local_event(struct batadv_priv *bat_priv,
 
        /* check for ADD+DEL or DEL+ADD events */
        spin_lock_bh(&bat_priv->tt.changes_list_lock);
+       changes = READ_ONCE(bat_priv->tt.local_changes);
        list_for_each_entry_safe(entry, safe, &bat_priv->tt.changes_list,
                                 list) {
                if (!batadv_compare_eth(entry->change.addr, common->addr))
@@ -468,21 +469,18 @@ static void batadv_tt_local_event(struct batadv_priv *bat_priv,
 del:
                list_del(&entry->list);
                kmem_cache_free(batadv_tt_change_cache, entry);
+               changes--;
                kmem_cache_free(batadv_tt_change_cache, tt_change_node);
-               event_removed = true;
-               goto unlock;
+               goto update_changes;
        }
 
        /* track the change in the OGMinterval list */
        list_add_tail(&tt_change_node->list, &bat_priv->tt.changes_list);
+       changes++;
 
-unlock:
+update_changes:
+       WRITE_ONCE(bat_priv->tt.local_changes, changes);
        spin_unlock_bh(&bat_priv->tt.changes_list_lock);
-
-       if (event_removed)
-               atomic_dec(&bat_priv->tt.local_changes);
-       else
-               atomic_inc(&bat_priv->tt.local_changes);
 }
 
 /**
@@ -950,7 +948,7 @@ static void batadv_tt_tvlv_container_update(struct batadv_priv *bat_priv)
        int tt_diff_entries_count = 0;
        u16 tvlv_len;
 
-       tt_diff_entries_num = atomic_read(&bat_priv->tt.local_changes);
+       tt_diff_entries_num = READ_ONCE(bat_priv->tt.local_changes);
        tt_diff_len = batadv_tt_len(tt_diff_entries_num);
 
        /* if we have too many changes for one packet don't send any
@@ -970,7 +968,7 @@ static void batadv_tt_tvlv_container_update(struct batadv_priv *bat_priv)
                goto container_register;
 
        spin_lock_bh(&bat_priv->tt.changes_list_lock);
-       atomic_set(&bat_priv->tt.local_changes, 0);
+       WRITE_ONCE(bat_priv->tt.local_changes, 0);
 
        list_for_each_entry_safe(entry, safe, &bat_priv->tt.changes_list,
                                 list) {
@@ -1380,7 +1378,7 @@ static void batadv_tt_changes_list_free(struct batadv_priv *bat_priv)
                kmem_cache_free(batadv_tt_change_cache, entry);
        }
 
-       atomic_set(&bat_priv->tt.local_changes, 0);
+       WRITE_ONCE(bat_priv->tt.local_changes, 0);
        spin_unlock_bh(&bat_priv->tt.changes_list_lock);
 }
 
@@ -3634,7 +3632,7 @@ static void batadv_tt_local_commit_changes_nolock(struct batadv_priv *bat_priv)
 {
        lockdep_assert_held(&bat_priv->tt.commit_lock);
 
-       if (atomic_read(&bat_priv->tt.local_changes) < 1) {
+       if (READ_ONCE(bat_priv->tt.local_changes) == 0) {
                if (!batadv_atomic_dec_not_zero(&bat_priv->tt.ogm_append_cnt))
                        batadv_tt_tvlv_container_update(bat_priv);
                return;

diff --git a/net/batman-adv/types.h b/net/batman-adv/types.h
index 04f6398b3a40e86f5dcd7b6a2a3cc1965f770b32..f491bff8c51b8bf68eb11dbbeb1a434d446c25f0 100644 (file)
--- a/net/batman-adv/types.h
+++ b/net/batman-adv/types.h
@@ -1022,7 +1022,7 @@ struct batadv_priv_tt {
        atomic_t ogm_append_cnt;
 
        /** @local_changes: changes registered in an originator interval */
-       atomic_t local_changes;
+       size_t local_changes;
 
        /**
         * @changes_list: tracks tt local changes within an originator interval
@@ -1044,7 +1044,7 @@ struct batadv_priv_tt {
         */
        struct list_head roam_list;
 
-       /** @changes_list_lock: lock protecting changes_list */
+       /** @changes_list_lock: lock protecting changes_list & local_changes */
        spinlock_t changes_list_lock;
 
        /** @req_list_lock: lock protecting req_list */