Skip to content

[staging CI] unslothai/unsloth#10564 - #685

Closed
danielhanchen wants to merge 9 commits into
mainfrom
pr-10564-xplat-ci
Closed

danielhanchen wants to merge 9 commits into
mainfrom
pr-10564-xplat-ci

Conversation

@danielhanchen

Copy link
Copy Markdown
Collaborator

Disposable CI run for unslothai#10564. Do not merge; closed after CI.

oobabooga and others added 9 commits September 8, 2026 20:34
Follow-ups from reviewing the device-registry discovery, all checked against
windows_get_device_registry_files in the Vulkan loader's loader_windows.c.

Skip devnodes pending reboot. An Adrenalin update writes VulkanDriverName and
drops its manifest before the restart that binds the driver. In between, the
adapter is present, the file is on disk, and the loader skips the devnode
(DN_HAS_PROBLEM with CM_PROB_NEED_RESTART or DN_NEED_RESTART). Counting it
routed a gfx115x host to the Vulkan bundle for a driver that cannot load yet,
which is the silent CPU fallback _amd_vulkan_icd_present exists to prevent.
CM_Get_DevNode_Status now gates each device, and an unreadable status is skipped
for the same reason the loader skips it.

Retry the device-id list on CR_BUFFER_SMALL. A device arriving between the size
query and the read makes the block outgrow the buffer, and the API refuses
rather than truncating. That transient reported presence as unknown, both
classes were skipped, and the host this feature exists for quietly installed
the HIP bundle. The loader loops on the same condition; this bounds the loop at
four attempts so a churning list answers unknown instead of spinning.

Scope the registry catch to one instance rather than the whole class. A single
unreadable entry discarded every remaining instance in that class, and on a
two-adapter host the integrated part often enumerates first, so the AMD
registration was the one lost.

Tests: pending-reboot and unrelated problem codes, an unreadable status, a
device arriving mid-enumeration, a list that never settles, and one unreadable
instance not costing the rest of the class.

Also cover what the existing suite could not reach. _FakeCfgMgr answers through
native ctypes, where sizeof(c_wchar) is 4 on Linux and 2 on Windows, so the
byte-to-element conversion for CM_Get_DevNode_Registry_PropertyW was only ever
exercised at the runner's width; a ctypes shim now runs the probe at Windows'
widths, and reverting the conversion fails it. Registration paths are checked
through ntpath, since tmp_path made every existing case posixpath, where
"C:\..." is not absolute. The loader-override test now asserts that neither the
registry nor cfgmgr32 was touched. The restricted-subkey case gained a
digit-named instance, without which the PermissionError arm was unreachable.
_CR_FAILURE was 0x0D, which is CR_NO_SUCH_DEVNODE; cfg.h puts CR_FAILURE at
0x13.
…oblem code

cfg.h lists DN_NEED_RESTART (DN_LIAR, 0x100) under "Device Instance status flags,
returned by call to CM_Get_DevInst_Status", so it is a bit in pulStatus. The check
compared it against pulProblemNumber, where CM_PROB_ codes stop at 0x39, so that
half of the comparison could never match. A devnode reporting the status bit
without also raising DN_HAS_PROBLEM took the earlier return and was counted, which
is the pending-reboot window the check exists to exclude: the manifest is
registered and on disk, the driver is not bound until the restart, and automatic
selection would move a gfx115x host to Vulkan and leave inference on CPU.

Test the bit in the status word, before the problem word and independently of
DN_HAS_PROBLEM, and reserve the problem comparison for CM_PROB_NEED_RESTART.

The parametrized regression test carried the same mistake, passing 0x100 as a
problem code; it now covers the status bit alone and the status bit beside an
unrelated problem, and both fail against the previous check.
@shimmyshimmer
shimmyshimmer deleted the pr-10564-xplat-ci branch September 30, 2026 11:09
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