Skip to content

Add experimental feature to use hard affinities when using best-effort (CP-54234) #6491

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 7 commits into from
Jun 2, 2025

Conversation

psafont
Copy link
Member

@psafont psafont commented May 29, 2025

This needed:

  • Changes in the C bindings to be able to set the hard affinity for each vcpu. This needed change in the basic call to be able to set hard and soft affinities, at the same time.
  • Stop feeding xenguest the hard affinities when user did not set any in particular. Prior to the all->all priorities were set instead, with the misfortune of setting them after the NUMA code had decided the affinities and set them, overriding the desired values. Fortunately xenguest is able to cope with the missing values. (edwin found this strange behaviour)

I took time to fix the exhaustivity issue when defining features, and now xenopsd sets the hard affinities when the user has hardcoded them for a VM, instead of sending them for xenguest to read them.

Tests:

  • manually tested that soft-pinning works by default
  • manually tested that enabling the feature and restart xapi enables hard-pinning
  • manually tested that setting the vcpu mask works (xe vm-param-set uuid=A_UUID VCPUs-params:mask=20)
  • manually checked that the all_features contains the same features as before using the REPL, in case some got forgotten (there are also unit-tests for the module)

The vcpu affinities were checked using xl vcpu-list and restarting a VM repeatedly while changing the feature and the vm arguments.

@psafont psafont force-pushed the private/paus/numa-hard-feature branch from 2e5eed4 to dd5f3f9 Compare May 29, 2025 16:31
int retval;
xc_interface* xch = xch_of_val(xch_val);
uint32_t domid = Int_val(domid_val);
int vcpu = Long_val(vcpu_val);
Copy link
Contributor

Choose a reason for hiding this comment

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

Long_val returns a long int. Or might want to use Int64_val here and Int32_val above for consistency?

Copy link
Member Author

Choose a reason for hiding this comment

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

The names of these conversion functions are very confusing, because they OCaml type they convert from is implicit. On top of that: Int_val truncates values to 32-bits on x86_64, see ocaml/ocaml#13852

Int64_val and Int32_val can't be used here because they don't access unboxed integers, they access blocks / boxed integers, i.e. Int64.t and Int32.t

@last-genius
Copy link
Contributor

Also worth dropping bits of the xenguest.patch in xen.spec relating to affinity handling?

@psafont
Copy link
Member Author

psafont commented May 30, 2025

Also worth dropping bits of the xenguest.patch in xen.spec relating to affinity handling?

In the long term, yes. But I'd rather do that after there are several xapi versions available that don't depend on it.

@psafont psafont force-pushed the private/paus/numa-hard-feature branch 3 times, most recently from f7d1e84 to c0ff16f Compare May 30, 2025 13:53
@psafont
Copy link
Member Author

psafont commented May 30, 2025

I've manually retested the new C code, both types of pinning still work as expected.

@psafont psafont added this pull request to the merge queue Jun 2, 2025
@psafont psafont removed this pull request from the merge queue due to a manual request Jun 2, 2025
@last-genius
Copy link
Contributor

The fixups look good to me

When adding a feature, developers had to change the variant, and the list
all_features. Now the list is autogenerated from the variant, and the
compiler will complain if its properties are not defined. Also reduced
complexity of the code in the rest of the module.

Signed-off-by: Pau Ruiz Safont <[email protected]>
@psafont psafont force-pushed the private/paus/numa-hard-feature branch from c0ff16f to 63344c1 Compare June 2, 2025 07:50
@last-genius
Copy link
Contributor

This commit message is no longer true - "The new common function allows users to set both at the same time, but this is
unused."

psafont added 6 commits June 2, 2025 08:56
I tried sharing more code between hard and soft affinities, but the memory
management of the two cpumaps blows up the number of branches that need to be
taken care of, making it more worthwhile to duplicate a bit of code instead.

Signed-off-by: Pau Ruiz Safont <[email protected]>
No functional change. This prepares pre_build in the domain module to be able
to set the hard affinity mask, without communicating the mask to xenguest
through xenstore.

Signed-off-by: Pau Ruiz Safont <[email protected]>
When all the vcpus can run on all pcpus, there's no need to do any call to set
the hard affinities, so omit this step.

Because xenguest does the calls to set the affinities of the vcpus after the
NUMA code can set the affinities, this frees up the latter to also set the hard
affinities, not just the soft ones

Signed-off-by: Pau Ruiz Safont <[email protected]>
previously they were set up in a roundabout way, writing to xenstore so
xenguest can read them and apply them. This reduces the churn needed to
communicate this information and instead is done close to where the NUMA
decisions are done.

Signed-off-by: Pau Ruiz Safont <[email protected]>
…P-54234)

This allows xapi to "hard-pin" instead of "soft-pin". This is useful to
restrict the scheduler to a single numa node, instead of letting it to move the
vpus across nodes. This must be used with care because the effect of
overprovisioning CPUs in this mode is unknown and will probalby have undesired
effects.

Signed-off-by: Pau Ruiz Safont <[email protected]>
…P-54234)

This allows users to enable the feature on a host by running:
    echo 1 > /etc/xenserver/features.d/hard_numa
    xe host-apply-edition edition=${CURRENT_EDITION} host-uuid=${HOST_UUID}
where CURRENT_EDITION is
    xe host-param-get param-name=edition uuid=${HOST_UUID}

set best-effort mode if it wasn't already set:
    xe host-param-set uuid=${HOST-UUID} numa-affinity-policy=best-effort

and finally restart the toolstack:
    xe-toolstack-restart

Signed-off-by: Pau Ruiz Safont <[email protected]>
@psafont psafont force-pushed the private/paus/numa-hard-feature branch from 63344c1 to 18c952f Compare June 2, 2025 08:00
@psafont psafont enabled auto-merge June 2, 2025 08:00
@psafont psafont added this pull request to the merge queue Jun 2, 2025
Merged via the queue into xapi-project:master with commit ae2d859 Jun 2, 2025
17 checks passed
@psafont psafont deleted the private/paus/numa-hard-feature branch June 2, 2025 10:21
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.

3 participants