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


"Stefan Sommerfeld" <sommerfeld@mikrom.de> writes:
> Hi,
>
> attached is the patch inclusive the changelog.

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.

Index: packages/kernel/current/ChangeLog
===================================================================
RCS file: /var/local/cvsroot/ecos/packages/kernel/current/ChangeLog,v
retrieving revision 1.5
diff -u -r1.5 ChangeLog
--- packages/kernel/current/ChangeLog	10 Apr 2006 05:32:53 -0000	1.5
+++ packages/kernel/current/ChangeLog	10 Apr 2006 06:02:36 -0000
@@ -1,3 +1,21 @@
+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>
 
 	* include/thread.hxx: removed extra qualifier of
Index: packages/kernel/current/cdl/interrupts.cdl
===================================================================
RCS file: /var/local/cvsroot/ecos/packages/kernel/current/cdl/interrupts.cdl,v
retrieving revision 1.1.1.1
diff -u -r1.1.1.1 interrupts.cdl
--- packages/kernel/current/cdl/interrupts.cdl	7 Dec 2005 12:06:34 -0000	1.1.1.1
+++ packages/kernel/current/cdl/interrupts.cdl	10 Apr 2006 05:34:11 -0000
@@ -74,7 +74,7 @@
     # 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
@@ -85,6 +85,23 @@
             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 1, 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 to
+                be 1."
+        }
+
     }
 
     cdl_component CYGIMP_KERNEL_INTERRUPTS_DSRS_TABLE {
Index: packages/kernel/current/include/intr.hxx
===================================================================
RCS file: /var/local/cvsroot/ecos/packages/kernel/current/include/intr.hxx,v
retrieving revision 1.1.1.1
diff -u -r1.1.1.1 intr.hxx
--- packages/kernel/current/include/intr.hxx	7 Dec 2005 12:06:34 -0000	1.1.1.1
+++ packages/kernel/current/include/intr.hxx	10 Apr 2006 05:34:11 -0000
@@ -195,11 +195,17 @@
     // 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
 
Index: packages/kernel/current/src/intr/intr.cxx
===================================================================
RCS file: /var/local/cvsroot/ecos/packages/kernel/current/src/intr/intr.cxx,v
retrieving revision 1.1.1.1
diff -u -r1.1.1.1 intr.cxx
--- packages/kernel/current/src/intr/intr.cxx	7 Dec 2005 12:06:34 -0000	1.1.1.1
+++ packages/kernel/current/src/intr/intr.cxx	10 Apr 2006 06:04:32 -0000
@@ -137,6 +137,10 @@
 
 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
 
 // -------------------------------------------------------------------------
@@ -170,6 +174,35 @@
 
 #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;
@@ -191,8 +224,10 @@
         
     }
     
-#endif
-    
+#  endif  // ! defined CYGSEM_KERNEL_INTERRUPTS_DSRS_LIST_FIFO
+
+#endif  // defined CYGIMP_KERNEL_INTERRUPTS_DSRS_LIST
+
 };
 
 externC void
@@ -245,17 +280,39 @@
 
     // 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: packages/kernel/current/tests/intr0.cxx
===================================================================
RCS file: /var/local/cvsroot/ecos/packages/kernel/current/tests/intr0.cxx,v
retrieving revision 1.1.1.1
diff -u -r1.1.1.1 intr0.cxx
--- packages/kernel/current/tests/intr0.cxx	7 Dec 2005 12:06:34 -0000	1.1.1.1
+++ packages/kernel/current/tests/intr0.cxx	10 Apr 2006 05:34:11 -0000
@@ -46,8 +46,9 @@
 // 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>
Index: packages/kernel/current/tests/kintr0.c
===================================================================
RCS file: /var/local/cvsroot/ecos/packages/kernel/current/tests/kintr0.c,v
retrieving revision 1.1.1.1
diff -u -r1.1.1.1 kintr0.c
--- packages/kernel/current/tests/kintr0.c	7 Dec 2005 12:06:34 -0000	1.1.1.1
+++ packages/kernel/current/tests/kintr0.c	10 Apr 2006 05:34:11 -0000
@@ -46,10 +46,10 @@
 // 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>

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