[En-Nut-Discussion] Feature Request 1572837: mutex API nesting level info

[Bernard Fouché]

Nathan Moore wrote:
>> An excellent explanation about why to avoid recursive mutexes by someone
>> that knows what he writes about:
>>
>> http://www.zaval.org/resources/library/butenhof1.html
>>     
>
> Exactly the opposite because NutOs doesn't have preemptable threading.
>   
I don't see why you restrict Buthenhof's remarks to preemptable 
threading. As soon as you need a mutex, then, IMHO, his remarks have to 
be considered. Otherwise, well, why would you need mutexes at all if 
nothing can change the resource you are manipulating ? :)
> That was all about using mutexes to protect memory-type resources, not IO
> that can block.
>   
This is completely similar: I/O resources are seen as memory for the MCU 
(I/O registers for instance, or a bit in a I/O register or in a 
particular MCU register). Once again, Buthenof remarks are about design, 
not low level details.
> In NutOs you protect memory resources with critical sections (turning off
> interrupts) which
> are the widest form of mutex (and sadly are recursive, which I will argue
> against until I'm blue
>   
Usually a CPU/MCU offers to turn on/off interrupts, without any nesting 
level. Nested levels may exist in interrupt processing, but a particular 
interrupt can only be turned off or on. You can't for instance, have the 
following code:

1) turn interrupt X off
2) turn interrupt X off
3) turn interrupt X on
4) turn interrupt X on

As long as you reach step 3, interrup X is definitely turned on, so I 
don't see how you can consider turning off/on interrupts being similar 
to recursives mutexes.
> in the face).   The only thing that can step on your memory resource usage
> is an ISR unless
> you throw NutThreadYields (directly or indirectly) into the middle of them.
>   
In a preemptable OS, an ISR is used by the scheduler to eventually yield 
the current thread and select a new one. An ISR in NutOs may wake up a 
thread that was waiting for an I/O to complete (or any other event). So 
basically the fact the OS is preemtable or not has no relation with 
design guidelines linked to recursives mutexes, as you show yourself:

> Recursive mutexes are very very good for IO routines, which do have
> NutThreadYields in the
> middle of them.
> Say I have the code:
>
> void print_record(FILE * f, struct record * r) {
>   NutMutexLock(&(f->o_lock));
>   fprintf(f, "%s : %i\t%f\n", r->name, r->count, r->percent);
>   NutMutexUnlock(&(f->o_lock));
> }
>
> void print_status(FILE *f) {
>   struct record * r;
>   NutMutexLock(&(f->o_lock));
>   fprintf(f, "Current status:\n");
>   for (r = record_list; r; r = r->next) {
>     print_record(f, r);
>   }
>   NutMutexUnlock(&(f->o_lock));
> }
>
> void print_record_named(char *n) {
>    struct record *r;
>    for (r = record_list, r, r = r->next) {
>            if (0 == strcmp(n, r->name) ) {
>               print_record(f, r);
>            }
>     }
> }
>
> Without the recursive mutex (which isn't actually in nut's FILE struct)
> running code that uses these
> (or any fprintf(f, ...) for that matter) could not guarentee that all of
> their output was contiguous.
>   
That's exactly memory sharing as described by Buthenof. You would 
'print' into a memory buffer or into a UART driven by I/O registed, the 
problem is exactly the same.
> A higher priority thread could pop in there and write it's junk in the
> middle of this output.
> With the recursive mutex you can reuse component print functions and also
> lock at the higher level
> when you need to print other stuff together with those.
>   

This is an excellent example of a design problem: you are using the same 
mutex for different needs.

1) You have a first level of locking which is 'lower' than the second. 
This first level is in print_record() and needs a mutex to protect the 
file descriptor being used to access the buffer that this file 
descriptor uses.

2) You have a second level of usage in print_status(), which is 'higher' 
than the first one, since you want to make sure that a bunch of callsto 
print_record() are atomic. But you are also making direct usage of a 
file descriptor in print_status (to print 'Current status:') which is 
still another need.

All this mixed usages of a single mutex makes you think that recursive 
mutexes are the solution.

Here is another way:

- have a 'lock_puts()' function:

void lock_puts(FILE * f, char *s) {
  NutMutexLock(&(f->o_lock));
  fputs(s,f);
  NutMutexUnlock(&(f->o_lock));
}

- everything that needs to print prepares a buffer in memory and sends 
it to lock_puts().

No more recursive mutex needs, shortest locking time, the mutex is 
clearly used only to protect the file descriptor and nothing else, 
someone coming later and reading lock_puts() description understands 
easily how to use it, and if ever you change of OS later, the same logic 
will apply to your code base.

I would not have thought of a better example :)

Now you can object that what you want to print is so big that it can not 
be first stored into memory before being sent out. But in that case it 
would mean again that you have to change your application's design to 
take that constraint into account. The solution would then to have only 
'high level' function lock the mutex, and no more low level locking into 
the function that directly print into the file descriptor: still no 
recursive mutex needs.