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

Antonio Quartulli
2018-08-04T09:39:42Z

[-- Attachment #1.1: Type: text/plain, Size: 1937 bytes --]

Hi Lunus,

On 21/05/18 22:48, Linus Lüssing wrote:
> 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

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]