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

Fix sys_net_bnet_poll regression #8337

Merged
merged 1 commit into from Jun 4, 2020
Merged

Fix sys_net_bnet_poll regression #8337

merged 1 commit into from Jun 4, 2020

Conversation

@AniLeo
Copy link
Member

@AniLeo AniLeo commented Jun 4, 2020

On #8019, a buffer for the fds parameter was added
However, the fds_buf buffer was only copied to the fds parameter on only one out of three possible function returns (at the very end of the function)

I'm unsure why this change was made, but copying the fds_buf buffer to the fds parameter before every possible return fixes networking issues such as failed Netflix connection


Netflix

Before:
image

After:
image


YouTube

Before: Black Screen

After:
image

Requesting review from @elad335

@AniLeo AniLeo added the Network label Jun 4, 2020
@elad335
Copy link
Contributor

@elad335 elad335 commented Jun 4, 2020

You need to touch VM memory outside of any mutex scope, page fault notification may arise during that and the faulting thread must not hold any mutex at the time.

@AniLeo
Copy link
Member Author

@AniLeo AniLeo commented Jun 4, 2020

I understand the problem, but can't figure out a correct fix, any suggestion?

I tried inverting the if (ms == 0 || signaled) check, removing the return inside and nesting the subsequent code inside to make that path go through the unlock process and the final memcpy instead, but it didn't work.

@elad335
Copy link
Contributor

@elad335 elad335 commented Jun 4, 2020

Maybe use a lambda wrapped to wrap the block of code from std::lock_guard nw_lock(g_fxo->get<network_context>()->s_nw_mutex); to the end of its' lifetime. return the value as is then forward the return value of the lambda to the syscall return value if needs to be returned.

Co-authored-by: Eladash <elad3356p@gmail.com>
@AniLeo AniLeo force-pushed the AniLeo:master branch from 4a2b997 to 5c8b6cf Jun 4, 2020
@AniLeo AniLeo added the Bugfix label Jun 4, 2020
@AniLeo
Copy link
Member Author

@AniLeo AniLeo commented Jun 4, 2020

Merging as hotfix, verified working

@AniLeo AniLeo merged commit 9657b3f into RPCS3:master Jun 4, 2020
6 checks passed
6 checks passed
FreeBSD 12.2 (snapshot) Task Summary
Details
RPCS3.rpcs3 Build #20200604.2 succeeded
Details
RPCS3.rpcs3 (Linux_Build Clang) Linux_Build Clang succeeded
Details
RPCS3.rpcs3 (Linux_Build GCC) Linux_Build GCC succeeded
Details
RPCS3.rpcs3 (Windows_Build) Windows_Build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@Nekotekina
Copy link
Member

@Nekotekina Nekotekina commented Jun 4, 2020

@elad335 @AniLeo It's too late, but note that it's quite an ugly lambda when there is an easier way. You need to unlock a mutex, you just unlock it (std::unique_lock).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants
You can’t perform that action at this time.