| Message ID | cover.1715954111.git.alejandro.vallejo@cloud.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | Clean the policy manipulation path in domain creation | expand |
On Fri, May 17, 2024 at 05:08:33PM +0100, Alejandro Vallejo wrote: > v2: > * Removed xc_cpu_policy from xenguest.h > * Added accessors for xc_cpu_policy so the serialised form can be extracted. > * Modified xen-cpuid to use accessors. > > ==== Original cover letter ==== > > In the context of creating a domain, we currently issue a lot of hypercalls > redundantly while populating its CPU policy; likely a side effect of > organic growth more than anything else. > > However, the worst part is not the overhead (this is a glacially cold > path), but the insane amounts of boilerplate that make it really hard to > pick apart what's going on. One major contributor to this situation is the > fact that what's effectively "setup" and "teardown" phases in policy > manipulation are not factored out from the functions that perform said > manipulations, leading to the same getters and setter being invoked many > times, when once each would do. > > Another big contributor is the code being unaware of when a policy is > serialised and when it's not. > > This patch attempts to alleviate this situation, yielding over 200 LoC > reduction. > > Patch 1: Mechanical change. Makes xc_cpu_policy_t public so it's usable > from clients of libxc/libxg. > Patch 2: Changes the (de)serialization wrappers in xenguest so they always > serialise to/from the internal buffers of xc_cpu_policy_t. The > struct is suitably expanded to hold extra information required. > Patch 3: Performs the refactor of the policy manipulation code so that it > follows a strict: PULL_POLICIES, MUTATE_POLICY (n times), PUSH_POLICY. I think patch 3 is no longer part of the set? I don't see anything in the review of v1 that suggests patch 3 was not going to be part of the next submission? Thanks, Roger.
On 20/05/2024 13:45, Roger Pau Monné wrote: > On Fri, May 17, 2024 at 05:08:33PM +0100, Alejandro Vallejo wrote: >> v2: >> * Removed xc_cpu_policy from xenguest.h >> * Added accessors for xc_cpu_policy so the serialised form can be extracted. >> * Modified xen-cpuid to use accessors. >> >> ==== Original cover letter ==== >> >> In the context of creating a domain, we currently issue a lot of hypercalls >> redundantly while populating its CPU policy; likely a side effect of >> organic growth more than anything else. >> >> However, the worst part is not the overhead (this is a glacially cold >> path), but the insane amounts of boilerplate that make it really hard to >> pick apart what's going on. One major contributor to this situation is the >> fact that what's effectively "setup" and "teardown" phases in policy >> manipulation are not factored out from the functions that perform said >> manipulations, leading to the same getters and setter being invoked many >> times, when once each would do. >> >> Another big contributor is the code being unaware of when a policy is >> serialised and when it's not. >> >> This patch attempts to alleviate this situation, yielding over 200 LoC >> reduction. >> >> Patch 1: Mechanical change. Makes xc_cpu_policy_t public so it's usable >> from clients of libxc/libxg. >> Patch 2: Changes the (de)serialization wrappers in xenguest so they always >> serialise to/from the internal buffers of xc_cpu_policy_t. The >> struct is suitably expanded to hold extra information required. >> Patch 3: Performs the refactor of the policy manipulation code so that it >> follows a strict: PULL_POLICIES, MUTATE_POLICY (n times), PUSH_POLICY. > > I think patch 3 is no longer part of the set? I don't see anything > in the review of v1 that suggests patch 3 was not going to be part of > the next submission? > > Thanks, Roger. It's there, there was just a shift. The implication of the first line of the changelog is that v1/patch1 was dropped. Sorry, should've been clearer. v1/patch1 => dropped v1/patch2 => v2/patch1 v1/patch3 => v2/patch2 Cheers, Alejandro
v2: * Removed xc_cpu_policy from xenguest.h * Added accessors for xc_cpu_policy so the serialised form can be extracted. * Modified xen-cpuid to use accessors. ==== Original cover letter ==== In the context of creating a domain, we currently issue a lot of hypercalls redundantly while populating its CPU policy; likely a side effect of organic growth more than anything else. However, the worst part is not the overhead (this is a glacially cold path), but the insane amounts of boilerplate that make it really hard to pick apart what's going on. One major contributor to this situation is the fact that what's effectively "setup" and "teardown" phases in policy manipulation are not factored out from the functions that perform said manipulations, leading to the same getters and setter being invoked many times, when once each would do. Another big contributor is the code being unaware of when a policy is serialised and when it's not. This patch attempts to alleviate this situation, yielding over 200 LoC reduction. Patch 1: Mechanical change. Makes xc_cpu_policy_t public so it's usable from clients of libxc/libxg. Patch 2: Changes the (de)serialization wrappers in xenguest so they always serialise to/from the internal buffers of xc_cpu_policy_t. The struct is suitably expanded to hold extra information required. Patch 3: Performs the refactor of the policy manipulation code so that it follows a strict: PULL_POLICIES, MUTATE_POLICY (n times), PUSH_POLICY. Alejandro Vallejo (2): tools/xg: Streamline cpu policy serialise/deserialise calls tools/xg: Clean up xend-style overrides for CPU policies tools/include/xenguest.h | 8 +- tools/libs/guest/xg_cpuid_x86.c | 537 ++++++++++------------------ tools/libs/guest/xg_private.h | 2 + tools/libs/guest/xg_sr_common_x86.c | 54 +-- tools/misc/xen-cpuid.c | 43 +-- 5 files changed, 231 insertions(+), 413 deletions(-)