11 Nov
2011
11 Nov
'11
3:56 p.m.
Hi,
While updating libosmocore in osmocom-bb, a problem arised: 100%CPU
usage and hanging. Turns out 'mobile' sometimes call
osmo_timer_schedule for an already scheduled timer.
Obviously this used to work and now it doesn't. The question is : Was
there a defined semantics for this case previously ? Or is it supposed
to be unsupported ? What would you expect it to do ?
Cheers,
Sylvain
11 Nov
11 Nov
6:50 p.m.
Hi all,
On Fri, Nov 11, 2011 at 03:56:21PM +0100, Sylvain Munaut wrote:
While updating libosmocore in osmocom-bb, a problem
arised: 100%CPU
usage and hanging. Turns out 'mobile' sometimes call
osmo_timer_schedule for an already scheduled timer.
Obviously this used to work and now it doesn't. The question is : Was
there a defined semantics for this case previously ? Or is it supposed
to be unsupported ? What would you expect it to do ?
I think a second/follow-up timer_schedule should re-schedule the
existing timer safely.
Regards,
Harald
--
- Harald Welte <laforge(a)gnumonks.org> http://laforge.gnumonks.org/
============================================================================
"Privacy in residential applications is a desirable marketing option."
(ETSI EN 300 175-7 Ch. A6)
12 Nov
12 Nov
10:55 p.m.
On Fri, Nov 11, 2011 at 06:50:31PM +0100, Harald Welte wrote:
Hi all,
On Fri, Nov 11, 2011 at 03:56:21PM +0100, Sylvain Munaut wrote:
The patch attached should recover the former behaviour, sorry for
missing this case.
While updating libosmocore in osmocom-bb, a
problem arised: 100%CPU
usage and hanging. Turns out 'mobile' sometimes call
osmo_timer_schedule for an already scheduled timer.
Obviously this used to work and now it doesn't. The question is : Was
there a defined semantics for this case previously ? Or is it supposed
to be unsupported ? What would you expect it to do ?
I think a second/follow-up timer_schedule should re-schedule the
existing timer safely.
11:07 p.m.
The patch attached should recover the former
behaviour, sorry for
missing this case.
Oops, sorry, we actually continued on IRC.
Look at the first few commits of
http://cgit.osmocom.org/cgit/libosmocore/log/?h=sylvain/for_jolly
They bring in some fixes/cleanups from the kernel for rbtree then
something that fixes the timer. This patch keeps the list head intact
and just remove the timer from the tree and re-adds it at the right
place.
Cheers,
Sylvain
13 Nov
13 Nov
1:28 a.m.
On Sat, Nov 12, 2011 at 11:07:03PM +0100, Sylvain Munaut wrote:
Hm, I think I found one weird scenario which is not handled fine with
your patch.
Say you have two timers A and B. Assume both expire on the same time
(and they are inserted in the eviction list in this order, first A,
then B). Then, A re-schedules B in its callback. B is reinserted
in the tree, however, B is still in the eviction list.
With aeeb7070f84437aa608a3d843346b1efa916d175, since we don't
touch the list head. B will be removed from the tree, but I think it
should remain there as it has been rescheduled.
Yes, this scenario is strange, but I think we try to cover all possible
situations. So I think my patch should be applied instead.
Thanks for collecting the patches that went into rbtree.c in the linux
kernel, btw.
The patch
attached should recover the former behaviour, sorry for
missing this case.
Oops, sorry, we actually continued on IRC.
Look at the first few commits of
http://cgit.osmocom.org/cgit/libosmocore/log/?h=sylvain/for_jolly
They bring in some fixes/cleanups from the kernel for rbtree then
something that fixes the timer. This patch keeps the list head intact
and just remove the timer from the tree and re-adds it at the right
place.
10:07 a.m.
Hm, I think I found one weird scenario which is not
handled fine with
your patch.
Say you have two timers A and B. Assume both expire on the same time
(and they are inserted in the eviction list in this order, first A,
then B). Then, A re-schedules B in its callback. B is reinserted
in the tree, however, B is still in the eviction list.
With aeeb7070f84437aa608a3d843346b1efa916d175, since we don't
touch the list head. B will be removed from the tree, but I think it
should remain there as it has been rescheduled.
Yes, this scenario is strange, but I think we try to cover all possible
situations. So I think my patch should be applied instead.
You're right. Even worse, with my patch during the scan of the
eviction list, we'll call the new B callback with the new B data at
the old time.
Thanks for collecting the patches that went into
rbtree.c in the linux
kernel, btw.
fyi, I skipped the few last ones that add a new "augmented rb tree"
functionality that we don't need (yet?).
Cheers,
Sylvain
5:06 p.m.
Hi Sylvain,
On Sun, Nov 13, 2011 at 10:07:58AM +0100, Sylvain Munaut wrote:
[...]
That skipping is fine to me, we can get it if we ever need it.
Thanks for
collecting the patches that went into rbtree.c in the linux
kernel, btw.
fyi, I skipped the few last ones that add a new "augmented rb tree"
functionality that we don't need (yet?).
4558
days inactive
4560
days old
6 comments
3 participants
participants (3)
-
Harald Welte -
Pablo Neira Ayuso -
Sylvain Munaut