[SMP]: Big Kernel Lock vs ISR_Disable

[Marta Rybczynska]

Hello all,
We're merging a very large SMP patch with RTEMS mainline. The main step
forward in the patch is the arrival of _Big_Kernel_lock. It is a recursive
lock that protects accesses to nearly all important data structures. The
lock is used next to _ISR_Disable and (partially) _Dispatch_Disable.

To finish the merge we need to change our API due to some older design
decisions that are in conflict with those made by Joel and Jennifer (for
the curious: we cannot implement our _Lock_Critical_Section and
_Unlock_Critical_Section using RTEMS SMP locks). However, the change can be
made in different ways and we prefer to discuss it before, and not after
submitting a large PR.

Normally the code looks like this:
--- git/cpukit/libfs/src/devfs/devfs_mknod.c    2011-08-22
21:10:06.230348773 +0200
+++ rtems/cpukit/libfs/src/devfs/devfs_mknod.c  2011-08-22
21:13:41.265537357 +0200
@@ -69,11 +69,13 @@ int devFS_mknod(
       rtems_set_errno_and_return_minus_one( ENOMEM );
 
   _ISR_Disable(level);
+  _Lock_Critical_Section();
   device_name_table[slot].device_name  = (char *)path;
   device_name_table[slot].device_name_length = strlen(path);
   device_name_table[slot].major = major;
   device_name_table[slot].minor = minor;
   device_name_table[slot].mode  = mode;
+  _Unlock_Critical_Section();
   _ISR_Enable(level);
 
   return 0;

In this case, it seems natural to merge _ISR_Disable and
_Lock_Critical_Section into one function.

However, we do have more complicated cases:
--- git/cpukit/posix/src/mutexlocksupp.c        2011-09-01
14:13:52.455646968 +0200
+++ rtems/cpukit/posix/src/mutexlocksupp.c      2011-09-10
18:50:03.379722079 +0200
@@ -43,6 +43,8 @@ int _POSIX_Mutex_Lock_support(
   Objects_Locations             location;
   ISR_Level                     level;
 
+  _Lock_Critical_Section();
+
   the_mutex = _POSIX_Mutex_Get_interrupt_disable( mutex, &location,
&level );
   switch ( location ) {
 
@@ -62,7 +64,8 @@ int _POSIX_Mutex_Lock_support(
     case OBJECTS_REMOTE:
 #endif
     case OBJECTS_ERROR:
-      break;
+       _Unlock_Critical_Section();
+       break;
   }
 
   return EINVAL;

Here we have a side-effect of the _POSIX_Mutex_Get_interrupt_disable
function.


The options we have are:
1. Add lock to _ISR_Disable/Enable
This will break certain points, especially around _Objects_Get (the
function exits in the critical section of Disable_Dispatch and it is up to
the further code to release it, sometimes the code can sleep in the
process).

2. Add a level parameter to our
_Lock_Critical_Section/_Unlock_Critical_Section functions and call SMP lock
inside
However, this will lead to unnecessary recursive interrupt disable

3. Add an SMP lock variant that does not disable the interrupts

What do you think and which option do you prefer?

Thanks,
Marta