From c33b11d09f526c156760b24bb88c9100f107a042 Mon Sep 17 00:00:00 2001 From: Fabrice Fontaine Date: Tue, 28 Sep 2010 13:27:37 +0200 Subject: [PATCH] Bug fix on burst of GENA notification When a lot of notifications were generated by a device in a short period of time then 100% of the CPU was used to reorder those notifications by pushing back the thread in the job queue. This mechanism has been modified so now thread sleep 1 ms before being pushed back into the job queue. Removing DEFAULT_SCHED_PARAM parameter and use sched_get_priority_min(DEFAULT_POLICY) instead. --- ChangeLog | 13 +++++++++++++ threadutil/inc/ThreadPool.h | 4 ---- threadutil/src/ThreadPool.c | 2 +- upnp/src/gena/gena_device.c | 13 ++++++++++++- 4 files changed, 26 insertions(+), 6 deletions(-) diff --git a/ChangeLog b/ChangeLog index 1c781cf..7d4d4a9 100644 --- a/ChangeLog +++ b/ChangeLog @@ -2,6 +2,19 @@ Version 1.6.7 ******************************************************************************* +2010-09-28 Marc Essayan + + Bug fix on burst of GENA notification + + When a lot of notifications were generated by a device in a short + period of time then 100% of the CPU was used to reorder those + notifications by pushing back the thread in the job queue. This + mechanism has been modified so now thread sleep 1 ms before being + pushed back into the job queue. + + Removing DEFAULT_SCHED_PARAM parameter and use + sched_get_priority_min(DEFAULT_POLICY) instead. + 2010-09-22 Fabrice Fontaine Bug fix on M-SEARCH response diff --git a/threadutil/inc/ThreadPool.h b/threadutil/inc/ThreadPool.h index c33e97c..0b1044f 100644 --- a/threadutil/inc/ThreadPool.h +++ b/threadutil/inc/ThreadPool.h @@ -158,10 +158,6 @@ typedef int PolicyType; #define DEFAULT_POLICY SCHED_OTHER -/*! Default priority */ -#define DEFAULT_SCHED_PARAM 0 - - /**************************************************************************** * Name: free_routine * diff --git a/threadutil/src/ThreadPool.c b/threadutil/src/ThreadPool.c index da8a91c..d183c75 100644 --- a/threadutil/src/ThreadPool.c +++ b/threadutil/src/ThreadPool.c @@ -246,7 +246,7 @@ static int SetPolicyType(PolicyType in) memset(¤t, 0, sizeof(current)); sched_getparam(0, ¤t); - current.sched_priority = DEFAULT_SCHED_PARAM; + current.sched_priority = sched_get_priority_min(DEFAULT_POLICY); sched_result = sched_setscheduler(0, in, ¤t); retVal = (sched_result != -1 || errno == EPERM) ? 0 : errno; #else diff --git a/upnp/src/gena/gena_device.c b/upnp/src/gena/gena_device.c index 4d60131..0c867ae 100644 --- a/upnp/src/gena/gena_device.c +++ b/upnp/src/gena/gena_device.c @@ -342,7 +342,12 @@ static void genaNotifyThread( struct Handle_Info *handle_info; ThreadPoolJob job; - HandleReadLock(); + /* This should be a HandleLock and not a HandleReadLock otherwise if there + * is a lot of notifications, then multiple threads will acquire a read + * lock and the thread which sends the notification will be blocked forever + * on the HandleLock at the end of this function. */ + //HandleReadLock(); + HandleLock(); //validate context if( GetHandleInfo( in->device_handle, &handle_info ) != HND_DEVICE ) { @@ -366,6 +371,12 @@ static void genaNotifyThread( TPJobInit( &job, ( start_routine ) genaNotifyThread, input ); TPJobSetFreeFunction( &job, ( free_function ) free_notify_struct ); TPJobSetPriority( &job, MED_PRIORITY ); + + /* Sleep a little before creating another thread otherwise if there is + * a lot of notifications to send, the device will take 100% of the CPU + * to create threads and push them back to the job queue. */ + imillisleep( 1 ); + ThreadPoolAdd( &gSendThreadPool, &job, NULL ); freeSubscription( &sub_copy );