xiaoxiang781216 commented on code in PR #10605: URL: https://github.com/apache/nuttx/pull/10605#discussion_r1346795877
########## include/nuttx/spinlock.h: ########## @@ -49,6 +49,21 @@ typedef uint8_t spinlock_t; * SP_LOCKED and SP_UNLOCKED must be constants of type spinlock_t. */ +#if defined(CONFIG_TICKET_SPINLOCK) +#include <stdatomic.h> + +struct ticket_spinlock_s Review Comment: > Do you want to only use spinlock_t and remove ticket_spinlock? > Yes, it's an implementation detail where to use the ticket algorithm, and then shouldn't expose the difference to the caller. Please consider how Linux switch the spinlock algo, do they add something like ticket_spinlock_s? > In 64bit machine spinlock is 64bit, I had tested modify spinlock to 32bit and it can't boot because the underlayer of up_testset is 64bit. So I just use first 32bit as ticket spinlock in 64bit machine. > 64bit support 32bit atomic too, you can simplify modify the implementation of up_testset on 64bit. > I think this modification will cause same problem. > > And spinlock_t is already defined at arch/spinlock.h That's why I suggest you don't include arch/spinlock.h in case of CONFIG_TICKET_SPINLOCK. -- 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
