[Bug 1876] Add SMP ISR support

bugzilla-daemon at rtems.org bugzilla-daemon at rtems.org
Tue Aug 2 19:50:03 UTC 2011


https://www.rtems.org/bugzilla/show_bug.cgi?id=1876

Gedare <giddyup44 at yahoo.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |giddyup44 at yahoo.com

--- Comment #1 from Gedare <giddyup44 at yahoo.com> 2011-08-02 14:50:03 CDT ---
This is tricky stuff! I tried to make sense of it and have some comments.

src/isrsmp.c:
Should the name be smpisr.c to be consistent with other smp files? The question
boils down to whether these new functions are part of the ISR package or the
SMP package? If the latter, the functions and file should be renamed to reverse
the ordering. If the former, leave as-is.

 _ISR_Xxx_on_core: Xxx=Disable/Enable/Flash
name implies you will pass a core to disable? I think add the word this:
_ISR_Xxx_on_this_core. 

_ISR_SMP_Xxx: Xxx=Disable/Enable/Flash
how about _ISR_Xxx_all_cores ?

honoured: British spelling. American spelling is honored.

The implementation of _ISR_SMP_Xxx (*Disable/Enable/Flash) is confusing,
because it does not enforce across all cores unless the USE_SPINLOCKS is
defined---should this define be configurable, or are we leaving it as a hack
for now?  Since it is commented out, I don't see a reason to add these new
functions: they do exactly the same thing as the original ones that have now
been renamed to _ISR_Xxx_on_core.

In places you have switched to calling _ISR_Xxx_on_core explicitly. Please
double-check these to make sure you intend that only the current core is
involved (Granted, that is the only supported implementation at present, so I'm
not sure why the change to make these calls explicit.) and that every change to
_ISR_Disable_on_core is matched with an explicit _ISR_Enable_on_core. If there
is mismatch, bugs may appear if the USE_SPINLOCKS implementation is ever
enabled.

+int _ISR_SMP_Enter(void)
_ISR_SMP_Exit
These functions are not exported by a header file and they lack documentation
or usage.

Should the _ISR_Set_level function be also made per-core or SMP (for-all)?

+ * Prototypes and structures used int the debug lock/unlock error log.
s/int the/in the/

cpukit/score/src/threaddispatchdisablelevel.c
@@ -96,6 +94,8 @@
     isr_level
   ); 

+  _ISR_Enable_on_core(isr_level);
+
Shouldn't need this ISR_Enable_on_core, it is already done in the call to
_SMP_lock_spinlock_nested_Release?

-- 
Configure bugmail: https://www.rtems.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are watching all bug changes.



More information about the bugs mailing list