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

lib/ukschedcoop: Keep track of thread execution times #965

Closed
wants to merge 5 commits into from

Conversation

skuenzer
Copy link
Member

Base target

  • Architecture(s): [all]
  • Platform(s): [all]
  • Application(s): [all]

Additional configuration

  • CONFIG_LIBUKSCHED=y
  • CONFIG_LIBUKSCHEDCOOP=y

Description of changes

This PR introduces keeping track of thread execution times with lib/schedcoop and is implemented with minimal overhead. An interface for returning the idle thread enables CPU utilization computations by putting thread execution times in relation over a time window.

@skuenzer skuenzer added this to the v0.14.0 (Prometheus) milestone Jun 30, 2023
@skuenzer skuenzer requested a review from michpappas June 30, 2023 13:34
@skuenzer skuenzer requested review from a team as code owners June 30, 2023 13:34
@skuenzer skuenzer removed request for a team June 30, 2023 13:34
Copy link
Member

@michpappas michpappas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @skuenzer only a few minor comments 👍🏼

include/uk/arch/time.h Outdated Show resolved Hide resolved
include/uk/arch/time.h Outdated Show resolved Hide resolved
lib/uksched/sched.c Outdated Show resolved Hide resolved
lib/uksched/exportsyms.uk Outdated Show resolved Hide resolved
lib/uksched/sched.c Outdated Show resolved Hide resolved
lib/uksched/include/uk/sched.h Outdated Show resolved Hide resolved
@unikraft-bot unikraft-bot added area/include Part of include/uk area/lib Internal Unikraft Microlibrary lang/c Issues or PRs to do with C/C++ lib/uksched lib/ukschedcoop labels Jul 5, 2023
@razvand razvand requested a review from mogasergiu July 24, 2023 20:11
Copy link
Member

@mogasergiu mogasergiu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just some small nitpicks, feel free to disregard them. By the way, I believe you forgot to write the commit message for the last commit of this PR (lib/ukschedcoop: Implement interface for returning idle thread).

lib/ukschedcoop/schedcoop.c Show resolved Hide resolved
lib/uksched/include/uk/sched.h Show resolved Hide resolved
`__nsec` and `__snsec` are defined as 64-bit integers based on `__u64` and
`__s64`. Depending on the target architecture, these data types are based
on either `long` or `long long`. The time conversion macros, such as
`ukarch_time_nsec_to_sec()`, did not strictly adhere to the data type
definition, as some basic constants were defined as `UL`, while others
were defined as `ULL`, resulting in promotion to `long long` when the
architecture used `long` for 64-bit values. Although `long long` is
effectively treated as a 64-bit integer by `gcc`, the compiler still
issues a warning for mismatched integer types when the print format
definitions `__PRInsec` and `__PRIsnec` are used and a promotion had
occurred.

Signed-off-by: Simon Kuenzer <simon@unikraft.io>
This commit extends `struct uk_thread` with a field for keeping track of
the execution time of a thread.

Checkpatch-Ignore: SPLIT_STRING
Signed-off-by: Simon Kuenzer <simon@unikraft.io>
This commit introduces an implementation for tracking thread execution
time with minimal execution overhead. On every scheduler call
(`uk_sched_yield()`), the execution time of the current thread is updated.

Signed-off-by: Simon Kuenzer <simon@unikraft.io>
@skuenzer
Copy link
Member Author

skuenzer commented Aug 8, 2023

Just some small nitpicks, feel free to disregard them. By the way, I believe you forgot to write the commit message for the last commit of this PR (lib/ukschedcoop: Implement interface for returning idle thread).

I did not really forget 😇 but I added something now.

With the intention for accessing metrics (like thread execution time), this
commit introduces an interface to return a reference to the idle thread.

Signed-off-by: Simon Kuenzer <simon@unikraft.io>
This commit implements the callback for `uk_sched_idle_thread()`
for `lib/ukschedcoop`.

Signed-off-by: Simon Kuenzer <simon@unikraft.io>
@unikraft-bot
Copy link
Member

Checkpatch passed

Beep boop! I ran Unikraft's checkpatch.pl support script on your pull request and it all looks good!

SHA commit checkpatch
47b052e include/arch: time.h: Avoid unnecessary integer promotion
ca00097 lib/uksched: Support thread execution time
Checkpatch-Ignore: SPLIT_STRING
45c8bd0 lib/ukschedcoop: Keep track of thread execution time
7c2084f lib/uksched: Interface for returning idle thread
38b3c61 lib/ukschedcoop: Implement interface for returning idle thread

Copy link
Member

@michpappas michpappas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, thanks.

Reviewed-by: Michalis Pappas michalis@unikraft.io

Copy link
Member

@mogasergiu mogasergiu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed-by: Sergiu Moga sergiu@unikraft.io

Copy link
Contributor

@razvand razvand left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved-by: Razvan Deaconescu razvand@unikraft.io

unikraft-bot pushed a commit that referenced this pull request Aug 9, 2023
This commit extends `struct uk_thread` with a field for keeping track of
the execution time of a thread.

Checkpatch-Ignore: SPLIT_STRING
Signed-off-by: Simon Kuenzer <simon@unikraft.io>
Reviewed-by: Michalis Pappas <michalis@unikraft.io>
Reviewed-by: Sergiu Moga <sergiu@unikraft.io>
Approved-by: Razvan Deaconescu <razvand@unikraft.io>
Tested-by: Unikraft CI <monkey@unikraft.io>
GitHub-Closes: #965
unikraft-bot pushed a commit that referenced this pull request Aug 9, 2023
This commit introduces an implementation for tracking thread execution
time with minimal execution overhead. On every scheduler call
(`uk_sched_yield()`), the execution time of the current thread is updated.

Signed-off-by: Simon Kuenzer <simon@unikraft.io>
Reviewed-by: Michalis Pappas <michalis@unikraft.io>
Reviewed-by: Sergiu Moga <sergiu@unikraft.io>
Approved-by: Razvan Deaconescu <razvand@unikraft.io>
Tested-by: Unikraft CI <monkey@unikraft.io>
GitHub-Closes: #965
unikraft-bot pushed a commit that referenced this pull request Aug 9, 2023
With the intention for accessing metrics (like thread execution time), this
commit introduces an interface to return a reference to the idle thread.

Signed-off-by: Simon Kuenzer <simon@unikraft.io>
Reviewed-by: Michalis Pappas <michalis@unikraft.io>
Reviewed-by: Sergiu Moga <sergiu@unikraft.io>
Approved-by: Razvan Deaconescu <razvand@unikraft.io>
Tested-by: Unikraft CI <monkey@unikraft.io>
GitHub-Closes: #965
unikraft-bot pushed a commit that referenced this pull request Aug 9, 2023
This commit implements the callback for `uk_sched_idle_thread()`
for `lib/ukschedcoop`.

Signed-off-by: Simon Kuenzer <simon@unikraft.io>
Reviewed-by: Michalis Pappas <michalis@unikraft.io>
Reviewed-by: Sergiu Moga <sergiu@unikraft.io>
Approved-by: Razvan Deaconescu <razvand@unikraft.io>
Tested-by: Unikraft CI <monkey@unikraft.io>
GitHub-Closes: #965
@unikraft-bot unikraft-bot added the ci/merged Merged by CI label Aug 9, 2023
@nderjung
Copy link
Member

nderjung commented Aug 9, 2023

This broke the build on staging with:

 E  /usr/bin/ld: warning: /github/workspace/_helloworld/.unikraft/build/libmusl/setjmp.x86_64.o: missing .note.GNU-stack section implies executable stack
 E  /usr/bin/ld: NOTE: This behaviour is deprecated and will be removed in a future version of the linker
 i    OBJCOPY libmusl.o
 i    LD      helloworld_qemu-x86_64.dbg
 E  /usr/bin/ld: warning: /github/workspace/_helloworld/.unikraft/build/libkvmplat.o: requires executable stack (because the .note.GNU-stack section is executable)
 E  /usr/bin/ld: /github/workspace/_helloworld/.unikraft/build/libmusl.o: in function `cpow':
 E  /github/workspace/_helloworld/.unikraft/build/libmusl/origin/musl-1.2.3//src/complex/cpow.c:7: undefined reference to `__muldc3'
 E  /usr/bin/ld: /github/workspace/_helloworld/.unikraft/build/libmusl.o: in function `cpowf':
 E  /github/workspace/_helloworld/.unikraft/build/libmusl/origin/musl-1.2.3//src/complex/cpowf.c:5: undefined reference to `__mulsc3'
 E  /usr/bin/ld: /github/workspace/_helloworld/.unikraft/build/libmusl.o: in function `cpowl':
 E  /github/workspace/_helloworld/.unikraft/build/libmusl/origin/musl-1.2.3//src/complex/cpowl.c:11: undefined reference to `__mulxc3'
 E  collect2: error: ld returned 1 exit status
 E  make[1]: *** [/github/workspace/plat/kvm/Linker.uk:48: /github/workspace/_helloworld/.unikraft/build/helloworld_qemu-x86_64.dbg] Error 1
 E  make: *** [Makefile:1055: sub-make] Error 2

See: https://github.com/unikraft/unikraft/actions/runs/5808848506/job/15746476437#step:5:3954

@StefanJum
Copy link
Member

This broke the build on staging with:

 E  /usr/bin/ld: warning: /github/workspace/_helloworld/.unikraft/build/libmusl/setjmp.x86_64.o: missing .note.GNU-stack section implies executable stack
 E  /usr/bin/ld: NOTE: This behaviour is deprecated and will be removed in a future version of the linker
 i    OBJCOPY libmusl.o
 i    LD      helloworld_qemu-x86_64.dbg
 E  /usr/bin/ld: warning: /github/workspace/_helloworld/.unikraft/build/libkvmplat.o: requires executable stack (because the .note.GNU-stack section is executable)
 E  /usr/bin/ld: /github/workspace/_helloworld/.unikraft/build/libmusl.o: in function `cpow':
 E  /github/workspace/_helloworld/.unikraft/build/libmusl/origin/musl-1.2.3//src/complex/cpow.c:7: undefined reference to `__muldc3'
 E  /usr/bin/ld: /github/workspace/_helloworld/.unikraft/build/libmusl.o: in function `cpowf':
 E  /github/workspace/_helloworld/.unikraft/build/libmusl/origin/musl-1.2.3//src/complex/cpowf.c:5: undefined reference to `__mulsc3'
 E  /usr/bin/ld: /github/workspace/_helloworld/.unikraft/build/libmusl.o: in function `cpowl':
 E  /github/workspace/_helloworld/.unikraft/build/libmusl/origin/musl-1.2.3//src/complex/cpowl.c:11: undefined reference to `__mulxc3'
 E  collect2: error: ld returned 1 exit status
 E  make[1]: *** [/github/workspace/plat/kvm/Linker.uk:48: /github/workspace/_helloworld/.unikraft/build/helloworld_qemu-x86_64.dbg] Error 1
 E  make: *** [Makefile:1055: sub-make] Error 2

See: https://github.com/unikraft/unikraft/actions/runs/5808848506/job/15746476437#step:5:3954

It's not related to this pr. Since this commit was merged in lib-musl, either LIBMUSL_COMPLEX needs to be unselected or lib-compiler-rt must be added to the build, so I guess the ci/cd configs must be updated to do one of those 2.

@nderjung
Copy link
Member

nderjung commented Aug 9, 2023

You're right, i just saw this now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/include Part of include/uk area/lib Internal Unikraft Microlibrary ci/merged Merged by CI lang/c Issues or PRs to do with C/C++ lib/uksched lib/ukschedcoop
Projects
Status: Done!
Development

Successfully merging this pull request may close these issues.

None yet

7 participants