Re: [PR] Fix Deadloop in VFS if CONFIG_CANCELLATION_POINTS is enabled [nuttx]

["pussuw (via GitHub)" <gi...@apache.org>]

pussuw commented on PR #9716:
URL: https://github.com/apache/nuttx/pull/9716#issuecomment-1798075605

   Hi @patacongo and @pkarashchenko 
   
   I'm currently working on a fix for semaphores in CONFIG_BUILD_KERNEL (all of the sem_t data is in user memory) and I encountered this change.
   
   Isn't the problem here simply the loop condition ?
   ```
        ret = _SEM_ERRVAL(ret);
        if (ret != -EINTR && ret != -ECANCELED)
          {
            break;
          }
   ```
   EINTR can be considered as "got woken by signal, try again", but ECANCELED should break from the loop at once.
   
   I also noticed a very old issue which duplicates this exact issue: https://github.com/apache/nuttx/issues/619 and the fix was to break the uninterruptible loop if ECANCELED is encountered.
   
   So for some reason, the same error was duplicated into mutex.h / lib_mutex.c later by: https://github.com/apache/nuttx/commit/b88a8cf39ff1019ad787c4316b22ce29c7daa2dc
   
   Isn't the fix just to remove ECANCELED from the ret != test and break the loop if the calling thread is canceled ? Assuming ECANCELED will be handled later (if we are indeed inside a nested cancellation point). This would remove the need to add 1 extra system call to a direct kernel private procedure, which would make the user API more coherent.
   
   Why I bring this up is that exposing nx_semwait() to the userspace kind of makes it impossible to know whether a semaphore request came from userspace or from kernelspace.
   
   


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