Re: [Xen-devel] [PATCH net] xen-netback: add the scenario which now beyond the range time_after_eq().

[annie li <annie.li@oracle.com>]

On 2013-10-17 23:41, jianhai luan wrote:
>
> On 2013-10-17 23:25, David Vrabel wrote:
>> On 17/10/13 16:23, jianhai luan wrote:
>>> On 2013-10-17 22:06, Wei Liu wrote:
>>>> On Thu, Oct 17, 2013 at 09:59:30PM +0800, jianhai luan wrote:
>>>> [...]
>>>>>>>>> If use time_after_eq64(), expire ,next_credit and other member
>>>>>>>>> will must
>>>>>>>>> be u64.
>>>>>>>> Yes, you'll need to store next_credit as a u64 in vif instead of
>>>>>>>> calculating it in tx_credit_exceeded from expires (which is 
>>>>>>>> only an
>>>>>>>> unsigned long).
>>>>>>> I know that.  Even we use u64, time_after_eq()  will also do wrong
>>>>>>> judge
>>>>>>> in theory (not in reality because need long long time).
>>>>>> If jiffies_64 has millisecond resolution that would be more than
>>>>>> 500,000,000 years.
>>>>> Yes, I agree the fact.
>>>>>>> I think the two better fixed way is below:
>>>>>>>     - By time_before() to judge if now beyond MAX_ULONG/2
>>>>>> This is broken, so no.
>>>>> Where is broken?  would you like to help me point it out.
>>>> I think David means you didn't actually fix the problem. Your 
>>>> solution is
>>>> merely a workaround.
>>> I have think  about using u64, but more code need to be modified and
>>> that is not all.  Key point is how to change the element of struct
>>> time_list (expires)  and don't affect other thing?
>> I already suggested a way that didn't require changing the timer
>> structure -- calculate and store next_credit in advanced.
> I think that  modify next_credit only  will not fix the issue. please 
> think about:
>   - If jiffies have beyond 32 bit. i assume expire  is 0, jiffies_64 
> is  0x1000000ff.
>     next_credit = 0 +  <always 32-bit value >
>
>     time_after_eq64(jiffies_64, next_credit ) will always true. 
> replenish will always do, rate control will lost their function.

At first, the case above only exists when the network device keep idle 
for a long time, not frequently happens. If this case really happens, it 
means lots of jiffies are available for the credit, so there is no 
necessary to add the timer. The code operates correctly and rate control 
does not lose. This case can be shown with following config,
------old_next_credit(expires replaced)----------next_credit--------now----

So till now, two solutions are available: one is the current one to 
change if condition, another is to change all connected variant to 64. I 
incline to the former one since it involves less code change than the 
latter one.

Thanks
Annie