This is the mail archive of the ecos-patches@sourceware.org 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: FIFO DSRs... the 2nd


Sergei Organov <osv@javad.com> writes:

> Attached is modified patch that implements FIFO scheduling of DSRs and
> makes it the default.
> 
> Compared to the original patch, less files are modified, more assertions
> are added, and FIFO variant of the call_pending_DSRs_inner() is
> optimized both for single DSR and for multiple DSRs in the list.
> 
> In addition, two related minor fixes to the comments in the kernel tests
> are provided.
> 
> Tested on ARM.

This has now been checked in, the actual patch committed is attached.



Index: hal/sh/arch/current/src/context.S
===================================================================
RCS file: /cvs/ecos/ecos/packages/hal/sh/arch/current/src/context.S,v
retrieving revision 1.6
diff -u -5 -r1.6 context.S
--- hal/sh/arch/current/src/context.S	5 Dec 2003 17:06:23 -0000	1.6
+++ hal/sh/arch/current/src/context.S	11 Aug 2006 09:20:18 -0000
@@ -238,15 +238,15 @@
         lds.l   @r0+,fpscr
 #endif
         
         lds.l   @r0+,pr                 ! pr
 
-        mov     r3,r15                  ! update stack pointer
-
         mov.l   @r0+,r2                 ! SR
         hal_cpu_int_merge r2,r0,r1      ! restore interrupt state
 
+        mov     r3,r15                  ! update stack pointer
+
         rts                             ! and return
          nop
 
 #------------------------------------------------------------------------------
 # HAL longjmp, setjmp implementations
Index: kernel/current/ChangeLog
===================================================================
RCS file: /cvs/ecos/ecos/packages/kernel/current/ChangeLog,v
retrieving revision 1.135
diff -u -5 -r1.135 ChangeLog
--- kernel/current/ChangeLog	19 May 2006 10:15:43 -0000	1.135
+++ kernel/current/ChangeLog	11 Aug 2006 09:20:29 -0000
@@ -9,10 +9,28 @@
 
 2006-04-11  Sergei Organov <osv@javad.com>
 
 	* doc/kernel.sgml: Fix typo
 
+2006-04-10  Sergei Organov  <osv@javad.com>
+
+	Implement FIFO variant of scheduling of DSRs and make it the
+	default. This is reworked patch originally suggested by Stefan
+	Sommerfeld <sommerfeld@mikrom.com>.
+
+	* cdl/interrupts.cdl (CYGIMP_KERNEL_INTERRUPTS_DSRS_LIST): make it
+	cdl_component.
+	* cdl/interrupts.cdl (CYGSEM_KERNEL_INTERRUPTS_DSRS_LIST_FIFO):
+	new option for CYGIMP_KERNEL_INTERRUPTS_DSRS_LIST.
+	* include/intr.hxx (class Cyg_Interrupt): new static variable
+	dsr_list_tail.
+	* src/intr/intr.cxx (call_pending_DSRs_inner): add
+	CYGSEM_KERNEL_INTERRUPTS_DSRS_LIST_FIFO variant.
+	(post_dsr): likewise.
+	* tests/intr0.cxx: fix comments to match actual option names.
+	* tests/kintr0.c: likewise.2006-03-27  Marco Cruz  <marco@daruma.com.br>
+	
 2006-03-27  Marco Cruz  <marco@daruma.com.br>
 
 	* include/thread.hxx: removed extra qualifier of
 	Cyg_Thread::reinitialize() to permit compile on gcc 4.1.0
 	* include/sched.hxx: removed extra qualifier of
Index: kernel/current/cdl/interrupts.cdl
===================================================================
RCS file: /cvs/ecos/ecos/packages/kernel/current/cdl/interrupts.cdl,v
retrieving revision 1.4
diff -u -5 -r1.4 interrupts.cdl
--- kernel/current/cdl/interrupts.cdl	23 May 2002 23:06:45 -0000	1.4
+++ kernel/current/cdl/interrupts.cdl	11 Aug 2006 09:20:29 -0000
@@ -72,21 +72,37 @@
     }
 
     # NOTE: the choice of list vs table should not be two separate
     # options. There is a single option which must have one of
     # two legal values.
-    cdl_option CYGIMP_KERNEL_INTERRUPTS_DSRS_LIST {
+    cdl_component CYGIMP_KERNEL_INTERRUPTS_DSRS_LIST {
         display       "Use linked lists for DSRs"
         default_value 1
         implements    CYGINT_KERNEL_INTERRUPTS_DSRS
         description   "
             When DSR support is enabled the kernel must keep track of all
             the DSRs that are pending. This information can be kept in a
             fixed-size table or in a linked list. The list implementation
             requires that the kernel disable interrupts for a very short
             period of time outside interrupt handlers, but there is no
             possibility of a table overflow occurring."
+
+        cdl_option CYGSEM_KERNEL_INTERRUPTS_DSRS_LIST_FIFO {
+            display       "Schedule DSRs in FIFO order"
+            flavor        bool
+            default_value 1
+            description   "When this option is set, DSRs are scheduled
+                in the natural FIFO (first in, first out) order,
+                otherwise they are scheduled in LIFO (last in, first
+                out) order. Applications should not rely on any
+                particular order of scheduling of DSRs. LIFO
+                scheduling is kept for backward compatibility only and
+                is not recommended as it may lead to high (up to 2
+                times higher then FIFO) IRQ-to-DSR latencies at some
+                (typically rare) conditions. If unsure, leave this set."
+        }
+
     }
 
     cdl_component CYGIMP_KERNEL_INTERRUPTS_DSRS_TABLE {
         display       "Use fixed-size table for DSRs"
         default_value 0
Index: kernel/current/include/intr.hxx
===================================================================
RCS file: /cvs/ecos/ecos/packages/kernel/current/include/intr.hxx,v
retrieving revision 1.11
diff -u -5 -r1.11 intr.hxx
--- kernel/current/include/intr.hxx	23 May 2002 23:06:47 -0000	1.11
+++ kernel/current/include/intr.hxx	11 Aug 2006 09:20:30 -0000
@@ -193,15 +193,21 @@
     volatile cyg_ucount32 dsr_count CYGBLD_ANNOTATE_VARIABLE_INTR; 
 
     // next DSR in list
     Cyg_Interrupt* volatile next_dsr CYGBLD_ANNOTATE_VARIABLE_INTR; 
 
-    // static list of pending DSRs
+    // head of static list of pending DSRs
     static Cyg_Interrupt* volatile dsr_list[CYGNUM_KERNEL_CPU_MAX]
                                            CYGBLD_ANNOTATE_VARIABLE_INTR;
-    
-#endif
+
+#  ifdef CYGSEM_KERNEL_INTERRUPTS_DSRS_LIST_FIFO
+    // tail of static list of pending DSRs
+    static Cyg_Interrupt* volatile dsr_list_tail[CYGNUM_KERNEL_CPU_MAX]
+                                           CYGBLD_ANNOTATE_VARIABLE_INTR;
+#  endif
+
+#endif  // defined  CYGIMP_KERNEL_INTERRUPTS_DSRS_LIST
 
 #ifdef CYGIMP_KERNEL_INTERRUPTS_CHAIN
 
     // The default mechanism for handling interrupts is to attach just
     // one Interrupt object to each vector. In some cases, and on some
Index: kernel/current/src/intr/intr.cxx
===================================================================
RCS file: /cvs/ecos/ecos/packages/kernel/current/src/intr/intr.cxx,v
retrieving revision 1.17
diff -u -5 -r1.17 intr.cxx
--- kernel/current/src/intr/intr.cxx	23 May 2002 23:06:54 -0000	1.17
+++ kernel/current/src/intr/intr.cxx	11 Aug 2006 09:20:30 -0000
@@ -135,10 +135,14 @@
 
 #ifdef CYGIMP_KERNEL_INTERRUPTS_DSRS_LIST
 
 Cyg_Interrupt* volatile Cyg_Interrupt::dsr_list[CYGNUM_KERNEL_CPU_MAX];
 
+#  ifdef CYGSEM_KERNEL_INTERRUPTS_DSRS_LIST_FIFO
+Cyg_Interrupt* volatile Cyg_Interrupt::dsr_list_tail[CYGNUM_KERNEL_CPU_MAX];
+#  endif
+
 #endif
 
 // -------------------------------------------------------------------------
 // Call any pending DSRs
 
@@ -168,10 +172,39 @@
     
 #endif
 
 #ifdef CYGIMP_KERNEL_INTERRUPTS_DSRS_LIST
 
+#  ifdef CYGSEM_KERNEL_INTERRUPTS_DSRS_LIST_FIFO
+
+    cyg_uint32 old_intr;
+    HAL_DISABLE_INTERRUPTS(old_intr);
+    Cyg_Interrupt* intr = dsr_list[cpu];
+    CYG_ASSERT(intr != 0, "No DSRs are pended");
+    dsr_list[cpu] = 0;
+    dsr_list_tail[cpu] = 0;
+    while(true)
+    {
+        cyg_count32 count = intr->dsr_count;
+        Cyg_Interrupt* next = intr->next_dsr;
+        intr->dsr_count = 0;
+        intr->next_dsr = 0;
+        HAL_RESTORE_INTERRUPTS(old_intr);
+
+        CYG_ASSERT(intr->dsr != 0, "No DSR defined");
+        CYG_ASSERT(count > 0, "DSR posted but post count is zero");
+        intr->dsr(intr->vector, count, (CYG_ADDRWORD)intr->data);
+
+        if (!next)
+            break;
+
+        intr = next;
+        HAL_DISABLE_INTERRUPTS(old_intr);
+    }
+
+#  else // ! defined CYGSEM_KERNEL_INTERRUPTS_DSRS_LIST_FIFO
+    
     while( dsr_list[cpu] != NULL )
     {
         Cyg_Interrupt* intr;
         cyg_uint32 old_intr;
         cyg_count32 count;
@@ -189,12 +222,14 @@
 
         intr->dsr( intr->vector, count, (CYG_ADDRWORD)intr->data );
         
     }
     
-#endif
-    
+#  endif  // ! defined CYGSEM_KERNEL_INTERRUPTS_DSRS_LIST_FIFO
+
+#endif  // defined CYGIMP_KERNEL_INTERRUPTS_DSRS_LIST
+
 };
 
 externC void
 cyg_interrupt_call_pending_DSRs(void)
 {
@@ -243,21 +278,43 @@
 
 #ifdef CYGIMP_KERNEL_INTERRUPTS_DSRS_LIST
 
     // Only add the interrupt to the dsr list if this is
     // the first DSR call.
-    // At present DSRs are pushed onto the list and will be
-    // called in reverse order. We do not define the order
-    // in which DSRs are called, so this is acceptable.
-    
     if( dsr_count++ == 0 )
     {
+#  ifdef CYGSEM_KERNEL_INTERRUPTS_DSRS_LIST_FIFO
+
+        // Add to the tail of the list.
+        Cyg_Interrupt* tail = dsr_list_tail[cpu];
+        dsr_list_tail[cpu] = this;
+        if( tail )
+        {
+            CYG_ASSERT( 0 != dsr_list[cpu] ,
+              "DSR list is not empty but its head is 0");
+            tail->next_dsr = this;
+        }
+        else
+        {
+            CYG_ASSERT( 0 == dsr_list[cpu] ,
+              "DSR list tail is 0 but its head is not");
+            dsr_list[cpu] = this;
+        }
+
+#  else   // ! defined CYGSEM_KERNEL_INTERRUPTS_DSRS_LIST_FIFO
+
+        // At present DSRs are pushed onto the list and will be called
+        // in reverse order. We do not define the order in which DSRs
+        // are called, so this is acceptable.
         next_dsr = dsr_list[cpu];
         dsr_list[cpu] = this;
+
+#  endif  // ! defined CYGSEM_KERNEL_INTERRUPTS_DSRS_LIST_FIFO
+
     }
-    
-#endif
+
+#endif  // defined CYGIMP_KERNEL_INTERRUPTS_DSRS_LIST
     
     HAL_RESTORE_INTERRUPTS(old_intr);    
 };
 
 // -------------------------------------------------------------------------
Index: kernel/current/tests/intr0.cxx
===================================================================
RCS file: /cvs/ecos/ecos/packages/kernel/current/tests/intr0.cxx,v
retrieving revision 1.12
diff -u -5 -r1.12 intr0.cxx
--- kernel/current/tests/intr0.cxx	23 May 2002 23:06:59 -0000	1.12
+++ kernel/current/tests/intr0.cxx	11 Aug 2006 09:20:31 -0000
@@ -44,12 +44,13 @@
 // Contributors:  dsm, jlarmour
 // Date:          1999-02-16
 // Description:   Very basic test of interrupt objects
 // Options:
 //     CYGIMP_KERNEL_INTERRUPTS_DSRS_TABLE
-//     CYGIMP_KERNEL_INTERRUPTS_DSRS_TABLE_SIZE
+//     CYGNUM_KERNEL_INTERRUPTS_DSRS_TABLE_SIZE
 //     CYGIMP_KERNEL_INTERRUPTS_DSRS_LIST
+//     CYGSEM_KERNEL_INTERRUPTS_DSRS_LIST_FIFO
 //####DESCRIPTIONEND####
 
 #include <pkgconf/kernel.h>
 
 #include <cyg/kernel/intr.hxx>
Index: kernel/current/tests/kintr0.c
===================================================================
RCS file: /cvs/ecos/ecos/packages/kernel/current/tests/kintr0.c,v
retrieving revision 1.11
diff -u -5 -r1.11 kintr0.c
--- kernel/current/tests/kintr0.c	23 May 2002 23:07:00 -0000	1.11
+++ kernel/current/tests/kintr0.c	11 Aug 2006 09:20:31 -0000
@@ -44,12 +44,13 @@
 // Contributors:  dsm, jlarmour
 // Date:          1999-02-16
 // Description:   Very basic test of interrupt objects
 // Options:
 //     CYGIMP_KERNEL_INTERRUPTS_DSRS_TABLE
-//     CYGIMP_KERNEL_INTERRUPTS_DSRS_TABLE_MAX
+//     CYGNUM_KERNEL_INTERRUPTS_DSRS_TABLE_SIZE
 //     CYGIMP_KERNEL_INTERRUPTS_DSRS_LIST
+//     CYGSEM_KERNEL_INTERRUPTS_DSRS_LIST_FIFO
 //####DESCRIPTIONEND####
 */
 
 #include <cyg/kernel/kapi.h>
 #include <cyg/hal/hal_intr.h>

-- 
Nick Garnett                                 eCos Kernel Architect
http://www.ecoscentric.com            The eCos and RedBoot experts

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