>> + start_bh_atomic();
>> del_timer(&shaper->timer);
>> + end_bh_atomic();
>> MOD_DEC_USE_COUNT;
>> return 0;
>> }
>
>Isn't that part of the patch overkill?
No, if the shaper->timer has some chance to be pending (and if it had no
chance to be pending also del_timer() would be an overkill).
>As I understand things, del_timer() must acquire the timerlist_lock
>spinlock to modify the timer list. I guess that all other code
The race is very more fun ;). The race is that you may have the
shaper->timer running on the other CPU while you are deleting it. Yes, the
timerlist is preserved from corruption, but you may remove the vmalloced
module code while the shaper->timer is running.
>(including the code in bottom half handlers) that want to modify the
>timer list do the same.
shaper_kick() now uses mod_timer() (now I can use mod_timer() because I
rewrote the locking scheme) to be sure that the shaper->timer will not
added two times causing the timer_bh() to loop forever.
>So why do we need to use [start|end]_bh_atomic() to guarantee that no
>BH on any CPU will execute while we modify the timer list?
No. The timer list has nothing to do this time with the bh_atomic(). The
bh_atomic will assure us that once we are in line xxxxxxxxxxx:
start_bh_atomic();
del_timer(&shaper->timer);
end_bh_atomic();
xxxxxxxxxxxx
we won't have the shaper->timer pending or _running_ anymore.
Andrea Arcangeli
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.rutgers.edu
Please read the FAQ at http://www.tux.org/lkml/