This is the mail archive of the
ecos-patches@sources.redhat.com
mailing list for the eCos project.
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/