Hello, this patch tries to fix http://forum.openwrt.org/viewtopic.php?pid=65184 . Broadcom's linux_timer.c contains error, which sometimes can result in 'nas' crashing. It applies against kamikaze_9.07 .
When alarm_handler is called, it stores dequeued event in its own 'event' variable. Problem arises when event's handler deletes its own handler and creates new one. Freeing an event returns it into list of free events and marks it as cancelled and deleted. Problem is that subsequent creation of new timer takes one event from the 'free' pool, and this is event we have just freed. This also erases the 'flags' field of events, with its deleted and cancelled flags, and enqueues this event to the event queue. After returning to the alarm_handler, the handler attempts to re-enqueue event which was actually processed. Normally, it would not to it, as the event was marked as cancelled, but as 'event' now points to freed-and-again-allocated event from the pool (with no sign of deletion), it sees just regular event and attempts to enqueue it, although there is one such event in the queue. Result is looped event queue. This fix adds one new flag to the 'struct event', which signifies presence of the event in event queue. alarm_handler clears this flag directly after dequeueing, and if it is set again when handler returns, the event was probably "recycled" and enqueued by timer_settime, and doesn't attempt to do any enqueuing. timer_settime is also modified to reflect this change. As I wrote in the forum, it would be great if people at Broadcom could incorporate this into their package, if it is possible (as most recent 'nas' doesn't depend on libshared.so anymore). If the inlined patch will be damaged anyhow, please download version from http://handhelds.org/~mmp/patches/brcm_libshared_avoid_reenqueue.patch If there anything more needed, please Cc: me as I'm not subscribed to this list. Please add into https://dev.openwrt.org/wiki/SubmittingPatches notice that one needs to be subscribed to openwrt-devel prior posting anything, this e-mail could have arrived six hours earlier:) Signed-off-by: Milan Plzik <[EMAIL PROTECTED]> Index: linux_timer.c =================================================================== --- linux_timer.c (revision 10594) +++ linux_timer.c (working copy) @@ -89,6 +89,7 @@ #define TFLAG_NONE 0 #define TFLAG_CANCELLED (1<<0) #define TFLAG_DELETED (1<<1) +#define TFLAG_QUEUED (1<<2) struct event { struct timeval it_interval; @@ -373,6 +374,7 @@ } event->flags &= ~TFLAG_CANCELLED; + event->flags |= TFLAG_QUEUED; unblock_timer(); @@ -470,6 +472,7 @@ event = event_queue; event_queue = event_queue->next; event->next = NULL; + event->flags &= ~TFLAG_QUEUED; #ifdef TIMER_PROFILE end = uclock(); @@ -483,7 +486,15 @@ (*(event->func))((timer_t) event, (int)event->arg); /* If the event has been cancelled, do NOT put it back on the queue. */ - if ( !(event->flags & TFLAG_CANCELLED) ) { + /* Check for TFLAG_QUEUED is to avoid pathologic case, when after + * dequeueing event handler deletes its own timer and allocates new one + * which (at least in some cases) gets the same pointer and thus its + * 'flags' will be rewritten, most notably TFLAG_CANCELLED, and, to + * complete the disaster, it will be queued. alarm_handler tries to + * enqueue 'event' (which is on the same memory position as newly + * allocated timer), which results in queueing the same pointer once + * more. And this way, loop in event queue is created. */ + if ( !(event->flags & TFLAG_CANCELLED) && !(event->flags & TFLAG_QUEUED) ) { // if the event is a recurring event, reset the timer and // find its correct place in the sorted list of events. @@ -518,6 +529,7 @@ // link our new event into the pending event queue. event->next = *ppevent; *ppevent = event; + event->flags |= TFLAG_QUEUED; } else { // there is no interval, so recycle the event structure. //timer_delete((timer_t) event); _______________________________________________ openwrt-devel mailing list [email protected] http://lists.openwrt.org/cgi-bin/mailman/listinfo/openwrt-devel
