Re: [B.A.T.M.A.N.] [PATCH v2 6/7] batman-adv: ELP - use tp meter to estimate the throughput if otherwise not available

Linus Lüssing
2018-05-21T14:48:55Z

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?