Re: [PATCH] pkt_sched: gen_estimator: use 64 bits intermediate counters for bps

[Eric Dumazet <dada1@cosmosbay.com>]

Jarek Poplawski a écrit :
> On Tue, May 19, 2009 at 01:59:55AM +0200, Eric Dumazet wrote:
> ...
>> diff --git a/net/core/gen_estimator.c b/net/core/gen_estimator.c
> ...
>> -		e->avbps += ((long)rate - (long)e->avbps) >> e->ewma_log;
>> +		e->avbps += ((s64)(brate - e->avbps)) >> e->ewma_log;
> 
> Btw., I'm a bit concerned about the syntax here: isn't such shifting
> of signed ints implementation dependant?
> 

You are right Jarek, I very often forget to never ever use signed quantities
at all ! (But also note original code has same undefined behavior)


Quoting wikipedia : (http://en.wikipedia.org/wiki/Arithmetic_shift)

The (1999) ISO standard for the, C programming language defines the C language's 
right shift operator in terms of divisions by powers of 2. Because of the 
aforementioned non-equivalence, the standard explicitly excludes from that
 definition the right shifts of signed numbers that have negative values.
 It doesn't specify the behaviour of the right shift operator in such circumstances,
 but instead requires each individual C compiler to specify the behaviour of shifting 
negative values right.

Apparently gcc does the *right* thing on x86_32, but we probably want something
stronger here. I could not find gcc documentation statement on right shifts of 
negative values.


 436:   8b 4b 14                mov    0x14(%ebx),%ecx
 439:   89 73 18                mov    %esi,0x18(%ebx)
 43c:   89 7b 1c                mov    %edi,0x1c(%ebx)
 43f:   8b 73 20                mov    0x20(%ebx),%esi
 442:   8b 7b 24                mov    0x24(%ebx),%edi
 445:   29 f0                   sub    %esi,%eax
 447:   19 fa                   sbb    %edi,%edx
 449:   0f ad d0                shrd   %cl,%edx,%eax
 44c:   d3 fa                   sar    %cl,%edx         << good >>
 44e:   f6 c1 20                test   $0x20,%cl
 451:   74 05                   je     458 <est_timer+0xb8>
 453:   89 d0                   mov    %edx,%eax
 455:   c1 fa 1f                sar    $0x1f,%edx       
 458:   01 f0                   add    %esi,%eax
 45a:   8b 4b 0c                mov    0xc(%ebx),%ecx
 45d:   89 43 20                mov    %eax,0x20(%ebx)
 460:   11 fa                   adc    %edi,%edx
 462:   83 c0 0f                add    $0xf,%eax
 465:   89 53 24                mov    %edx,0x24(%ebx)
 468:   83 d2 00                adc    $0x0,%edx
 46b:   0f ac d0 05             shrd   $0x5,%edx,%eax
 46f:   89 01                   mov    %eax,(%ecx)