search

libsdl-org / SDL

Simple Directmedia Layer
https://libsdl.org
zlib License
9.55k stars 1.77k forks source link

Infinite Loop in devices without native Atomic support #5723

Closed fjtrujy closed 2 years ago

fjtrujy commented 2 years ago

Hello guys, I'm starting a Playstation 2 port of SDL, so far I have made some progress already, however running the tests I have noticed an infinite loop.

Having in mind that PS2 doesn't have native atomic support, we have the following scenario:

  1. Function SDL_CreateMutex calling SDL_Malloc https://github.com/libsdl-org/SDL/blob/main/src/thread/generic/SDL_sysmutex.c#L43
  2. Function SDL_malloc calling SDL_AtomicIncRef https://github.com/libsdl-org/SDL/blob/main/src/stdlib/SDL_malloc.c#L5390
  3. #define SDL_AtomicIncRef(a) SDL_AtomicAdd(a, 1) https://github.com/libsdl-org/SDL/blob/main/include/SDL_atomic.h#L321
  4. Function SDL_AtomicAdd calling SDL_AtomicCAS https://github.com/libsdl-org/SDL/blob/main/src/atomic/SDL_atomic.c#L257
  5. Function SDL_AtomicCAS calling enterLock https://github.com/libsdl-org/SDL/blob/main/src/atomic/SDL_atomic.c#L145
  6. Function enterLock calling SDL_AtomicLock https://github.com/libsdl-org/SDL/blob/main/src/atomic/SDL_atomic.c#L114
  7. Function SDL_AtomicLock calling SDL_AtomicTryLock https://github.com/libsdl-org/SDL/blob/main/src/atomic/SDL_spinlock.c#L165
  8. Function SDL_AtomicTryLock calling SDL_CreateMutex https://github.com/libsdl-org/SDL/blob/main/src/atomic/SDL_spinlock.c#L63

What is your suggestion to follow?

I was thinking of implementing my own "atomic" support, by disabling interruptions, then performing the operation, and finally, enabling interruption backs. https://github.com/libsdl-org/SDL/blob/main/src/atomic/SDL_atomic.c#L236

fjtrujy commented 2 years ago

Pinging @sharkwouter because I have already notified him.

sezero commented 2 years ago

Having in mind that PS2 doesn't have native atomic support,

Is this a limitation of MIPS cpu in PS2? (forgive my ignorance on the topic.)

icculus commented 2 years ago

There's only one CPU core; can we just implement the atomics with standard operations?

(I'm not sure the ramifications of actually doing that, but if there's nowhere the CPU could race...)

fjtrujy commented 2 years ago

Having in mind that PS2 doesn't have native atomic support,

Is this a limitation of MIPS cpu in PS2? (forgive my ignorance on the topic.)

Yes is a limitation in that CPU, I suppose that legacy CPUs could have that limitation

fjtrujy commented 2 years ago

There's only one CPU core; can we just implement the atomics with standard operations?

(I'm not sure the ramifications of actually doing that, but if there's nowhere the CPU could race...)

This is exactly what I was suggesting, we have just a single CPU, however, we could have several threads, and then you can't assure an atomic operation unless you disable the interruptions, then the interruption to change thread won't be triggered and you can perform the operation safely, once the operation is done, you enable back the interruptions

icculus commented 2 years ago

This is exactly what I was suggesting, we have just a single CPU, however, we could have several threads, and then you can't assure an atomic operation unless you disable the interruptions, then the interruption to change thread won't be triggered and you can perform the operation safely, once the operation is done, you enable back the interruptions

Ah, I missed this part in your original post, sorry to make you repeat yourself. This seems like the correct approach.

cgutman commented 2 years ago

Having in mind that PS2 doesn't have native atomic support

Are you sure? According to Wikipedia, it's a MIPS III CPU and LL/SC instructions were introduced in MIPS II.

LL/SC can be used to implement SDL_AtomicCAS() which can then be used as the base atomic primitive for all the other SDL_Atomic ops.

Edit: Nevermind, saw https://lore.kernel.org/linux-mips/cover.1567326213.git.noring@nocrew.org/

  1. The R5900 is the main processor that runs the kernel[1]. It implements the 64-bit MIPS III instruction set except LL, SC, LLD and SCD

:(

fjtrujy commented 2 years ago

Having in mind that PS2 doesn't have native atomic support

Are you sure? According to Wikipedia, it's a MIPS III CPU and LL/SC instructions were introduced in MIPS II.

LL/SC can be used to implement SDL_AtomicCAS() which can then be used as the base atomic primitive for all the other SDL_Atomic ops.

Edit: Nevermind, saw https://lore.kernel.org/linux-mips/cover.1567326213.git.noring@nocrew.org/

  1. The R5900 is the main processor that runs the kernel[1]. It implements the 64-bit MIPS III instruction set except LL, SC, LLD and SCD

:(

Exactly :( the R5900 is a custom MIPS processor almost MIPS III with some MIPS IV instructions

icculus commented 2 years ago

Are we good to close this issue, since there's a plan in place?

fjtrujy commented 2 years ago

I have a PR pending to be done with a solution over there.

I didn’t created yet because I wanted to double-check that everything was working fine (testing using your available tests).

I want to create a initial PS2 port just implementing Threads, Atomics, FileSystem, Mutex and Semaphores. Then I will create other PRs adding more drivers.

So If you don’t mind keep this open and I will link this issue to the PR.