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

Fix syscall numbers for non-x86_64 architectures #41

Merged
merged 2 commits into from
Sep 13, 2024

Conversation

topolarity
Copy link
Member

This gets LinuxPerf.jl working for me on aarch64-linux:

julia> @pstats nothing
┌ Warning: LinuxPerf.EventTypeExt(hw:branches, false, 0x0000000000000006) not supported, skipping
└ @ LinuxPerf ~/.julia/dev/LinuxPerf/src/LinuxPerf.jl:299
━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
┌ cpu-cycles               2.50e+02  100.0%  #  0.0 cycles per ns
│ stalled-cycles-frontend  2.18e+02  100.0%  # 87.2% of cycles
└ stalled-cycles-backend   2.30e+01  100.0%  #  9.2% of cycles
┌ instructions             1.80e+01  100.0%  #  0.1 insns per cycle
└ branch-misses            2.00e+00  100.0%
┌ task-clock               1.64e+05  100.0%  # 164.5 μs
│ context-switches         0.00e+00  100.0%
│ cpu-migrations           0.00e+00  100.0%
└ page-faults              0.00e+00  100.0%
━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━

Fixes #39

(note: the warning is due to a bug in that hasn't been fixed yet in my kernel:
https://patchwork.kernel.org/project/linux-arm-kernel/patch/20230403091547.441550-1-peternewman@google.com/)

This gets LinuxPerf.jl working for me on `aarch64-linux`:

```
julia> @pstats nothing
┌ Warning: LinuxPerf.EventTypeExt(hw:branches, false, 0x0000000000000006) not supported, skipping
└ @ LinuxPerf ~/.julia/dev/LinuxPerf/src/LinuxPerf.jl:299
━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
┌ cpu-cycles               2.50e+02  100.0%  #  0.0 cycles per ns
│ stalled-cycles-frontend  2.18e+02  100.0%  # 87.2% of cycles
└ stalled-cycles-backend   2.30e+01  100.0%  #  9.2% of cycles
┌ instructions             1.80e+01  100.0%  #  0.1 insns per cycle
└ branch-misses            2.00e+00  100.0%
┌ task-clock               1.64e+05  100.0%  # 164.5 μs
│ context-switches         0.00e+00  100.0%
│ cpu-migrations           0.00e+00  100.0%
└ page-faults              0.00e+00  100.0%
━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
```

The warning is due to a bug in that hasn't been fixed yet in my kernel:
https://patchwork.kernel.org/project/linux-arm-kernel/patch/20230403091547.441550-1-peternewman@google.com/
@vchuravy vchuravy merged commit e19e1f2 into JuliaPerf:master Sep 13, 2024
1 check passed
res = ccall(:syscall, Cint, (Clong, Clong...), SYS_prctl, PR_TASK_PERF_EVENTS_ENABLE)
Base.systemerror(:prctl, res < 0)
end
function disable_all!()
SYS_prctl == -1 && error("prctl error : unknown architecture")
Copy link
Collaborator

@Zentrik Zentrik Sep 13, 2024

Choose a reason for hiding this comment

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

Do you know how much overhead this adds? Does it get compiled out, if so can we just ccall prctl in this branch.

Copy link
Member Author

@topolarity topolarity Sep 13, 2024

Choose a reason for hiding this comment

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

It gets compiled out 👍

julia> @code_typed LinuxPerf.disable_all!()
CodeInfo(
1nothing::Nothing%2 = $(Expr(:foreigncall, :(:syscall), Int32, svec(Int64, Int64), 1, :(:ccall), 167, 31, 31, 167))::Int32%3 = Core.sext_int(Core.Int64, %2)::Int64%4 = Base.slt_int(%3, 0)::Bool
└──      goto #3 if not %4
2 ─      invoke Core.kwcall($(QuoteNode((extrainfo = nothing,)))::@NamedTuple{extrainfo::Nothing}, Base.systemerror::typeof(systemerror), :prctl::Symbol)::Union{}
└──      unreachable
3 ─      goto #4
4 ─      goto #5
5return nothing
) => Nothing

You can see that it goes straight into the :foreigncall there on the second line, which is the ccall

Copy link
Collaborator

Choose a reason for hiding this comment

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

Seems sensible to then just ccall prctl instead of erroring.

Copy link
Member Author

Choose a reason for hiding this comment

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

Maybe.. I'd prefer the error so that an issue gets raised against the repo and then this gets fixed

Copy link
Collaborator

@Zentrik Zentrik Sep 13, 2024

Choose a reason for hiding this comment

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

I mean we do

if Sys_prctl == -1
    ccall(:prctl, ...)
else
    ccall(:syscall,...)

Unless some machines don't have prctl as a syscall, this should always work right?

@topolarity
Copy link
Member Author

@vchuravy It'd be nice to get a minor release with this + #38 + #42, when time permits

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.

Does this only support x86_64
3 participants