This is the mail archive of the ecos-patches@sources.redhat.com mailing list for the eCos project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: timed condition variable patch


Nick Garnett <nickg@calivar.demon.co.uk> writes:

> I can see two solutions:
> 
> 1. Remove the assertion from unlock_inner(). This was useful during
>    development for catching programming bugs in the kernel. The kernel
>    is now reasonably stable, and the added semantics of
>    unlock_reschedule() now make it less useful -- and in this case a
>    positive menace. We have has problems with this assertion before,
>    so maybe it is time for it to go.
> 
> 2. Fix the calculation of the end time in select() so that the current
>    time is only added after the scheduler has been locked and it
>    cannot advance any further.
> 
> Neither of these is ideal, but both should probably be applied.
> 
> I'll put together a patch to do this.
> 

And here it is:

Index: io/fileio/current/ChangeLog
===================================================================
RCS file: /cvs/ecos/ecos/packages/io/fileio/current/ChangeLog,v
retrieving revision 1.21
diff -u -r1.21 ChangeLog
--- io/fileio/current/ChangeLog	29 May 2002 18:28:24 -0000	1.21
+++ io/fileio/current/ChangeLog	8 Aug 2002 12:56:57 -0000
@@ -1,3 +1,9 @@
+2002-08-08  Nick Garnett  <nickg@calivar.demon.co.uk>
+
+	* src/select.cxx (select): Changed mechanism for calculating
+	select timeout to avoid possible race conditions between this code
+	and the timer DSR.
+
 2002-05-29  Jesper Skov  <jskov@redhat.com>
 
 	* tests/fileio1.c: Removed strcat definition. Rely on string.h to
Index: io/fileio/current/src/select.cxx
===================================================================
RCS file: /cvs/ecos/ecos/packages/io/fileio/current/src/select.cxx,v
retrieving revision 1.7
diff -u -r1.7 select.cxx
--- io/fileio/current/src/select.cxx	23 May 2002 23:06:10 -0000	1.7
+++ io/fileio/current/src/select.cxx	8 Aug 2002 12:56:59 -0000
@@ -162,8 +162,7 @@
 
     // Compute end time
     if (tv)
-        ticks = Cyg_Clock::real_time_clock->current_value() +
-            cyg_timeval_to_ticks( tv );
+        ticks = cyg_timeval_to_ticks( tv );
     else ticks = 0;
 
     // Lock the mutex
@@ -226,11 +225,14 @@
                     FILEIO_RETURN_VALUE(0);
                 }
 
+                ticks += Cyg_Clock::real_time_clock->current_value();
+                
                 if( !selwait.wait( ticks ) )
                 {
                     // A non-standard wakeup, if the current time is equal to
                     // or past the timeout, return zero. Otherwise return
                     // EINTR, since we have been released.
+
                     if( Cyg_Clock::real_time_clock->current_value() >= ticks )
                     {
                         select_mutex.unlock();
@@ -239,6 +241,8 @@
                     }
                     else error = EINTR;
                 }
+
+                ticks -= Cyg_Clock::real_time_clock->current_value();
             }
             else
             {
Index: kernel/current/ChangeLog
===================================================================
RCS file: /cvs/ecos/ecos/packages/kernel/current/ChangeLog,v
retrieving revision 1.79
diff -u -r1.79 ChangeLog
--- kernel/current/ChangeLog	5 Aug 2002 21:52:58 -0000	1.79
+++ kernel/current/ChangeLog	8 Aug 2002 12:57:26 -0000
@@ -1,3 +1,10 @@
+2002-08-08  Nick Garnett  <nickg@calivar.demon.co.uk>
+
+	* src/sched/sched.cxx (unlock_inner): Removed initial
+	assertion. This has served its purpose and with the introduction
+	of routines such as unlock_reschedule() is prone to firing in
+	otherwise benign circumstances.
+
 2002-08-05  Bart Veer  <bartv@tymora.demon.co.uk>
 
 	* cdl/kernel.cdl, include/kapidata.h, include/kapi.h:
Index: kernel/current/src/sched/sched.cxx
===================================================================
RCS file: /cvs/ecos/ecos/packages/kernel/current/src/sched/sched.cxx,v
retrieving revision 1.15
diff -u -r1.15 sched.cxx
--- kernel/current/src/sched/sched.cxx	23 May 2002 23:06:54 -0000	1.15
+++ kernel/current/src/sched/sched.cxx	8 Aug 2002 12:57:30 -0000
@@ -141,14 +141,6 @@
     CYG_REPORT_FUNCTION();
 #endif    
 
-    // This assert must be outside the loop because running DSRs can make
-    // it fail if the current thread that was about to sleep is awoken by
-    // the DSR!  Going round the loop to run new DSRs does the same.
-    CYG_ASSERT( (new_lock == 0) ||
-                (get_current_thread()->state != Cyg_Thread::RUNNING ||
-                 get_need_reschedule()) ,
-                "Unnecessary call to unlock_inner()" );
-        
     do {
 
         CYG_PRECONDITION( new_lock==0 ? get_sched_lock() == 1 :


-- 
Nick Garnett - eCos Kernel Architect
http://www.eCosCentric.com/


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]