managarm / libasync

Async primitives library for C++20
https://docs.managarm.org/libasync/
MIT License
16 stars 5 forks source link

Use on platforms that have no mutexes #21

Open niansa opened 1 year ago

niansa commented 1 year ago

Hey!

I was attempting to use this library on a plaform that doesn't have preemptive multithreading (it's cooperative) and therefore no mutexes. Libasync however really seems like it wants mutexes. I could probably write a fake mutex class, but that seems like a dirty option. Is there a way to just tell libasync to not use mutexes?

Thanks niansa

avdgrinten commented 1 year ago

Mutexes can also make sense in a cooperate scheduling world. For example, Boost's Fiber library also has mutexes (which will simply invoke the cooperative scheduler if there is contention). Does that make sense in your context or not?

niansa commented 1 year ago

Not really, since I am not running (and not planning to run) more than a single thread anyways. The platform I am working on doesn't really have well documented multithreading anyways. Came across a couple of undocumented error codes last time I tried. 😆

But yes, you are right, mutexes make sense if you are doing multithreading even on cooperative systems. Took me a while to realize.

ArsenArsen commented 1 year ago

Even on a single thread, they can make sense. The following code without mutexes could have the same issue concurrency could:

auto x = this->m_something;
co_await check_something (x);
this->m_something = std::move (x);

... as check_something suspending could lead to another coroutine that uses m_something to override the contents.

qookei commented 1 year ago

... as check_something suspending could lead to another coroutine that uses m_something to override the contents.

In this case you'd use an async::mutex as not to deadlock the program (return to dispatcher instead of blocking the thread in case of contention), while I'm assuming this issue is talking about async::platform::mutex (which is a std::mutex by default).

You could define a custom platform header (<async/platform.hpp> somewhere in the include path), and define LIBASYNC_CUSTOM_PLATFORM to tell libasync to include it instead of the defaults, and define a no-op mutex type there (or use frg::ticket_spinlock, which only needs atomics to work).

ArsenArsen commented 1 year ago

Ah, right. Terminology confusion, my bad. I thought the issue is about the async::mutex.

niansa commented 1 year ago

So I eliminated all std::mutex occurencies by hand, just for giving it a try, however it seems like the platform I am targeting doesn't support atomics either. And these seem to be absolutely mandatory. :-/

ArsenArsen commented 1 year ago

Which platform is this?

niansa commented 1 year ago

It's armv5te (through devKitPro). :-)

ArsenArsen commented 1 year ago

what compiler does that use? I'm unaware of any platform that omits atomics (and the standard, AFAIK, doesn't permit that anyway)

niansa commented 1 year ago

It's latest GCC. I think they share the STL headers between all targets and just vary the underlaying library.

ArsenArsen commented 1 year ago

AFAIK, the GCC arm5te target provides atomics. I'll need more info, or compiler access, to be able to debug further why they might not be present. Alternatively, we could have a platform::atomic that we can switch between some dummy class and std::atomic via the custom platform mechanism, but this very much feels like a toolchain problem

niansa commented 1 year ago

I am going to ask in the devKitPro forum and come back here if they were able to tell me anything useful :-)

ArsenArsen commented 1 year ago

Sure, keep us posted. Also, out of curiosity, could you post the error that you get due to a lack of atomics?

niansa commented 1 year ago
/opt/devkitpro/devkitARM/bin/../lib/gcc/arm-none-eabi/12.2.0/../../../../arm-none-eabi/bin/ld: CMakeFiles/Bot4All.dir/main.o: in function `std::atomic<async::coroutine_cfp>::exchange(async::coroutine_cfp, std::memory_order)':
/opt/devkitpro/devkitARM/arm-none-eabi/include/c++/12.2.0/atomic:303: undefined reference to `__atomic_exchange_4'
/opt/devkitpro/devkitARM/bin/../lib/gcc/arm-none-eabi/12.2.0/../../../../arm-none-eabi/bin/ld: CMakeFiles/Bot4All.dir/main.o: in function `std::__atomic_base<unsigned int>::fetch_sub(unsigned int, std::memory_order)':
/opt/devkitpro/devkitARM/arm-none-eabi/include/c++/12.2.0/bits/atomic_base.h:628: undefined reference to `__atomic_fetch_sub_4'
/opt/devkitpro/devkitARM/bin/../lib/gcc/arm-none-eabi/12.2.0/../../../../arm-none-eabi/bin/ld: CMakeFiles/Bot4All.dir/main.o: in function `std::atomic<async::coroutine_cfp>::exchange(async::coroutine_cfp, std::memory_order)':
/opt/devkitpro/devkitARM/arm-none-eabi/include/c++/12.2.0/atomic:303: undefined reference to `__atomic_exchange_4'
/opt/devkitpro/devkitARM/bin/../lib/gcc/arm-none-eabi/12.2.0/../../../../arm-none-eabi/bin/ld: /opt/devkitpro/devkitARM/arm-none-eabi/include/c++/12.2.0/atomic:303: undefined reference to `__atomic_exchange_4'
/opt/devkitpro/devkitARM/bin/../lib/gcc/arm-none-eabi/12.2.0/../../../../arm-none-eabi/bin/ld: CMakeFiles/Bot4All.dir/Rest.o: in function `std::atomic<async::coroutine_cfp>::exchange(async::coroutine_cfp, std::memory_order)':
/opt/devkitpro/devkitARM/arm-none-eabi/include/c++/12.2.0/atomic:303: undefined reference to `__atomic_exchange_4'
/opt/devkitpro/devkitARM/bin/../lib/gcc/arm-none-eabi/12.2.0/../../../../arm-none-eabi/bin/ld: /opt/devkitpro/devkitARM/arm-none-eabi/include/c++/12.2.0/atomic:303: undefined reference to `__atomic_exchange_4'
/opt/devkitpro/devkitARM/bin/../lib/gcc/arm-none-eabi/12.2.0/../../../../arm-none-eabi/bin/ld: CMakeFiles/Bot4All.dir/HTTP.o: in function `std::atomic<async::coroutine_cfp>::exchange(async::coroutine_cfp, std::memory_order)':
/opt/devkitpro/devkitARM/arm-none-eabi/include/c++/12.2.0/atomic:303: undefined reference to `__atomic_exchange_4'
/opt/devkitpro/devkitARM/bin/../lib/gcc/arm-none-eabi/12.2.0/../../../../arm-none-eabi/bin/ld: CMakeFiles/Bot4All.dir/HTTP.o:/opt/devkitpro/devkitARM/arm-none-eabi/include/c++/12.2.0/atomic:303: more undefined references to `__atomic_exchange_4' follow
/opt/devkitpro/devkitARM/bin/../lib/gcc/arm-none-eabi/12.2.0/../../../../arm-none-eabi/bin/ld: CMakeFiles/Bot4All.dir/Receiver.o: in function `std::__atomic_base<unsigned int>::fetch_sub(unsigned int, std::memory_order)':
/opt/devkitpro/devkitARM/arm-none-eabi/include/c++/12.2.0/bits/atomic_base.h:628: undefined reference to `__atomic_fetch_sub_4'
/opt/devkitpro/devkitARM/bin/../lib/gcc/arm-none-eabi/12.2.0/../../../../arm-none-eabi/bin/ld: CMakeFiles/Bot4All.dir/Receiver.o: in function `std::atomic<async::coroutine_cfp>::exchange(async::coroutine_cfp, std::memory_order)':
/opt/devkitpro/devkitARM/arm-none-eabi/include/c++/12.2.0/atomic:303: undefined reference to `__atomic_exchange_4'
/opt/devkitpro/devkitARM/bin/../lib/gcc/arm-none-eabi/12.2.0/../../../../arm-none-eabi/bin/ld: CMakeFiles/Bot4All.dir/Receiver.o: in function `std::__atomic_base<unsigned int>::fetch_add(unsigned int, std::memory_order)':
/opt/devkitpro/devkitARM/arm-none-eabi/include/c++/12.2.0/bits/atomic_base.h:618: undefined reference to `__atomic_fetch_add_4'
/opt/devkitpro/devkitARM/bin/../lib/gcc/arm-none-eabi/12.2.0/../../../../arm-none-eabi/bin/ld: CMakeFiles/Bot4All.dir/Sender.o: in function `std::__atomic_base<unsigned int>::fetch_add(unsigned int, std::memory_order)':
/opt/devkitpro/devkitARM/arm-none-eabi/include/c++/12.2.0/bits/atomic_base.h:618: undefined reference to `__atomic_fetch_add_4'
/opt/devkitpro/devkitARM/bin/../lib/gcc/arm-none-eabi/12.2.0/../../../../arm-none-eabi/bin/ld: CMakeFiles/Bot4All.dir/AsyncManager.o: in function `std::__atomic_base<unsigned int>::fetch_add(unsigned int, std::memory_order)':
/opt/devkitpro/devkitARM/arm-none-eabi/include/c++/12.2.0/bits/atomic_base.h:618: undefined reference to `__atomic_fetch_add_4'
/opt/devkitpro/devkitARM/bin/../lib/gcc/arm-none-eabi/12.2.0/../../../../arm-none-eabi/bin/ld: CMakeFiles/Bot4All.dir/SPWS.o: in function `std::atomic<async::coroutine_cfp>::exchange(async::coroutine_cfp, std::memory_order)':
/opt/devkitpro/devkitARM/arm-none-eabi/include/c++/12.2.0/atomic:303: undefined reference to `__atomic_exchange_4'
/opt/devkitpro/devkitARM/bin/../lib/gcc/arm-none-eabi/12.2.0/../../../../arm-none-eabi/bin/ld: /opt/devkitpro/devkitARM/arm-none-eabi/include/c++/12.2.0/atomic:303: undefined reference to `__atomic_exchange_4'
ArsenArsen commented 1 year ago

Ah, I see. In this case, depending on your system, you're likely best off implementing a libatomic-esque library suited for your environment. If this is a non-SMP environment, that could be as simple as temporarily turning off interrupts or something like that.

With LTO, that could probably optimize to a regular load/store (plus whatever interrupt handling you add).

This might be a worthy inclusion in devkit too.