jlaitine commented on code in PR #16194: URL: https://github.com/apache/nuttx/pull/16194#discussion_r2059065297
########## sched/semaphore/sem_trywait.c: ########## @@ -74,29 +77,57 @@ int nxsem_trywait_slow(FAR sem_t *sem) /* If the semaphore is available, give it to the requesting task */ - semcount = atomic_read(NXSEM_COUNT(sem)); + if (mutex) + { + new = nxsched_gettid(); + } + + lock = atomic_read(NXSEM_COUNT(sem)); do { - if (semcount <= 0) + if (mutex) { - leave_critical_section(flags); - return -EAGAIN; + if (NXSEM_MACQUIRED(lock)) + { + goto out; + } + } + else + { + if (lock <= 0) + { + goto out; + } + + new = lock - 1; } } - while (!atomic_try_cmpxchg_acquire(NXSEM_COUNT(sem), - &semcount, semcount - 1)); + while (!atomic_try_cmpxchg_acquire(plock, &lock, new)); /* It is, let the task take the semaphore */ ret = nxsem_protect_wait(sem); if (ret < 0) { - atomic_fetch_add(NXSEM_COUNT(sem), 1); + if (mutex) + { + atomic_set(NXSEM_MHOLDER(sem), NXSEM_NO_MHOLDER); Review Comment: No. What I just wrote was bollocks. It is not possible, the mutex was just acquired for this task, and we are inside the critical section. There is no race betwee libc fast path and this. Also this is the only holder of the mutex, so if the nxsem_wait_protect fails, the correct value is NO_HOLDER. No-one can steal this in between. After it is set to NO_HOLDER, it is of course free to someone else to acquire. -- 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-unsubscr...@nuttx.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org