Skip to content

CA-408339: Respect xenopsd's NUMA-placement-policy default #6368

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

Merged
merged 2 commits into from
Mar 19, 2025
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion ocaml/xapi-idl/xen/xenops_interface.ml
Original file line number Diff line number Diff line change
Expand Up @@ -501,6 +501,8 @@ module Host = struct
(** best effort placement on the smallest number of NUMA nodes where possible *)
[@@deriving rpcty]

type numa_affinity_policy_opt = numa_affinity_policy option [@@deriving rpcty]

type guest_agent_feature_list = guest_agent_feature list [@@deriving rpcty]
end

Expand Down Expand Up @@ -657,7 +659,7 @@ module XenopsAPI (R : RPC) = struct
let numa_affinity_policy_p =
Param.mk
~description:["Host NUMA affinity policy"]
~name:"numa_affinity_policy" Host.numa_affinity_policy
~name:"numa_affinity_policy" Host.numa_affinity_policy_opt
in
declare "HOST.set_numa_affinity_policy"
["Sets the host's NUMA aware VM scheduling policy"]
Expand Down
6 changes: 3 additions & 3 deletions ocaml/xapi/xapi_xenops.ml
Original file line number Diff line number Diff line change
Expand Up @@ -3098,11 +3098,11 @@ let set_numa_affinity_policy ~__context ~value =
let open Xenops_interface.Host in
match value with
| `any ->
Any
Some Any
| `best_effort ->
Best_effort
Some Best_effort
| `default_policy ->
Any
None
in
Client.HOST.set_numa_affinity_policy dbg value

Expand Down
15 changes: 14 additions & 1 deletion ocaml/xenopsd/lib/xenops_server.ml
Original file line number Diff line number Diff line change
Expand Up @@ -3398,8 +3398,13 @@ module VIF = struct
()
end

let default_numa_affinity_policy = ref Xenops_interface.Host.Any

let numa_placement = ref Xenops_interface.Host.Any

let string_of_numa_affinity_policy =
Xenops_interface.Host.(function Any -> "any" | Best_effort -> "best-effort")

module HOST = struct
let stat _ dbg =
Debug.with_thread_associated dbg
Expand All @@ -3413,7 +3418,15 @@ module HOST = struct
let set_numa_affinity_policy _ dbg =
Debug.with_thread_associated dbg @@ fun policy ->
debug "HOST.set_numa_affinity_policy" ;
numa_placement := policy
match policy with
| None ->
info "Enforcing default NUMA affinity policy (%s)"
(string_of_numa_affinity_policy !default_numa_affinity_policy) ;
numa_placement := !default_numa_affinity_policy
| Some p ->
info "Enforcing '%s' NUMA affinity policy"
(string_of_numa_affinity_policy p) ;
numa_placement := p

let get_console_data _ dbg =
Debug.with_thread_associated dbg
Expand Down
5 changes: 4 additions & 1 deletion ocaml/xenopsd/xc/xenops_server_xen.ml
Original file line number Diff line number Diff line change
Expand Up @@ -5148,8 +5148,11 @@ let init () =
{Xs_protocol.ACL.owner= 0; other= Xs_protocol.ACL.READ; acl= []}
) ;
Device.Backend.init () ;
Xenops_server.numa_placement :=
Xenops_server.default_numa_affinity_policy :=
if !Xenopsd.numa_placement_compat then Best_effort else Any ;
info "Default NUMA affinity policy is '%s'"
Xenops_server.(string_of_numa_affinity_policy !default_numa_affinity_policy) ;
Xenops_server.numa_placement := !Xenops_server.default_numa_affinity_policy ;
Copy link
Contributor

Choose a reason for hiding this comment

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

I would suggest wrap these lines in a function like Xenops_server.init_numa_affinity_policy !Xenopsd.numa_placement_compat. Then the references in Xenops_server will not need to be exposed.

Copy link
Member Author

Choose a reason for hiding this comment

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

Unfortunately, Xenops_server.numa_placement is actually used outside Xenops_server, so we can hide it.

Domain.numa_init () ;
debug "xenstore is responding to requests" ;
let () = Watcher.create_watcher_thread () in
Expand Down
Loading