[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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




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


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.