Skip to content

draft: hardware partition and XCP capacity - #17

Draft
fishman wants to merge 12 commits into
Project-HAMi:mainfrom
fishman:partitioning
Draft

draft: hardware partition and XCP capacity#17
fishman wants to merge 12 commits into
Project-HAMi:mainfrom
fishman:partitioning

Conversation

@fishman

@fishman fishman commented Aug 18, 2026

Copy link
Copy Markdown
  • testdata: add MI355X node fixtures for hardware-free regression tests
  • amdgpu: configurable sysfs root, amdsmi memory-partition and profile lookups
  • plugin: injectable AMD SMI seams, fixture-backed registration test
  • README: document partition profiles and mode-change semantics
  • allocator, utils, labeller: nil-guard stale device ids, dual allocation keys
  • plugin, amdgpu: advertise per-XCP capacity as an even share of the GPU
  • testdata, README: xgmi/topology fixtures, per-XCP capacity, memory-partition rationale

@coderabbitai

coderabbitai Bot commented Aug 18, 2026

Copy link
Copy Markdown

Important

Review skipped

Draft detected.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 4e0d8a1a-4cf7-4825-9acb-f53a61987898

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@hami-robot

hami-robot Bot commented Aug 18, 2026

Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: fishman

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

Reza Jelveh added 7 commits August 19, 2026 11:19
Captured from nodepool-gpu-7b3016af2911 (kernel 6.8.0-136): KFD topology,
amdgpu_xcp devices, per-GPU partition state, and amd-smi static, partition,
memory-partition and per-GPU partition JSON. SPI captures document the
zero-registration behavior on this kernel (XCP render minors absent from
KFD topology).

Signed-off-by: Reza Jelveh <fishmangit@dynamia.ai>
…lookups

GetAMDGPUs takes a sysfs root (defaults to /sys) so discovery can run
against captured fixtures. GetAMDSCurrentMemoryPartitions and
GetAMDGPUPartitionProfiles extend the cgo AMD SMI layer: the first resolves
the current NPS mode per BDF, the second the accelerator partition profiles
(SPX/DPX/QPX/CPX with partition geometry) from
amdsmi_get_gpu_accelerator_partition_profile_config.

Signed-off-by: Reza Jelveh <fishmangit@dynamia.ai>
WithSysfsRoot, WithAmdSMI and WithAMDSPartitionProfiles constructor options
let registration run without hardware or AMD SMI; nil means the real /sys
and cgo implementations. Registration overrides the sysfs memory partition
with the amdsmi value and advertises available partition profiles per
whole GPU in customInfo. TestRegistrationFromFixture drives the full flow
against the captured MI355X tree and pins the kernel 6.8 zero-registration
behavior; TestPartitionProfilesFromFixture pins the fixture profile truth.

Signed-off-by: Reza Jelveh <fishmangit@dynamia.ai>
Explains the per-GPU profile table (SPX/DPX/QPX/CPX with partition x XCC
geometry), profile_index semantics, the idle requirement for partition
changes, and the per-GPU compute vs node-wide memory reload distinction.

Signed-off-by: Reza Jelveh <fishmangit@dynamia.ai>
…on keys

Best-effort policy skips stale kubelet device ids across plugin restarts
instead of panicking in sort.Slice. Allocation reads the committed
hami.io/amd-devices-allocated payload and falls back to the fork's own
to-allocate annotation. Labeller Dockerfile picks up the glibc patch.

Signed-off-by: Reza Jelveh <fishmangit@dynamia.ai>
XCP partitions share their parent's PCI BDF. Their registered capacity is
now the whole-GPU VRAM and CU count divided by the number of XCP partitions,
computed once at start from the libdrm whole-GPU read. Floor division
under-advertises rather than over-committing.

Signed-off-by: Reza Jelveh <fishmangit@dynamia.ai>
…rtition rationale

Store amd-smi xgmi and topology node captures next to the existing
fixtures. Document the profile storage model (register annotation, mirroring
HAMi's MIG storage), the per-XCP capacity derivation, and why NPS memory
partitions are not an allocation mechanism for HAMi.

Signed-off-by: Reza Jelveh <fishmangit@dynamia.ai>
Reza Jelveh added 2 commits August 19, 2026 11:23
Generate a per-GPU catalog of valid compute partition x NPS combinations
with the NPS->physical memory count table at plugin Start() (gfx950:
spx/dpx/qpx x nps1/4/8). Held in memory only, never written to disk
(immutable-OS friendly); complementable later by HAMi default templates.

Signed-off-by: Reza Jelveh <fishmangit@dynamia.ai>
MI355X (gfx950) admission fixes:

- isSchedulableTopologyKey: spx/dpx/qpx whole GPUs have no amdgpu_xcp_
  children in KFD topology and keep their own DRI card, so they register.
  Only XCP parents (BDF with amdgpu_xcp_ children) are skipped. Previously
  all 8 GPUs were skipped, leaving zero registered devices, an empty
  sortedBDFs, and Allocate unable to resolve upstream AMDGPU-<i> ids.
- GetPreferredAllocation: kubelet passes checkpoint-persisted ids (bare
  BDFs from the pre-fork plugin) that the split-keyed allocator cannot
  map. GPA is advisory and Allocate resolves the pod annotation, so echo
  kubelet's own ids instead of failing admission.

Signed-off-by: Reza Jelveh <fishmangit@dynamia.ai>
Reza Jelveh added 3 commits August 19, 2026 15:03
…ition flip

Per-node operating mode (NVIDIA mig.config-style), defaulting to cu:

- dp.operatingMode chart value / OPERATING_MODE env, overridable per node
  via the hami.io/amd-operating-mode annotation: "cu" registers soft
  whole-GPU devices (Count 10, unchanged behavior); "partition" registers
  hard compute partitions (id#mode, Count 1) and publishes only hard
  entries to kubelet.
- dp.computePartition / COMPUTE_PARTITION env, overridable via
  hami.io/amd-compute-partition: in partition mode the plugin flips every
  GPU to the target profile through amdsmi_set_gpu_accelerator_partition_profile
  at startup; no manual amd-smi on the node. Busy GPUs fail (status 30)
  and keep their current mode.
- XCP capacity divisor now uses the AMD SMI profile NumPartitions of the
  current compute type instead of amdgpu_xcp_ child counts (7 XCD
  chiplets per GPU regardless of partition mode).
- Docs: configuration.md operating-mode section, new
  mi355x-partitioning.md (kernel caveats, verification), QPX example at
  example/vllm-qpx-serve/ (deployment with gpumem/gpucores requests, HAMi
  device-config amd section, README).

Signed-off-by: Reza Jelveh <fishmangit@dynamia.ai>
A busy GPU refuses the compute-partition flip (AMD SMI status 30) and
stays in its old mode, leaving the node heterogeneous. Previously
getResourceList aborted the plugin before Start() ran, so the flip
never executed and the node crash-looped forever.

- main.go: heterogeneous node with single strategy logs a warning and
  reports all devices under "gpu" instead of exiting
- plugin.go: ListAndWatch publishes every device when the resource
  name is not a per-partition-type key
- plugin.go: log flip duration at glog V(2) (measured 435ms for 7 GPUs)

Signed-off-by: Reza Jelveh <fishmangit@dynamia.ai>
Skipping unknown ids prevents the panic, but a silent fix hides the
cause: another device plugin running on the same node merged its
devices into kubelet's list. Log a warning naming the likely cause
and the remedy (disable the other plugin, restart this one).

Signed-off-by: Reza Jelveh <fishmangit@dynamia.ai>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant