On Fri, May 18, 2018 at 09:47:53AM +0800, Marek
Lindner wrote:
+/**
+ * batadv_v_elp_tp_start() - start a tp meter session for a neighbor
+ * @neigh: neighbor to run tp meter on
+ */
+static void batadv_v_elp_tp_start(struct batadv_hardif_neigh_node *neigh)
+{
+ struct batadv_hard_iface *hard_iface = neigh->if_incoming;
+ struct batadv_priv *bat_priv = netdev_priv(hard_iface->soft_iface);
+
+ neigh->bat_v.tp_meter_running = true;
+ batadv_tp_start(bat_priv, neigh->addr, neigh, 10, NULL, BATADV_TP_ELP);
+}
This seems racy here? We have no ordering constraints between
the assigment and batadv_tp_start() which might lead to...
@@ -152,6 +192,25 @@ static u32
batadv_v_elp_get_throughput(struct batadv_hardif_neigh_node *neigh)
return throughput * 10;
}
+fallback_throughput:
+ last_tp_run_jiffies = jiffies - neigh->bat_v.last_tp_meter_run;
+ last_tp_run_msecs = jiffies_to_msecs(last_tp_run_jiffies);
+
+ /* check the tp_meter_running flag before checking the timestamp to
+ * avoid a race condition where a new tp meter session is scheduled
+ * right after the previous tp meter session has completed
+ */
+ if (!neigh->bat_v.tp_meter_running &&
+ last_tp_run_msecs > BATADV_ELP_TP_RUN_INTERVAL)
+ batadv_v_elp_tp_start(neigh);
...multiple instances potentially being started at the same time here?
- batadv_tp_start() is only invoked by batadv_v_elp_get_throughput();
- batadv_v_elp_get_throughput() is only invoked by
batadv_v_elp_throughput_metric_update();
- batadv_v_elp_throughput_metric_update() is a periodic function
executed by a timer and we can't have concurrent executions (unless we
use crazy interval values).
Maybe we should add a comment to make this more clear?
--
Antonio Quartulli