kernel + posix: add k_spin_trylock(), pthread_spin_lock(), etc

[cfriedt]

This change adds a new spinlock API, k_spin_trylock(), in order to implement pthread_spin_lock() and friends. Part of the POSIX Roadmap for LTSv3.

Fixes #59910

[evgeniy-paltsev]

Why runtime assertion? Let's use BUILD_ASSERT.

[cfriedt]

Good idea

[cfriedt]

@evgeniy-paltsev - Fixed!

[peter-mitsis]

Looks good to me.

outdated

[ycsin]

re-stamp

[cfriedt]

@andyross, @peter-mitsis, @ycsin - please re-stamp.

I added a change to scripts/checkpatch.pl to avoid the USE_NEGATIVE_ERRNO false positive when dealing with .*/lib/posix/.*\.c.

That will let us avoid the need for merge superpowers constantly for lib/posix/, which was annoying.

[xyzzy42]

There is some opportunity to increase compiler optimization by providing the public pthread methods as inline wrappers that take a pthread_spinlock_t* argument and passes it to an internal function, but first convert the argument to a pthread_spinlock_t or bit.

The check that lock is not NULL can likely be resolved by the compiler at compile time. It's possible to avoid even allocating space for lock and have it live in a register (would need a somewhat limited scope for the lock to be defined over to actually be done). *lock needs to be loaded from RAM on each call, since the compiler can not determine nothing changes it. But if an inline function (or LTO!) is used, the compiler can see that nothing changes the value and avoid the instructions to reload it.

[cfriedt]

There is some opportunity to increase compiler optimization by providing the public pthread methods as inline wrappers that take a pthread_spinlock_t* argument and passes it to an internal function, but first convert the argument to a pthread_spinlock_t or bit.

There is always room for optimization but that can be done on an as-needed basis at a later date with evidence / measurements. Additionally, having something that works and meets the spec is more important, and I would avoid inlining anything that isn't specifically mentioned that it could possibly be an inline function or macro.

The check that lock is not NULL can likely be resolved by the compiler at compile time. It's possible to avoid even

Technically, that check isn't even required by the standard and it represents undefined behaviour. I was thinking about making it an __ASSERT_NO_MSG() since that can easily be disabled.

allocating space for lock and have it live in a register (would need a somewhat limited scope for the lock to be defined over to actually be done). *lock needs to be loaded from RAM on each call, since the compiler can not determine nothing changes it. But if an inline function (or LTO!) is used, the compiler can see that nothing changes the value and avoid the instructions to reload it.

Yep - keeping lock in a register would have benefits, but it also isn't clear to me how to return multiple values in registers. Additionally, there is no "invalid lock" value as part of the k_spinlock API. We can leave that to future optimization (ideally before the next release).

[andyross]

Refresh +1

[xyzzy42]

Technically, that check isn't even required by the standard and it represents undefined behaviour. I was thinking about making it an __ASSERT_NO_MSG() since that can easily be disabled.

The same also for both checks for use of an uninitialized spinlock, undefined behavior in POSIX, but certainly a nice debugging aid. The second check makes all spinlocks contend for access to the single spinlock protecting posix_spinlock_bitarray. I suppose scaling probably isn't much of a concern. The flash sharing caused by having all spinlocks be adjacent in memory will not help scaling either.