On Fri, May 18, 2018 at 09:47:53AM +0800, Marek Lindner wrote:
> @@ -251,6 +253,21 @@ static void batadv_tp_caller_notify(struct batadv_priv 
> *bat_priv,
>  
>               break;
>       case BATADV_TP_ELP:
> +             if (reason_is_error) {
> +                     batadv_v_elp_tp_fail(tp_vars->hardif_neigh);
> +                     return;
> +             }
> +
> +             test_time = jiffies_to_msecs(jiffies - tp_vars->start_time);
> +             total_bytes = atomic64_read(&tp_vars->tot_sent);
> +
> +             /* The following calculation includes these steps:
> +              * - convert bytes to bits
> +              * - divide bits by the test length (msecs)
> +              * - convert result from bits/ms to 0.1Mb/s (* 1024 * 10 / 1000)
> +              */
> +             throughput = total_bytes * 8 >> ilog2(test_time) / 10;
> +             batadv_v_elp_tp_finish(tp_vars->hardif_neigh, throughput);

I find the throughput calculation quite hard to read here, would
it be possible to put this into an extra (inline?) function?

Also the comment for the "convert result..." seems wrong,
"[bits/ms]*1024*10/1000" would be 0.01Mb/s, not 0.1Mb/s?

What is the advantage of using the ilog2 and shift operator here
compared to plain multiplications and divisions?

Also, when trying this in a small C program I get weird results:

-----
#include <stdio.h>
#include <math.h>

int main()
{
        unsigned long test_time = 10000;        // 10s
        unsigned long total_bytes = 20000000;   // 16MBit/s
        unsigned long throughput, throughput2;
        unsigned long log_test_time = log(test_time) / log(2);

        throughput = total_bytes * 8 >> log_test_time / 10;

        // Straightforward approach?
        throughput2 = total_bytes * 8 / test_time * 1000 / 1024 / 100;

        printf("Result: %lu (log_test_time: %lu)\n", throughput, log_test_time);
        printf("Result2: %lu\n", throughput2);

        return 0;
}
-----
$ ./test
Result: 80000000 (log_test_time: 13)
Result2: 156
$ file ./test
./test: ELF 32-bit LSB pie executable ARM, EABI5 version 1 (SYSV), dynamically 
linked, interpreter /lib/ld-linux-armhf.so.3, for GNU/Linux 3.2.0, 
BuildID[sha1]=d18f32829cdd2bc42cf744cdcafde7cdbd315cb0, not stripped
-----


Regards, Linus

Reply via email to