Skip to content

scx_utils: Fix cargo warning#1486

Merged
hodgesds merged 1 commit intosched-ext:mainfrom
hodgesds:utils-fix
Mar 10, 2025
Merged

scx_utils: Fix cargo warning#1486
hodgesds merged 1 commit intosched-ext:mainfrom
hodgesds:utils-fix

Conversation

@hodgesds
Copy link
Contributor

Fix cargo linter warning for a variable being unnecessarily mutable.

Fix cargo linter warning for a variable being unnecessarily mutable.

Signed-off-by: Daniel Hodges <hodgesd@meta.com>
@hodgesds hodgesds enabled auto-merge March 10, 2025 17:41
@hodgesds hodgesds added this pull request to the merge queue Mar 10, 2025
Merged via the queue into sched-ext:main with commit 60816bd Mar 10, 2025
28 checks passed
@hodgesds hodgesds deleted the utils-fix branch March 10, 2025 18:03
@JohnRTitor
Copy link
Contributor

Hi, this once again broke scx_lavd, causing the same problem as #1482. This was fixed in #1483 by @multics69, by querying whether the file exists and trying the next possible file path. Removing the mut once again broke it.

CC @JakeHillion

@hodgesds
Copy link
Contributor Author

I guess it needs a more proper fix like querying the tracefs mount. Maybe I'll move this helper into scx_utils.

@JakeHillion
Copy link
Contributor

Hi, this once again broke scx_lavd, causing the same problem as #1482. This was fixed in #1483 by @multics69, by querying whether the file exists and trying the next possible file path. Removing the mut once again broke it.

CC @JakeHillion

Hey, this specific change didn't break Nix. It's a no-op: Rust doesn't need this variable to be mutable because the match statement selects either the initial result or the second and places that into the variable. This means that the variable is not mutated after its initial assignment.

For future reference, removing mut with code that still compiles in Rust will always be safe.

@JohnRTitor

@JohnRTitor
Copy link
Contributor

Right, I checked it myself, and it's still working, tested 8e33fa9 commit.

I got a bug report from a user (@s0me1newithhand7s) on Telegram that lavd was broken, along with a cryptic error message "Error: No such file or directory (os error 2)
".

image

https://gist.github.com/s0me1newithhand7s/fd3cf2b5d2c2262468972fb02129195d

I am not sure what's going on here, as I can't reproduce it. But hopefully @s0me1newithhand7s can add more context.

@s0me1newithhand7s
Copy link

I am not sure what's going on here, as I can't reproduce it. But hopefully @s0me1newithhand7s can add more context.

firstly:

 ~ 𝑠𝑛𝑜𝑤𝑦 𝑝𝑙𝑎𝑐𝑒, 𝑓𝑢𝑙𝑙 𝑜𝑓 𝑓𝑙𝑎𝑘𝑒𝑠! 

╭──╼ s0mePC-nix on NixOS 25.5.0 ❄️
┆ ~
╰─> uname -a                                                                                                                     ✓  at ❗ 14:45
Linux s0mePC-nix 6.13.6-cachyos #1-NixOS SMP PREEMPT_DYNAMIC Fri Mar  7 17:27:16 UTC 2025 x86_64 GNU/Linux

so:

  1. NixOS Unstable
  2. cachyos-linux kernel

brief run of all other scheds resulted with no errors;
scx_lavd runs trough scxctl still ends with same Error: No such file or directory (os error 2);
reverting versions won't help; i've tried, scx_lavd behave same way;

i think this is some boot.kernelParam or/and boot.sysctl that could cause this weird scx_lavd behavior.
attaching my current sysctls and kernelParams:
image
image

i can't really find any reasons why scx_lavd doesn't works while others do..

@JohnRTitor
Copy link
Contributor

Right, you passed debugfs=off as a kernel parameter. scx_lavd needs either tracefs or debugfs. NixOS have always had debugfs support, but tracefs by default was only enabled recently. That feature hasn't yet reached unstable channels yet. NixOS/nixpkgs#388751

It should reach nixos-unstable in coming days. For now, you can reenable debugfs to use lavd properly.

@s0me1newithhand7s
Copy link

It should reach nixos-unstable in coming days. For now, you can reenable debugfs to use lavd properly.

oh i see, tysm!

@multics69
Copy link
Contributor

@JohnRTitor -- FYI... The new refactoring on the previous fix creates a bug. And that bug was fixed a minute ago (#1506).

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.

6 participants