You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@nuttx.apache.org by GitBox <gi...@apache.org> on 2023/01/18 10:04:09 UTC

[GitHub] [nuttx] zyfeier opened a new pull request, #8175: sched/semaphore: increase sem count when holder task exit

zyfeier opened a new pull request, #8175:
URL: https://github.com/apache/nuttx/pull/8175

   ## Summary
   
   Increase the sem count when the holder task exit, to avoid lock failure at next time.
   
   ## Impact
   
   task exit
   
   ## Testing
   
   sabre-6quad:smp ostest


-- 
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


[GitHub] [nuttx] pkarashchenko commented on pull request #8175: sched/semaphore: increase sem count when holder task exit

Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on PR #8175:
URL: https://github.com/apache/nuttx/pull/8175#issuecomment-1396543499

   But I do not see that pholder code modifies sem count value. How is this different from the case without priority inheritance enabled?


-- 
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


[GitHub] [nuttx] zyfeier commented on pull request #8175: sched/semaphore: increase sem count when holder task exit

Posted by "zyfeier (via GitHub)" <gi...@apache.org>.
zyfeier commented on PR #8175:
URL: https://github.com/apache/nuttx/pull/8175#issuecomment-1407341608

   @pkarashchenko 
   bench thread call pthread_exit() run on the cpu1, main thread call pthread_cancel(g_bench_threads) run on cpu0:
   ![image](https://user-images.githubusercontent.com/49738499/215255972-4a110381-6731-4fea-a9a1-993ecfce63e9.png)
   bench thread was deleted and will no longer run, the sem count value of group->tg_joinlock is always -1.


-- 
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


[GitHub] [nuttx] zyfeier commented on pull request #8175: sched/semaphore: increase sem count when holder task exit

Posted by "zyfeier (via GitHub)" <gi...@apache.org>.
zyfeier commented on PR #8175:
URL: https://github.com/apache/nuttx/pull/8175#issuecomment-1409969819

   > Meanwhile I will try to run the test you provided on my end
   
   @pkarashchenko  can you repeat the question with the test case?


-- 
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


[GitHub] [nuttx] pkarashchenko commented on pull request #8175: sched/semaphore: increase sem count when holder task exit

Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on PR #8175:
URL: https://github.com/apache/nuttx/pull/8175#issuecomment-1398115501

   So this PR is more like an attempt to fix some partial case while generic case is still left broken


-- 
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


[GitHub] [nuttx] pkarashchenko commented on pull request #8175: sched/semaphore: increase sem count when holder task exit

Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on PR #8175:
URL: https://github.com/apache/nuttx/pull/8175#issuecomment-1398125566

   Meanwhile I will try to run the test you provided on my end


-- 
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


[GitHub] [nuttx] xiaoxiang781216 merged pull request #8175: sched/semaphore: increase sem count when holder task exit

Posted by "xiaoxiang781216 (via GitHub)" <gi...@apache.org>.
xiaoxiang781216 merged PR #8175:
URL: https://github.com/apache/nuttx/pull/8175


-- 
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


[GitHub] [nuttx] zyfeier commented on pull request #8175: sched/semaphore: increase sem count when holder task exit

Posted by GitBox <gi...@apache.org>.
zyfeier commented on PR #8175:
URL: https://github.com/apache/nuttx/pull/8175#issuecomment-1396582225

   > But I do not see that pholder code modifies sem count value. How is this different from the case without priority inheritance enabled? And is the same issue exists in non-SMP case?
   
   1. The situation is even worse if priority inheritance is not enabled, because no holder information was recorded.
   2. non-SMP is the same.
   
   If the task is released while waiting for sem, nxsem_recover() function will release the count:
   
   ```
     if (tcb->task_state == TSTATE_WAIT_SEM)
       {
         FAR sem_t *sem = tcb->waitobj;
         DEBUGASSERT(sem != NULL && sem->semcount < 0);
   
         /* Restore the correct priority of all threads that hold references
          * to this semaphore.
          */
   
         nxsem_canceled(tcb, sem);
   
         /* And increment the count on the semaphore.  This releases the count
          * that was taken by sem_wait().  This count decremented the semaphore
          * count to negative and caused the thread to be blocked in the first
          * place.
          */
   
         sem->semcount++;
       }
   ```
   
   But, when the task hold the mutex,  count was not released by nxsem_release_all().


-- 
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


[GitHub] [nuttx] pkarashchenko commented on pull request #8175: sched/semaphore: increase sem count when holder task exit

Posted by "pkarashchenko (via GitHub)" <gi...@apache.org>.
pkarashchenko commented on PR #8175:
URL: https://github.com/apache/nuttx/pull/8175#issuecomment-1412358586

   @zyfeier @xiaoxiang781216 sorry for running late with this. I was able to start proposed code, but in non-SMP case and priority inheritance disabled. What I see is that something is definitely going wrong because in my case test always stops at the same value:
   ```
   No.4973 -> Test Success!
   Pthread Create Success!
   ```
   I need some time to figure out why `4973` is a magic value here. Also when an issue happens the NSH is not responding anymore, so I suspect that there is memory leak somewhere because debugger shows device is in idle loop.


-- 
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


[GitHub] [nuttx] xiaoxiang781216 commented on pull request #8175: sched/semaphore: increase sem count when holder task exit

Posted by "xiaoxiang781216 (via GitHub)" <gi...@apache.org>.
xiaoxiang781216 commented on PR #8175:
URL: https://github.com/apache/nuttx/pull/8175#issuecomment-1425741666

   @pkarashchenko do you have any progress?


-- 
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


[GitHub] [nuttx] zyfeier commented on pull request #8175: sched/semaphore: increase sem count when holder task exit

Posted by GitBox <gi...@apache.org>.
zyfeier commented on PR #8175:
URL: https://github.com/apache/nuttx/pull/8175#issuecomment-1396333791

   > @zyfeier
   > 
   > > Increase the sem count when the holder task exit, to avoid lock failure at next time.
   > 
   > Could you explain the lock failure more concretely? Is it possible to detect the failure with ostest?
   
   Here is the test case for this issue, need enable CONFIG_PRIORITY_INHERITANCE and CONFIG_SMP.
   This case will finally stop at` nxmutex_lock(&group->tg_joinlock)`,  and we can see no one is holding
   this lock, but the semcount value is 0.
   
   ```
   static pthread_t g_bench_threads;
   int priority = 98;
   
   static void bench_spawn_helper(void *args)
   {
     UNUSED(args);
     pthread_exit(NULL);
   }
   
   static void thread_test(void)
   {
     struct sched_param param;
     pthread_attr_t attr;
     int policy;
   
     pthread_attr_init(&attr);
     pthread_getschedparam(0, &policy, &param);
     param.sched_priority = priority;
     pthread_attr_setschedparam(&attr, &param);
   
     pthread_create(&g_bench_threads, &attr,
                   (pthread_startroutine_t)bench_spawn_helper, NULL);
     printf("Pthread Create Success!\n");
     pthread_attr_destroy(&attr);
   }
   
   int main(int argc, FAR char *argv[])
   {
     int i = 0;
     for (;;)
       {
         thread_test();
         pthread_cancel(g_bench_threads);
         printf("No.%d -> Test Success!\n", i);
         i++;
       }
   
     return 0;
   }
   ```


-- 
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


[GitHub] [nuttx] zyfeier commented on pull request #8175: sched/semaphore: increase sem count when holder task exit

Posted by "zyfeier (via GitHub)" <gi...@apache.org>.
zyfeier commented on PR #8175:
URL: https://github.com/apache/nuttx/pull/8175#issuecomment-1413031325

   > @zyfeier @xiaoxiang781216 sorry for running late with this. I was able to start proposed code, but in non-SMP case and priority inheritance disabled. What I see is that something is definitely going wrong because in my case test always stops at the same value:
   > 
   > ```
   > No.4973 -> Test Success!
   > Pthread Create Success!
   > ```
   > 
   > I need some time to figure out why `4973` is a magic value here. Also when an issue happens the NSH is not responding anymore, so I suspect that there is memory leak somewhere because debugger shows device is in idle loop.
   
   @pkarashchenko maybe you've already repeated the problem, you can check whether the main task is waiting for the semaphore, and the semaphore semcount is 0.


-- 
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


[GitHub] [nuttx] masayuki2009 commented on pull request #8175: sched/semaphore: increase sem count when holder task exit

Posted by GitBox <gi...@apache.org>.
masayuki2009 commented on PR #8175:
URL: https://github.com/apache/nuttx/pull/8175#issuecomment-1386920351

   @zyfeier 
   
   >Increase the sem count when the holder task exit, to avoid lock failure at next time.
   
   Could you explain the lock failure more concretely?
   Is it possible to detect the failure with ostest?


-- 
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


[GitHub] [nuttx] pkarashchenko commented on pull request #8175: sched/semaphore: increase sem count when holder task exit

Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on PR #8175:
URL: https://github.com/apache/nuttx/pull/8175#issuecomment-1398122888

   I also can't track a path that leads to pthread exiting while holding `group->tg_joinlock`. Is it possible to provide some backtrace information so we can better understand the flow?


-- 
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


[GitHub] [nuttx] xiaoxiang781216 commented on pull request #8175: sched/semaphore: increase sem count when holder task exit

Posted by "xiaoxiang781216 (via GitHub)" <gi...@apache.org>.
xiaoxiang781216 commented on PR #8175:
URL: https://github.com/apache/nuttx/pull/8175#issuecomment-1411403349

   Any result @pkarashchenko ?


-- 
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