kernel: timer: looping issue, need reset variable 'found'
PATCH 1/1 kernel: timer: looping issue, need reset variable 'found'
According to __internal_add_timer(), in _next_timer_interrupt(), when 'tv1.vec' find one, but need 'cascade bucket(s)', we still need find each slot of 'tv*.vec'.
So need reset variable 'found', so can fully scan ''do {...} while()'' for 'tv*.vec'.
--- kernel/timer.c | 6 +++--- 1 files changed, 3 insertions(+), 3 deletions(-)
kernel/timer.c +3 -3
@@ -1197,7 +1197,7 @@ static unsigned long __next_timer_interrupt(struct tvec_base *base) {unsigned long timer_jiffies = base->timer_jiffies;unsigned long expires = timer_jiffies +NEXT_TIMER_MAX_DELTA ; -int index, slot, array, found =0 ; +int index, slot, array;struct timer_list *nte;struct tvec *varray[4 ];
@@ -1208,7 +1208,6 @@ static unsigned long __next_timer_interrupt(struct tvec_base *base)if (tbase_get_deferrable (nte->base))continue ; - found =1 ; expires = nte->expires;/* Look at the cascade bucket(s)? */ if (!index || slot < index)
@@ -1232,6 +1231,7 @@ cascade:for (array =0 ; array <4 ; array++) {struct tvec *varp = varray[array]; +bool found = false; index = slot = timer_jiffies &TVN_MASK ; do {
@@ -1239,7 +1239,7 @@ cascade:if (tbase_get_deferrable (nte->base))continue ; - found =1 ; + found = true;if (time_before (nte->expires, expires)) expires = nte->expires; }
Signature · 1 line
1.7.7.6
Quoted 4 lines · Chen Gang
According to
__internal_add_timer(), in_next_timer_interrupt(), when 'tv1.vec' find one, but need 'cascade bucket(s)', we still need find each slot of 'tv*.vec'.
No, we do not. We only need to scan the first cascade array after the enqueued timer. If there is nothing in tv2 which might come before the found timer, then any timer in tv3 will be later than the one we found in the primary wheel.
Quoted 2 lines
So need reset variable 'found', so can fully scan ''do {...}
while()'' for 'tv*.vec'.
And thereby lose the information, that we already found a timer in the scan of the primary array.
Thanks,
tglx
Sorry for replying late during these days, firstly.
Quoted 11 lines · Thomas Gleixner
Quoted 4 lines · Chen Gang
Quoted 4 lines
According to
__internal_add_timer(), in_next_timer_interrupt(), when 'tv1.vec' find one, but need 'cascade bucket(s)', we still need find each slot of 'tv*.vec'.No, we do not. We only need to scan the first cascade array after the enqueued timer. If there is nothing in tv2 which might come before the found timer, then any timer in tv3 will be later than the one we found in the primary wheel.
OK, thanks, I should read details to confirm it (at least now, I still not quite be sure about it).
Quoted 4 lines
Quoted 2 lines
Quoted 2 lines
So need reset variable 'found', so can fully scan ''do {...}
while()'' for 'tv*.vec'.And thereby lose the information, that we already found a timer in the scan of the primary array.
Hmm..., I should read details.
And excuse me, I also have to do another things during these days, so maybe reply late.
Thanks.
Signature · 3 lines
Chen Gang Asianux Corporation
Sorry again for replying late, and now I can focus on this patch, I will see the related details again.
Thanks.
Quoted 35 lines · Chen Gang
Sorry for replying late during these days, firstly.
Quoted 11 lines · Thomas Gleixner
Quoted 4 lines · Chen Gang
Quoted 4 lines
According to
__internal_add_timer(), in_next_timer_interrupt(), when 'tv1.vec' find one, but need 'cascade bucket(s)', we still need find each slot of 'tv*.vec'.No, we do not. We only need to scan the first cascade array after the enqueued timer. If there is nothing in tv2 which might come before the found timer, then any timer in tv3 will be later than the one we found in the primary wheel.
OK, thanks, I should read details to confirm it (at least now, I still not quite be sure about it).
Quoted 4 lines
Quoted 2 lines
Quoted 2 lines
So need reset variable 'found', so can fully scan ''do {...}
while()'' for 'tv*.vec'.And thereby lose the information, that we already found a timer in the scan of the primary array.
Hmm..., I should read details.
And excuse me, I also have to do another things during these days, so maybe reply late.
Thanks.
Signature · 3 lines
Chen Gang Asianux Corporation
Quoted 12 lines · Thomas Gleixner
Quoted 4 lines · Chen Gang
According to
__internal_add_timer(), in_next_timer_interrupt(), when 'tv1.vec' find one, but need 'cascade bucket(s)', we still need find each slot of 'tv*.vec'.No, we do not. We only need to scan the first cascade array after the enqueued timer. If there is nothing in tv2 which might come before the found timer, then any timer in tv3 will be later than the one we found in the primary wheel.
If we assume "If there is nothing in tv2 which might come before the found timer, then any timer in tv3 will ..." is correct.
When we found a timer in 'tv1', we will not search all timers in 'tv2' (we only search first looping of tv2 for the specific 'slot').
Is it still OK ?
Quoted 6 lines
Quoted 2 lines
So need reset variable 'found', so can fully scan ''do {...}
while()'' for 'tv*.vec'.And thereby lose the information, that we already found a timer in the scan of the primary array.
When we found a timer, 'expires' would be set. So resetting 'found' is still correct, but may let performance lower (if original implement is correct too)
I think we can treat original implementation as for speed optimization, so our discussion is "whether this speed optimization has effect with correctness".
Thanks.
Signature · 3 lines
Chen Gang Asianux Corporation
Quoted 4 lines · Chen Gang
On 06/10/2013 10:12 PM, Thomas Gleixner wrote:
I think we can treat original implementation as for speed optimization, so our discussion is "whether this speed optimization has effect with correctness".
Then I recommend that you to sit down and analyze the correctness of the code.
Come back when you have a proof of the code being wrong. And by proof I mean factual proof not just handwaving theories.
Thanks,
tglx
Quoted 7 lines · Thomas Gleixner
Quoted 4 lines · Chen Gang
Quoted 4 lines · Thomas Gleixner
I think we can treat original implementation as for speed optimization, so our discussion is "whether this speed optimization has effect with correctness".
Then I recommend that you to sit down and analyze the correctness of the code.
That is only your recommend, not mean I have duty to.
Quoted 3 lines
Come back when you have a proof of the code being wrong. And by proof I mean factual proof not just handwaving theories.
At least before say so, please reply:
-------------------------------------------------------------------------------
If we assume "If there is nothing in tv2 which might come before the found timer, then any timer in tv3 will ..." is correct.
When we found a timer in 'tv1', we will not search all timers in 'tv2' (we only search first looping of tv2 for the specific 'slot').
Is it still OK ?
-------------------------------------------------------------------------------
If you do not want to discuss with others, better quite politely, not need judging or checking others, it is useless for the cooperation with each other, is it right ? ;-)
Thanks.
Signature · 3 lines
Chen Gang Asianux Corporation
Quoted 7 lines · Chen Gang
On 06/20/2013 03:47 PM, Thomas Gleixner wrote:
-------------------------------------------------------------------------------
If we assume "If there is nothing in tv2 which might come before the found timer, then any timer in tv3 will ..." is correct.When we found a timer in 'tv1', we will not search all timers in 'tv2' (we only search first looping of tv2 for the specific 'slot').
Yes, because that's how the timer wheel works. And I'm not going to explain you every little detail of it.
Quoted 1 line
Is it still OK ?
Yes, it is.
Quoted 3 lines
If you do not want to discuss with others, better quite politely, not need judging or checking others, it is useless for the cooperation with each other, is it right ? ;-)
I discussed all your patches which fall into my area of responsibility with you and I explained to you very politely why your patches are incorrect.
When I noticed, that you do not even understand how the timer wheel works in detail, which is necessary to understand why the code in __next_timer_interrupt() is correct, I asked you politely:
Quoted 2 lines
Quoted 2 lines
Then I recommend that you to sit down and analyze the correctness of the code.
And you answered:
Quoted 1 line
That is only your recommend, not mean I have duty to.
Right, it's only a recommendation. Though without proof of a failure, I'm not going to discuss that further and I'm not going to apply a patch.
Thanks,
tglx
Quoted 11 lines · Thomas Gleixner
Quoted 7 lines · Chen Gang
Quoted 7 lines · Thomas Gleixner
-------------------------------------------------------------------------------
If we assume "If there is nothing in tv2 which might come before the found timer, then any timer in tv3 will ..." is correct.When we found a timer in 'tv1', we will not search all timers in 'tv2' (we only search first looping of tv2 for the specific 'slot').
Yes, because that's how the timer wheel works. And I'm not going to explain you every little detail of it.
OK, thanks.
Quoted 3 lines
Quoted 1 line
Quoted 1 line
Is it still OK ?
Yes, it is.
OK, thanks.
Quoted 7 lines
Quoted 3 lines
Quoted 3 lines
If you do not want to discuss with others, better quite politely, not need judging or checking others, it is useless for the cooperation with each other, is it right ? ;-)
I discussed all your patches which fall into my area of responsibility with you and I explained to you very politely why your patches are incorrect.
Firstly, thank you for your explanations in details in your area of responsibility.
At least this means you are warm-hearted, and can spend your time resources on it.
Quoted 4 lines
When I noticed, that you do not even understand how the timer wheel works in detail, which is necessary to understand why the code in
__next_timer_interrupt()is correct, I asked you politely:
I really do not know about the timer wheel works in detail.
I never said I am familiar with kernel, in fact, I often said I am not quite familiar with kernel (you can find it in my many other replying mails).
Is it necessary to give some related comments for it (the 'found' is really in an informal using way) ?
We'd better let our code much readable for 'many readers' (which may not know about more details, e.g. timer wheel).
Do my reply politely ? ;-)
Quoted 8 lines
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Then I recommend that you to sit down and analyze the correctness of the code.
And you answered:
Quoted 1 line
Quoted 1 line
That is only your recommend, not mean I have duty to.
Right, it's only a recommendation. Though without proof of a failure, I'm not going to discuss that further and I'm not going to apply a patch.
OK.
For cooperation, every members are joined with their own willing, not by force, but really need duty.
This time, it does not need duty, it is really a recommend.
Thanks.
Signature · 3 lines
Chen Gang Asianux Corporation