-
Notifications
You must be signed in to change notification settings - Fork 157
recipes-support: Add resource-tuner recipe #1135
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
base: master
Are you sure you want to change the base?
Conversation
lumag
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
On top of that:
- Commit message is missing.
- Commit subject doesn't follow the established practice
- The PV is invalid, the relase is 0.1, not 1.0.0
- SRCREV doesn't point to the release commit.
|
Please avoid using |
|
No stray merge commits, thank you. |
| and policy enforcement using kernel interfaces like cgroups and sysfs." | ||
|
|
||
| SRC_URI = "git://github.com/qualcomm/resource-tuner.git;protocol=https;branch=main" | ||
| SRCREV = "26ff78ec039431538c5fa0c6b6fd30d9883fe486" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This SRCREV doesn't belong to any of the tags. At the same time you've named the recipe foo_1.0.0, whcih means that you've packaged the 1.0.0 release of the tuner.
|
|
||
| inherit cmake pkgconfig systemd | ||
|
|
||
| EXTRA_OECMAKE += "${S}/Build" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why?
| PACKAGECONFIG[tests] = "-DBUILD_TESTS=ON,-DBUILD_TESTS=OFF" | ||
|
|
||
| SYSTEMD_SERVICE:${PN} = "resource-tuner.service" | ||
| FILES:${PN} += "${sysconfdir}/resource-tuner/**" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Double asterisk
|
|
||
| inherit cmake pkgconfig systemd | ||
|
|
||
| PACKAGECONFIG ??= "signals cli state-detector tests" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: we try to keep lists sorted, swapping signals and cli would help with that. I've marked this as a nit since PACKAGECONFIG entries often benefit from being sorted by effect.
00ee08f to
f44b3b2
Compare
| SRC_URI = "git://github.com/qualcomm/resource-tuner.git;protocol=https;branch=main;tag=v${PV}" | ||
| SRCREV = "681a19ba3f6731f3acf1e1022fcd94298eecff82" | ||
|
|
||
| inherit cmake pkgconfig systemd |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you really have a strict dependency on systemd?
Can we make this optional and enable based on PACKAGECONFIG? You could check DISTRO_FEATURES and only enable systemd when it is used by the distro.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd say, it should be dropped. I think it's another case of systemd dependency just to get the system unit location.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd say, it should be dropped. I think it's another case of systemd dependency just to get the system unit location.
It seems to be using it for more than that: https://github.com/qualcomm/resource-tuner/blob/600e1ca306de1ddfa8b4a1c0a4676e65e71fa311/StateDetector.cpp#L5
And, compilation of this is controlled by state-detector PACKAGECONFIG in this recipe so the explicit systemd dependency can probably go there.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We are using systemd to get the Display states (ON/OFF) in this module, we will control systemd dependency using DISTRO_FEATURES.
This is because nodistro is used in yocto-check-layer and you are explicitly depending on systemd. |
I think I wrote this in another recipe review, but anyway: recipes MAY NOT depend on |
Add initial BitBake recipe for resource-tuner in the recipe-support layer. Signed-off-by: Jagadeesh Pagadala <[email protected]>
recipes-support: Add resource-tuner recipe.
Resource Tuner is a lightweight daemon that monitors and dynamically regulates CPU, memory, and I/O usage of user-space processes. It leverages kernel interfaces like procfs, sysfs and cgroups to enforce runtime policies, ensuring system stability and performance in embedded and resource-constrained environments.