[GitHub] [incubator-nuttx] pkarashchenko commented on a change in pull request #5539: mm: handle take mm sem in IRQ

[GitBox <gi...@apache.org>]

pkarashchenko commented on a change in pull request #5539:
URL: https://github.com/apache/incubator-nuttx/pull/5539#discussion_r809727872



##########
File path: mm/mm_heap/mm_sem.c
##########
@@ -80,9 +80,21 @@ bool mm_takesemaphore(FAR struct mm_heap_s *heap)
 
   if (up_interrupt_context())
     {
-      /* Can't take semaphore in the interrupt handler */
+#if !defined(CONFIG_SMP) && defined(CONFIG_DEBUG_MM)
+      int val;
+
+      /* Check the semaphore value, if held by someone, then return false.
+       * Else, we can take it, return true.
+       */
+
+      _SEM_GETVALUE(&heap->mm_semaphore, &val);

Review comment:
       I just noticed that in https://github.com/apache/incubator-nuttx/pull/5504 we removed `return _SEM_TRYWAIT(&heap->mm_semaphore) >= 0;` line and replaced it with `_SEM_GETVALUE(&heap->mm_semaphore, &val);` + `return val > 0;`so the heap semaphore is not actually taken anymore in case of `else if (sched_idletask())` and `if (up_interrupt_context())`. Is this expected? Looks like it changes behavior of the system tremendously 




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org