Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove SPU and PPU destructors #9630

Merged
merged 2 commits into from Jan 21, 2021
Merged

Remove SPU and PPU destructors #9630

merged 2 commits into from Jan 21, 2021

Conversation

elad335
Copy link
Contributor

@elad335 elad335 commented Jan 20, 2021

Enhancements

  • Allow to hold smart pointers of SPU and PPU outside of IDM, need not worrying about delaying the destructors.

Bugfixes

  • PPU exit syscall delayed stack memory deallocation to the destructor of PPU, causing stack to not be released for some time or not at all with detached PPU thread.
  • Fix race-condition in SPU LS mapping and unmapping.

@elad335 elad335 force-pushed the destruct branch 2 times, most recently from 819b862 to 73fd0c9 Compare January 20, 2021 04:04
@elad335
Copy link
Contributor Author

elad335 commented Jan 20, 2021

  • SPU LS unmapping was not thread-safe as a result of a typo, fixed.

@elad335 elad335 force-pushed the destruct branch 2 times, most recently from 4f9f67e to 9f0def4 Compare January 20, 2021 04:23
@Nekotekina
Copy link
Member

Nekotekina commented Jan 20, 2021

Actually, memory_reserve should be under vm mutex too, totally forgot about it.
memory_release is fine though.

@elad335
Copy link
Contributor Author

elad335 commented Jan 20, 2021

Done.

@Nekotekina
Copy link
Member

Nekotekina commented Jan 21, 2021

Tested it a bit, freezes on close.
Noticed deallocation failure in spu_thread::cleanup another time.

rpcs3/Emu/Cell/SPUThread.cpp:1680:12: warning: unused variable ‘addr’ [-Wunused-variable]
 1680 |  const u32 addr = group ? SPU_FAKE_BASE_ADDR + SPU_LS_SIZE * (id & 0xffffff) : RAW_SPU_BASE_ADDR + RAW_SPU_OFFSET * index;
      |            ^~~~

@Nekotekina
Copy link
Member

Nekotekina commented Jan 21, 2021

Seems to work now! 👍
or at least ready for testing in production

@Nekotekina Nekotekina merged commit 12e1be2 into RPCS3:master Jan 21, 2021
elad335 added a commit to elad335/rpcs3 that referenced this pull request Jan 21, 2021
Co-Authored-By: Ivan <nekotekina@gmail.com>
elad335 added a commit to elad335/rpcs3 that referenced this pull request Jan 21, 2021
Co-Authored-By: Ivan <nekotekina@gmail.com>
elad335 added a commit to elad335/rpcs3 that referenced this pull request Jan 21, 2021
Co-Authored-By: Ivan <nekotekina@gmail.com>
elad335 added a commit to elad335/rpcs3 that referenced this pull request Jan 21, 2021
Co-Authored-By: Ivan <nekotekina@gmail.com>
Nekotekina added a commit that referenced this pull request Jan 21, 2021
Co-Authored-By: Ivan <nekotekina@gmail.com>
@elad335 elad335 deleted the destruct branch March 2, 2021 13:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants