au-ts
Multicore support for echo server
This PR adds multicore support to the echo server example.
To build for SMP, first a core configuration file must be created associating a core value to each PD in the system. There are three example core configuration files added by this PR in the core_config directory: single_core.json, two_core.json and four_core.json. single_core.json sets each PD's core to 0, but will use the SMP microkit board.
Once the core configuration json has been created, it must be passed to make by setting the SMP_CONFIG flag to its path. If this flag is set, make will automatically add the _4_cores suffix to MICROKIT_BOARD where required. The flag is optional, and if it is not set the non-SMP microkit board will be used.
Changes introduced by this PR:
benchmark.chas been restructured so that it is aware of which core it is running on for better benchmark result output.- Each
benchmarkPD signals the benchmark on the next core to propagate thestartandstopsignals. utilization_socket.cis now aware of how many cores are being utilised, and tracks the cycle counts for each active core. Thus it now has a variable number of cycle counter memory regions mapped in.- Restructure of benchmark configuration structs to enable variable behaviour depending on the number of active cores.
- Restructure of
meta.pyto enable additionalbenchmarkandidlePDs to be created depending on the number of active cores. This also requires that a variable number of.elfand.datafiles be created, thus these files are now created and formatted bymeta.pyrather thanecho.mk.
These changes enable the build system to automatically restructure the system file and outputted elfs based on the core configuration file passed, so that no additional code changes are required.
Other changes required for this to be merged:
- The non-SMP microkit SDKs must allow setting the cpu flag to 0 to avoid having to generate different system file formats for SMP and non-SMP.
- SMP versions of boards must be supported by microkit.
Note that the struct serialisation performed by meta.py assumes the target architecture is 8 byte aligned.
Not sure if we need to include the license prefix for the .json files. Seems like we don't use it for the driver config files?
Not sure if we need to include the license prefix for the
.jsonfiles. Seems like we don't use it for the driver config files?
You just need to add the files to here to get the CI to pass.
The JSON files are really annoying for the license check since you can't have comments in JSON so you can't put the license headers there. I'll have more of a look into the license checker to see if there's anything smarter we can do to about this, it's really annoying having to add an entry into that file every time we add a new driver. The reason I don't pattern match on all .json files is that there could be some in the future that are not UNSW copy-righted? Maybe there's a way around that? I don't know.
@Ivan-Velickovic Some things:
-
I notice that we now have a Python style checker, however the diff it produces is impossible to use (unlike the C diff), It is impossible for me to apply it with the git diff command without breaking it into many different chunks and modifying the file paths it references. Is there something that can be done to make this similar to the diff that the C style checker produces? As currently it would take me a very long time to manually fix it up....
-
Another thing that needs to be fixed up with Microkit is allowing the
cpu=0to be set for a PD on a single core Microkit board. This was something we discussed a while ago but forgot about.
I have done basic testing and everything works fine, @KurtWu10 will do some more extensive benchmarking.
It is impossible for me to apply it with the git diff command without breaking it into many different chunks and modifying the file paths it references. Is there something that can be done to make this similar to the diff that the C style checker produces? As currently it would take me a very long time to manually fix it up....
The intent is that you run the formatter tool locally (black, install via pip). I didn't realise that people did it via applying the diff directly.
But also, that diff looks like a standard git diff in the output to me? It's got the @ notation for line numbers and everything, all as one.
Another thing that needs to be fixed up with Microkit is allowing the cpu=0 to be set for a PD on a single core Microkit board. This was something we discussed a while ago but forgot about.
Both in my rewrite and Ivan's original this is allowed? If CPU is not specified it defaults to 0. CI fails because it doesn't have this changeset.
The intent is that you run the formatter tool locally (black, install via pip). I didn't realise that people did it via applying the diff directly.
I have now run it directly. However there was some reason I had been doing it manually up until now. I can't remember exactly why as it's been a long time - it may have had to do with the fact that the style checker only needed to be run on some files and not others, and it was a pain to configure then when applying the diff was relatively simple.
But also, that diff looks like a standard git diff in the output to me? It's got the
@notation for line numbers and everything, all as one.
@KurtWu10 also corroborated that it was corrupted/failed for him as well. I agree it looks standard but it was not :)
Both in my rewrite and Ivan's original this is allowed? If CPU is not specified it defaults to 0. CI fails because it doesn't have this changeset.
Sorry not sure what you mean by this. Which changeset am I missing?
But also, that diff looks like a standard git diff in the output to me? It's got the @ notation for line numbers and everything, all as one.
The issue with the diff is that it appears to be AI-generated to me: in the diff, this line does not exist in the source code.
Both in my rewrite and Ivan's original this is allowed? If CPU is not specified it defaults to 0. CI fails because it doesn't have this changeset.
As in, CI doesn't have the SMP version of microkit installed, because it's in no official version. Otherwise with the SMP version it should work fine? Unless it doesn't?
The issue with the diff is that it appears to be AI-generated to me: in the diff, this line does not exist in the source code.
It did, though? Courtney's formatting commit does make that change.
As in, CI doesn't have the SMP version of microkit installed, because it's in no official version. Otherwise with the SMP version it should work fine? Unless it doesn't?
Currently I'm using the smp branch of microkit for benchmarking, and I can get some data. I'm not sure whether comparing the smp microkit with the non-smp microkit is fair: should I re-run the single-core sddf benchmark on the smp microkit as well?
It did, though? Courtney's formatting commit does make that change.
Oh right, then I don't understand why the patch fails. She locally executed the formatter tool in the end.
OK, so if we run patch -d/ -p0 < diff.diff then it's fine (Tested locally). The issue is that it doesn't like the absolute path from /, it expects it to be relative to the current dir. (or in git apply's case relative to the git repository).
The other way is using -p<number of slashes> (which is the --strip option, per the manpage).
Currently I'm using the smp branch of microkit for benchmarking, and I can get some data. I'm not sure whether comparing the smp microkit with the non-smp microkit is fair: should I re-run the single-core sddf benchmark on the smp microkit as well?
Definitely yes, from my benchmarks (which are in the google sheets) this is where the majority of the overhead has come from. FYI, the multikernel branch in sDDF I copied from this branch and modified (a little) to work with both SMP and multikernel configs, which is what I did my benchmarks with. (not rebased though). For those benchmarks the main difference was just the SMP config itself, even on single core (again, see benchmarks in the google sheets and systems core channel).