plugins: add amd_x3d plugin for dual-CCD X3D scheduling mode#833
plugins: add amd_x3d plugin for dual-CCD X3D scheduling mode#833ptr1337 wants to merge 1 commit into
Conversation
Add a new amd_x3d plugin that controls the amd_x3d_vcache kernel driver's scheduling preference on dual-CCD 3D V-Cache processors. The plugin manages the sysfs knob exposed at: /sys/bus/platform/drivers/amd_x3d_vcache/*/amd_x3d_mode and supports the two kernel-defined modes: - cache: prefer the X3D CCD for gaming and cache-sensitive workloads - frequency: prefer the non-X3D CCD for compute workloads The sysfs path is discovered via glob so the plugin works across boards with different ACPI device names. On unsupported systems, or when the driver is not present, the plugin is a no-op. State is integrated with TuneD's standard command_set/command_get flow, so the previous mode is saved and restored automatically on profile switch. Add unit tests covering discovery, apply/verify/rollback, and the no-device case. Signed-off-by: Peter Jung <admin@ptr1337.dev>
|
@coderabbitai review |
📝 WalkthroughWalkthroughThis PR introduces ChangesAMD X3D Scheduling Plugin
🎯 3 (Moderate) | ⏱️ ~20 minutes
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tuned/plugins/plugin_amd_x3d.py`:
- Around line 105-106: The read of the sysfs node via
self._cmd.read_file(paths[0]) can raise OSError(ENOENT) if the device
disappears; wrap the call in a try/except that catches OSError and treats ENOENT
as a benign missing-device case by setting data = "" (or returning/continuing
the no-op behavior), while re-raising other OS errors; use errno.ENOENT (or
e.errno) to detect the specific error and update the logic around the data
variable (the block that currently does data =
self._cmd.read_file(paths[0]).strip() and the subsequent if not data:) so
disappearance becomes a no-op instead of an exception.
- Line 5: Replace the wildcard import from .decorators with an explicit import
of the two used symbols: import command_set and command_get from .decorators
(replace the current "from .decorators import *" with an explicit import of
command_set, command_get) so the module only imports the names it uses and
avoids the Ruff F403 wildcard import warning.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 016b196d-fba1-45fd-8350-6153385a8f62
📒 Files selected for processing (2)
tests/unit/plugins/test_plugin_amd_x3d.pytuned/plugins/plugin_amd_x3d.py
| import errno | ||
|
|
||
| from . import base | ||
| from .decorators import * |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Verify wildcard import exists and confirm actual decorator symbols used here.
rg -n 'from \.decorators import \*|@command_set|@command_get' tuned/plugins/plugin_amd_x3d.py
# Verify those symbols are defined in decorators module.
fd -i 'decorators.py' tuned/plugins --exec rg -n '\bdef command_set\b|\bdef command_get\b'Repository: redhat-performance/tuned
Length of output: 228
🏁 Script executed:
# Check if any other decorators or symbols from .decorators are used in the file
rg -n '@[a-z_]+' tuned/plugins/plugin_amd_x3d.py | head -20Repository: redhat-performance/tuned
Length of output: 135
🏁 Script executed:
# Check the full decorators.py file to see all exported symbols
fd -i 'decorators.py' tuned/plugins --exec cat -n {}Repository: redhat-performance/tuned
Length of output: 1401
Replace wildcard decorator import with explicit symbols.
Line 5 uses from .decorators import *, which is flagged by Ruff (F403) and makes imported names less explicit. The file only uses command_set and command_get.
💡 Proposed fix
-from .decorators import *
+from .decorators import command_get
+from .decorators import command_set📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| from .decorators import * | |
| from .decorators import command_get | |
| from .decorators import command_set |
🧰 Tools
🪛 Ruff (0.15.12)
[error] 5-5: from .decorators import * used; unable to detect undefined names
(F403)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tuned/plugins/plugin_amd_x3d.py` at line 5, Replace the wildcard import from
.decorators with an explicit import of the two used symbols: import command_set
and command_get from .decorators (replace the current "from .decorators import
*" with an explicit import of command_set, command_get) so the module only
imports the names it uses and avoids the Ruff F403 wildcard import warning.
| data = self._cmd.read_file(paths[0]).strip() | ||
| if not data: |
There was a problem hiding this comment.
Handle disappearing sysfs node during reads.
Line 105 can raise OSError(ENOENT) if the device vanishes after discovery. That breaks the intended no-op behavior for unsupported/missing devices.
💡 Proposed fix
- data = self._cmd.read_file(paths[0]).strip()
+ try:
+ data = self._cmd.read_file(paths[0]).strip()
+ except OSError as err:
+ if err.errno == errno.ENOENT:
+ return None
+ raise
if not data:
return None🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tuned/plugins/plugin_amd_x3d.py` around lines 105 - 106, The read of the
sysfs node via self._cmd.read_file(paths[0]) can raise OSError(ENOENT) if the
device disappears; wrap the call in a try/except that catches OSError and treats
ENOENT as a benign missing-device case by setting data = "" (or
returning/continuing the no-op behavior), while re-raising other OS errors; use
errno.ENOENT (or e.errno) to detect the specific error and update the logic
around the data variable (the block that currently does data =
self._cmd.read_file(paths[0]).strip() and the subsequent if not data:) so
disappearance becomes a no-op instead of an exception.
Add a new amd_x3d plugin that controls the amd_x3d_vcache kernel driver's scheduling preference on dual-CCD 3D V-Cache processors.
The plugin manages the sysfs knob exposed at:
/sys/bus/platform/drivers/amd_x3d_vcache/*/amd_x3d_mode
and supports the two kernel-defined modes:
The sysfs path is discovered via glob so the plugin works across boards with different ACPI device names. On unsupported systems, or when the driver is not present, the plugin is a no-op.
State is integrated with TuneD's standard command_set/command_get flow, so the previous mode is saved and restored automatically on profile switch. Add unit tests covering discovery, apply/verify/rollback, and the no-device case.
Summary by CodeRabbit
New Features
Tests