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

Implement std threads support #10

Merged
merged 16 commits into from
Mar 1, 2022

Conversation

library/std/src/thread/mod.rs Outdated Show resolved Hide resolved
@Meziu
Copy link
Owner

Meziu commented Feb 10, 2022

Also, while the function docs are fine, add an explanation of why we need those implemented.

library/std/src/sys/unix/thread.rs Outdated Show resolved Hide resolved
library/std/src/thread/mod.rs Outdated Show resolved Hide resolved
library/std/src/sys/unix/thread.rs Outdated Show resolved Hide resolved
library/std/src/sys/unix/thread.rs Outdated Show resolved Hide resolved
@AzureMarker
Copy link
Author

Also, while the function docs are fine, add an explanation of why we need those implemented.

I added a little bit, but can you give an example of what you're thinking?

@AzureMarker AzureMarker mentioned this pull request Feb 12, 2022
@AzureMarker
Copy link
Author

@ian-h-chamberlain based on your comment here I did some poking around and it may be possible to use libctru APIs to set the priority and affinity after-the-fact:
https://rust-lang.zulipchat.com/#narrow/stream/122651-general/topic/std.3A.3Athread.20support.20for.20armv6k-nintendo-3ds

I'll look into supporting that in std, but we still may want the current API + the ability to set it after thread spawn.

@AzureMarker
Copy link
Author

AzureMarker commented Feb 12, 2022

These are some interesting wiki pages:
https://www.3dbrew.org/wiki/Multi-threading
https://www.3dbrew.org/wiki/KThread

From this, we should change how we refer to the core ID we pass during thread creation. "Affinity" usually refers to a CPU affinity bit mask that tells the kernel which cores a thread could run on (or a list of masks, for multiple CPUs). The 3DS wiki refers to the core ID passed during thread creation as the "ideal processor" ID. The 3DS kernel stores both pieces of information (see offsets 0x7E and 0x90 on the KThread page).

The affinity mask concept is pretty common, so there are some existing pthread APIs we could reuse. Similar for thread priority by the way. But the ideal processor idea is not so common, so I think we will still need our own nonstandard pthread function for that. Based on the naming scheme used below, I nominate pthread_attr_setidealprocessor_np and pthread_setidealprocessor_np (plus get variants).

Here is a listing of the known (and slightly less known - nonstandard) pthread functions we could reuse:

Function Usage Link
pthread_attr_setschedparam Set the priority attribute value https://man7.org/linux/man-pages/man3/pthread_attr_getschedparam.3p.html
pthread_attr_getschedparam Get the priority attribute value https://man7.org/linux/man-pages/man3/pthread_attr_getschedparam.3p.html
pthread_setschedparam Set the thread's priority value https://man7.org/linux/man-pages/man3/pthread_setschedparam.3.html
pthread_getschedparam Get the thread's priority value https://man7.org/linux/man-pages/man3/pthread_setschedparam.3.html
pthread_attr_setaffinity_np Set the affinity mask attribute value https://man7.org/linux/man-pages/man3/pthread_attr_setaffinity_np.3.html
pthread_attr_getaffinity_np Get the affinity mask attribute value https://man7.org/linux/man-pages/man3/pthread_attr_setaffinity_np.3.html
pthread_setaffinity_np Set the thread's affinity mask value https://man7.org/linux/man-pages/man3/pthread_setaffinity_np.3.html
pthread_getaffinity_np Get the thread's affinity mask value https://man7.org/linux/man-pages/man3/pthread_setaffinity_np.3.html

Note that the pthread_attr_setaffinity_np function, if we used it, would only apply after creating the thread since we don't have an API to set it during creation. See also https://stackoverflow.com/a/35278467.

I think we should keep the thread builder extension trait added in this PR, but we could also consider adding APIs for modifying the priority, ideal processor, and affinity mask of existing threads (might want to keep these in std::os::horizon::thread).

@Meziu
Copy link
Owner

Meziu commented Feb 12, 2022

Great. This PR will be merged after the std PR, so you can make all the changes needed. Just rename them in libc and we are good to go.

@AzureMarker
Copy link
Author

We could also merge this PR when it's ready to get it out of the way, and make a separate branch for the std PR. We may need to clean up the commit history or split it into two PRs, so making a separate branch might be necessary either way.

@Meziu
Copy link
Owner

Meziu commented Feb 12, 2022

Yeah we should squash some stuff and clean up the repo. Let’s make a branch.

@AzureMarker
Copy link
Author

AzureMarker commented Feb 13, 2022

By the way, this thread is interesting (even though it never got properly answered):
https://gbatemp.net/threads/can-horizon-indepently-relocate-threads-to-cpus-in-their-affinity-masks.556096/

I don't have a New 3DS/2DS to test with (to see if it balances across multiple app cores), but it does seem a little odd that there's the whole affinity mask feature if it's not really used by the kernel after initial scheduling.

@AzureMarker
Copy link
Author

I did a quick test to try migrating a thread from sys core to app core (and vice versa) via svcSetThreadIdealProcessor. It fails with an error (result), level = RL_FATAL, summary = RS_NOTSUPPORTED, description = RD_NOT_IMPLEMENTED. This happens even if the thread's affinity mask allows it to exist on either app or sys core (mask = 0x3).

Due to this, I don't think it makes sense to add this feature (setting the ideal processor after spawning) to std. I'm not sure if it's worthwhile to add the get version either, since that could return stuff like -2 or -1 which don't tell you what processor it is actually on. The only API I know of to do that is svcGetProcessorID, and that only gets the current thread's value.

So basically, (I think) we should just have functions to get the current thread's priority and processor ID in std, but not the ideal thread or affinity mask.

@ian-h-chamberlain
Copy link

I don't have a New 3DS/2DS to test with (to see if it balances across multiple app cores), but it does seem a little odd that there's the whole affinity mask feature if it's not really used by the kernel after initial scheduling.

I have a N3DS if you have an example you'd like me to test – if those functions are implemented on the new processors perhaps it would be worth keeping the implementation?

@AzureMarker
Copy link
Author

AzureMarker commented Feb 15, 2022

@ian-h-chamberlain nice, I was testing with a variant of this example:
rust3ds/ctru-rs@353b6c1

I think you have to enable the new cores through some mechanism. This is what the docs say:

Processor #2 is New3DS exclusive. Normal applications can create threads on this core if the exheader kernel flags bitmask has 0x2000 set.

Then just change the ideal processor processor ID setting of the thread to -1 and try to move the thread to another core with something like this:

println!("Moving to app core 2");
let result =
    unsafe { ctru_sys::svcSetThreadIdealProcessor(ctru_sys::CUR_THREAD_HANDLE, 2) };
if ctru_sys::R_FAILED(result) {
    println!("result level = {}", ctru_sys::R_LEVEL(result));
    println!("result summary = {}", ctru_sys::R_SUMMARY(result));
    println!("result description = {}", ctru_sys::R_DESCRIPTION(result));
    return;
}

println!("On app core 2");
print_affinity_mask("app 2 thread");
print_thread_id("app 2 thread");
print_thread_list();

@Meziu
Copy link
Owner

Meziu commented Feb 15, 2022

Even if it was technically possible to move threads around, it looks like the 3DS is the worst system to do so. There are 2 VERY different cores on a normal 3DS, and 4 (yet one is totally unusable) on a New3DS. I have never even thought of trying: why would you try to move a thread that’s supposed to be preemptive to a core that isn’t? Or try to move a thread to a core some systems don’t even have? It usually ends up being: common threads on core#0, and something important on core#1. I don’t think it matters.

@ian-h-chamberlain
Copy link

Even if it was technically possible to move threads around, it looks like the 3DS is the worst system to do so. There are 2 VERY different cores on a normal 3DS, and 4 (yet one is totally unusable) on a New3DS. I have never even thought of trying: why would you try to move a thread that’s supposed to be preemptive to a core that isn’t? Or try to move a thread to a core some systems don’t even have? It usually ends up being: common threads on core#0, and something important on core#1. I don’t think it matters.

Ok, I got a chance to test this on my device and it seems that it's not supported anyway.
I got an error code with summary code "Not supported", description "Not implemented", so I think we can safely ignore this functionality for the reasons you describe and the fact that it doesn't work anyway.

@AzureMarker
Copy link
Author

AzureMarker commented Feb 16, 2022

I think the PR is ready to merge, whenever we want to do that. For the std PRs we can cherry-pick changes to separate branches, so I don't think that would block merging this PR.

Priority and affinity are passed through special pthread attr functions
which will be added to our libc fork.

The current thread's priority also needs to be fetched since it is the
default priority value. This is done using another new pthread function,
though this value/mechanism isn't exposed as part of std's API.
@AzureMarker
Copy link
Author

@Meziu Are you waiting to merge this? What's the plan?

@Meziu
Copy link
Owner

Meziu commented Feb 25, 2022

@Meziu Are you waiting to merge this? What's the plan?

Aren’t there some memory issues related with #15 and rust3ds/ctru-rs#48 ? I thought the code wasn’t final yet.

@AzureMarker
Copy link
Author

@Meziu Are you waiting to merge this? What's the plan?

Aren’t there some memory issues related with #15 and Meziu/ctru-rs#48 ? I thought the code wasn’t final yet.

Oh yeah, not sure how I forgot that blocker. I guess I thought of this code as "done", and any future changes would need another PR. With that in mind though, I do have one small change planned for this PR before it merges (along with the review comments).

@AzureMarker
Copy link
Author

Once rust3ds/pthread-3ds#14 merges, I think this could merge too. The small change I wanted to make was 3b5e287.

@Meziu Meziu merged commit a89957a into Meziu:horizon-std Mar 1, 2022
@AzureMarker
Copy link
Author

Thanks @Meziu and @ian-h-chamberlain for reviewing and keeping up with this PR!

@AzureMarker AzureMarker deleted the feature/std-threads branch March 2, 2022 04:13
bors added a commit to rust-lang/libc that referenced this pull request Mar 12, 2022
Horizon (Nintendo 3DS) pthread functions and non-portable extensions

This PR adds some standard and nonstandard pthread/threading functions to the horizon (Nintendo 3DS) operating system. This will allow us to open a PR to std for std threads support.

The Nintendo 3DS doesn't have a full libc implementation, so there are some user libraries which implement part of libc, such as libctru:
https://github.com/devkitPro/libctru
For some more context on this situation, see rust-random/getrandom#248

But std doesn't want to interface directly with anything that could be seen as using the "internal" interfaces of the 3DS or consoles in general:
rust-lang/rust#88529 (comment)

So we work around this by implementing standard pthread interfaces, plus some nonstandard ones as necessary, and expose that interface to std:
https://github.com/Meziu/pthread-3ds

Here's the justifications for the nonstandard interfaces:
* `pthread_attr_setprocessorid_np` (and the `get` version):
  The 3DS requires the core a thread runs on to be specified at thread creation time. I'm pretty sure you can't migrate threads across cores. Additionally, the cores act differently (ex. one core is cooperatively scheduled). This means we need to have an API to set the core to use in the thread attributes struct. We didn't find any relevant standard API for this, so we added a nonstandard one.
* `pthread_getprocessorid_np`:
  The 3DS lets you get the core/processor ID of the executing thread. But it doesn't have a function to get the core ID of any arbitrary thread. We didn't find any standard APIs which would match these semantics, so we added a nonportable one. Once place this API is used is to get the default core/processor ID when spawning a thread (spawn the thread on the current processor).

For more context, see:
* Meziu/rust-horizon#10
  * Especially Meziu/rust-horizon#10 (comment)
* rust-random/getrandom#248

cc: `@ian-h-chamberlain` `@Meziu`
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.

3 participants