jlaitine commented on PR #16194: URL: https://github.com/apache/nuttx/pull/16194#issuecomment-2853322370
> @jlaitine after this patchset get merged, we can simplify the holder logic for priority inheritance since: > > 1. Don't track the multiple holders since mutex always can have one and only one holder > 2. But need track multiple thread which is blocking/waiting on the mutex with the local variable > > Do you think so? Yes, cleanup is definitely welcome, but I didn't quite understand yet the idea of the local variable, perhaps you can elaborate a bit more on that? My thinking is as follows: 1. When a thread comes a holder, it needs to set it to the semaphore; in case of mutex it is just the PID to the val.mholder (the single holder) or adding the holder structure to the semaphore's list in case of counting semaphores. 2. When a thread blocks on the mutex or counting semaphore, i.e. when the priority boost happens, it needs to add the semaphore to the tcb list of semaphores/mutexes (the "tcb->holdsem" list). A thread can be a holder of multiple counting semaphores and mutexes, and any of those may have caused priority boost; so this list is always needed for proper priority restoration. These two a bit different things (list of sem holders and tracking priority boosts) are now mixed togheter in nxsem_add_holder_tcb/nxsem_allocholder, which just allocates the holder and puts it to both lists, even though for mutexes adding the structure to the semaphores own holder list is redundant, and they don't need to be added at the same time. Because 2. is always needed, I found in easier to just use all the existing code and semholder structure for adding the holder and using the priority inheritance code as is, even though it adds the same structure also to the semaphore's list. It doesn't really matter if it is added also to the semaphore's list; you don't save anything by removing it (the holder list pointer is there allocated in sem_s anyway as long as the structure is the same for a mutex and a counting semaphore). But, for the further cleanup / optimization possibilities, these two could be separated. For example making own functions for "nxsem_add_holder_tcb" to add it to tcb->holdsem and "nxsem_add_counting_sem_holder" to add it to semaphores own list. Does this make sense? Another thing for optimizations, there is one very small and beneficial optimization that we could add; for counting semaphores the "fast path" in libc can be also enabled when the priority inheritance is disabled on the semaphore (basically in that case the libc fast functions can just increase/decrease the counter as long as it doesn't block or wake up another threads). This would be beneficial for performance especially in queue implementations (e.g. multiple-writer - single reader, where writers post a signalling semaphore and reader sleeps on the semaphore waiting for the writers). I have code for this ready, if you are interested after this PR. -- 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
