Repository navigation
Conversation
Code that polls until something succeeds or a deadline passes writes its own loop: call, check the deadline, sleep, maybe double the sleep, call again. xapi's Helpers.retry_until_timeout lives in xapi, adds up its sleeps instead of reading a clock and raises an internal error; Throttle.Batching backs off with spans but has no deadline, and its library, xapi-aux, links this one. Add one bounded retry function that any component can use, with the deadline left to the caller. Threadext.retry ?delay ?max_delay ~stop f calls f until it returns Ok, or until stop () is true after an Error; it then returns the error of the call before, if any, as the deadline may have cut the last call short. Between two calls it sleeps delay (default 50 ms), doubled after each call up to max_delay (default delay, a fixed sleep); stop is usually a monotonic timer. Nothing calls it yet, so there is no change in behaviour: its first users are the waits of the host GPU lock, added by the next commits. The library now links clock, on which its package already depends. Signed-off-by: Marcus Granado <marcus.granado@citrix.com>
OCaml's Unix offers lockf, whose POSIX locks belong to the process: a second open of the file in the same process does not conflict with the first, and closing any descriptor of the file releases them. A lock that a forked child keeps while it runs, and that two threads of one process cannot both hold, needs flock(2), which Unix does not bind. Add the two bindings that the file lock of the next commits needs. Unixext.flock_try fd ~exclusive takes a shared or exclusive flock with LOCK_NB and returns false, instead of waiting, when another holder conflicts; any other error raises Unix_error. Unixext.open_tmpfile dir opens a new unnamed file of mode 0600 with O_TMPFILE and O_CLOEXEC, so that no other process can open a lock file before it is complete; linkat(2) names it then. Nothing calls them yet. No change in behaviour: _GNU_SOURCE, which O_TMPFILE needs, changes none of the file's existing calls. Signed-off-by: Marcus Granado <marcus.granado@citrix.com>
Separate programs of a host often need to take turns on one resource:
a PCI device that one program unbinds while another binds it, a state
file that a daemon and a tool both rewrite, a job that must not run
twice. xen-api's OCaml libraries have no lock for this: xenopsd's
bind_lock (device.ml) is a mutex that keeps only its own threads apart,
and nbd_client_manager.py waits on fcntl.flock with no bound and no way
to name the holder. A lock between processes, released when its holder
dies, kept by the children it forks, with waits that end and holders
that can be listed, could serve both; the GPUs below are its first use.
Its two modes are those of the classic readers-writer lock [1].
In the paper's terms, readers take Lock_rw.Shared and writers
Lock_rw.Exclusive on the object a key names. As in the paper's
Problem 1, no reader waits only because a writer is waiting: writers
have no priority. Unlike there, every wait is bounded, so a writer that
readers keep out is refused instead of starving.
Programs of a host change or read the state of its GPUs: xapi enables
SR-IOV virtual functions, xenopsd unbinds a GPU from its driver for
pass-through, and monitoring tools query the driver; nothing makes
them take turns.
Add Lock_rw, a readers-writer lock on named objects that is independent
of the primitive behind it, and Flock, its implementation with flock(2).
Lock_rw takes shared or exclusive holds on a key; the caller's waiter
reads the clock and decides how long each wait lasts. Flock keeps one
file per key in a directory of mode 0700: a taker lstats the file (a
regular file of this user that no group or other user can write),
opens, fstats and flocks it; the hold is attested when the file's mark
names its key, boot, inode and link count.
Also: tests of the taker's checks, of a child that keeps the lock and
of shared holds. Flock gives a waiter no blockers yet: the record and
the `Stuck error are in the waiter's type, so that listing the holders
and finding one stuck build that record, with a new field, and leave
the waiter as it is. Nothing calls Flock yet, nor Shared: a later
change reads the lock's key under it.
[1] P. J. Courtois, F. Heymans and D. L. Parnas, "Concurrent control
with 'readers' and 'writers'", Communications of the ACM 14(10),
667-668, 1971, https://doi.org/10.1145/362759.362813
Signed-off-by: Marcus Granado <marcus.granado@citrix.com>
A Lock_rw waiter decides how long a taker waits for a lock that another holds, but Flock reads no clock, so nothing yet bounds a wait. Add the waits of one lock, each bounded by a monotonic deadline, so that no caller waits forever. Lock_rw_wait.waiter t span stops once a Clock.Timer of span, started when the waiter is made, has expired. While another holds the lock, the wait retries with Threadext.retry, sleeping 10 ms and doubling up to 50 ms, and ends in `Busy at the deadline. A span of zero is the caller's one try: it waits for nothing. No change in behaviour: nothing calls Lock_rw_wait yet. Also: t holds no state yet; the waits of one lock can keep there what they learn about its holders. The module has no test of its own: the tests of the GPU lock, added later in this series, wait through it. Signed-off-by: Marcus Granado <marcus.granado@citrix.com>
This change fixes one danger: two programs both believing they hold the
lock.
Background. A flock lock belongs to a file's inode, the file itself,
not to its name. An earlier change gave programs the taker's side:
open the key's file (for the GPU lock, a later change's
/run/xapi-gpu-lock/host.lock), lock it, check it is the right file.
But two things were missing.
Problem 1: nobody creates the lock file. On the first use after boot,
/run/xapi-gpu-lock/host.lock does not exist yet.
- If each program simply created it when absent, xapi and xenopsd
could create it at the same moment and each lock a different file.
- Or one program could open a half-written file that has no mark yet.
Problem 2: a replaced file breaks the lock silently. For example:
1. xapi locks host.lock (inode 100) and starts sriov-manage -e.
2. Something deletes or renames host.lock, or restores an old copy of
it: a cleanup script, an admin, a backup restore.
3. xenopsd wants to unbind the same device. It opens host.lock, now a
different file (inode 200), and locks it. That succeeds, because
nobody holds inode 200.
4. xapi and xenopsd now both "hold the lock" and change the device at
the same time: the race the lock exists to prevent, and nothing
reports it.
What this change adds:
- fresh, a safe creator. It makes the new file under no name at all
(O_TMPFILE), locks it, writes its key into it, and only then gives it
the name host.lock (linkat). No program can see a half-made file, and
the creator already holds the lock when the name appears.
- attest_if_alone, the check against replacement. Before declaring a
new file to be "the" lock, the creator runs a census of /proc: does
any process still hold or have open an older file of the same key?
- If one does, as in steps 1-3 above, the new file is not attested.
Takers accept only an attested file, so xenopsd waits for xapi's
hold on the old file to end, or is refused when its wait runs out,
instead of silently running alongside xapi.
- If none does, the file gets its mark:
<magic> v=1.0 id=host boot=<boot id> ino=200 links=1 nonce=<random> label=0
- What the mark lets a taker check:
- the right key (id);
- created since this boot, not a stale copy (boot);
- this file and no other (ino), since a restored copy has a different
inode;
- no second name pointing at it (links=1).
label=0 is a field for a later change; an older mark's other fields
are kept, and a mark of another major version is never overwritten.
- holders, for operators. It lists who holds a lock file: process id,
command, process state, shared, exclusive or only open, which file,
and any of its threads waiting in the kernel (state D). It reads only
/proc and the lock directory, stops at the waiter's deadline, and
feeds the refusals, which give the number of holders and name them in
the host's log.
Tests: adoption, another boot, renamed and kept holds, the creator's
own hold, another key's creator, and marks.
Signed-off-by: Marcus Granado <marcus.granado@citrix.com>
xapi enables a card's SR-IOV virtual functions and xenopsd unbinds a card from its driver for pass-through, while GPU tools may be querying the same driver. Each step can disturb the others, and none waits for the others. Add one lock for the GPUs of a host, which these programs take before they change a card's state, built on Lock_rw, Flock and Lock_rw_wait. with_lock t intent ~timeout f runs f holding the exclusive lock of /run/xapi-gpu-lock/host.lock, or unlocked, as today, while the engaged function given to create is false. f receives the descriptors that a forked command keeps to hold the lock, and the commands' time limits: where engaged, sriov-manage gets 4 minutes. A missing or damaged lock file is repaired once; a wait that runs out raises Refused. Also: an engaged host's client sees "the GPU lock stayed busy: unbind 0000:3b:00.0; holders: 1 (see the host's log)"; the log names them in JSON (pid, name, state, mode, file, d_threads), escaped by the logger; an adoption of a lock file logs a warning; a killed sriov-manage says what it may have left. Tests of these rules. Nothing calls the lock, nor transient, yet: a later busy poll reads it. Signed-off-by: Marcus Granado <marcus.granado@citrix.com>
Forkhelpers.execute_command_get_output runs a command with only its standard input, output and error open. Under a lock that a descriptor holds, such as a flock(2) lock, the command then runs without the lock once its caller gives up waiting or dies, while it still runs. Let the caller name descriptors that the command keeps open, so that the command keeps such a lock until it exits. execute_command_get_output gains ?keep_fds (default []). Each descriptor gets a fresh identifier and joins the list that safe_close_and_exec already passes to the child, which forkexecd or vfork keep open in the command. No change in behaviour until the host GPU lock is engaged on the host, which no code in this pull request does: until then the lock's callers pass an empty list, as the default is. Signed-off-by: Marcus Granado <marcus.granado@citrix.com>
xapi enables the SR-IOV virtual functions of an NVIDIA card by running
sriov-manage -e, one card at a time under nvidia_vf_setup_mutex, but
nothing keeps xenopsd's unbinds or GPU tools away from the driver while
sriov-manage runs.
Run sriov-manage under the host GPU lock, so that, once the lock is
engaged, a VF enable excludes the others that change or query the GPUs.
nvidia_vf_setup runs sriov-manage under Gpu_lock.with_lock for the
card's Vf_enable intent, waiting at most 30 s for the lock (about twice
the longest hold measured, a VF enable of about 16 s); sriov-manage
keeps the lock through ~keep_fds until it exits. Once engaged, a run
that outlasts 4 minutes is killed and fails the VF enable, and so the
VM start, with INTERNAL_ERROR and the text:
sriov-manage -e 0000:3b:00.0 did not end within 4min and was killed: the card may be left without its driver, with its driver_override set, or with its VFs half set up, and the GPU monitoring services stopped
No change in behaviour until the host GPU lock is engaged on the host,
which no code in this pull request does: gpu_lock_engaged stays false,
so no lock is taken, no file is made and sriov-manage has no timeout.
Also: xapi links xapi-gpu.lock.
Signed-off-by: Marcus Granado <marcus.granado@citrix.com>
Before passing an NVIDIA card through to a VM, xenopsd disables its
SR-IOV virtual functions with sriov-manage -d, turns persistence mode
off with nvidia-smi, sets the driver's unbind lock and unbinds the card
from the driver. Nothing keeps xapi's VF enables or GPU tools away from
the card meanwhile, and neither command has a timeout.
Run those steps under the host GPU lock, once the lock is engaged, and
bound the two commands there.
unbind_from_nvidia runs them under Gpu_lock.with_lock for the card's
Unbind intent, waiting at most 30 s for the lock; both commands keep
the lock through ~keep_fds. Once engaged, sriov-manage -d is killed
after 4 minutes, failing the VM start with INTERNAL_ERROR, "xenopsd
internal error:" and the first text below; nvidia-smi is killed after
30 s, logging the second, and the nvidia driver's unbind lock then
decides if the unbind can proceed:
sriov-manage -d 0000:3b:00.0 did not end within 4min and was killed: the card may be left without its driver or with its driver_override set, and the GPU monitoring services stopped
pci: nvidia-smi --persistence-mode=0 timed out on 0000:3b:00.0 and was killed; the nvidia driver's unbind lock will now decide if the unbind can proceed
No change in behaviour until the host GPU lock is engaged on the host,
which no code in this pull request does: xenopsd's lock is engaged
while a file gpu-lock-engaged exists in its root directory (by default
under /var/run, cleared at boot), and nothing creates it. Also:
deactivate_nvidia_sriov takes ~keep_fds and ~timeout; xenopsd's xc
library links xapi-gpu.lock.
Signed-off-by: Marcus Granado <marcus.granado@citrix.com>
mg12
force-pushed
the
CP-315158-host-gpu-lock
branch
from
November 6, 2026 15:19
908e53b to
3fa4256
Compare
A holder of the host GPU lock can get stuck: a driver call that hangs leaves one of its threads in an uninterruptible wait in the kernel (state D), which no kill can end, so the holder keeps the lock. A wait for the lock cannot see this today: it learns only that the lock is busy, and waits out its whole timeout, 30 s, before it is refused. Lay the ground for refusing such a holder quickly: find the processes that hold the lock against a wait and, thread by thread (tid), which of their threads are in state D. From the next commit on, a wait refuses a holder whose same thread stays in one kernel wait for 2 s, instead of waiting out its timeout. The same list identifies the holders and tells apart the holds of the caller's own process. A blocker gives the process's pid and start time (unique across pid reuse), whether the caller's process took the hold and, for each of its threads in state D, its tid and voluntary context switch count: a thread that stays in D with the same count is stuck in one wait. The first search reads every descriptor in /proc, trying the lock every 64; later searches re-read only the holders found before. No change in behaviour until the host GPU lock is engaged on the host, which no code in this pull request does: only an engaged lock waits. Also: tests of the creator and of a child as blockers, and of the search's try of the lock. Signed-off-by: Marcus Granado <marcus.granado@citrix.com>
A holder of the host GPU lock can get stuck and keep the lock for
minutes or more: a driver call that hangs leaves one of its threads in
an uninterruptible wait in the kernel (state D), which no kill can end.
A wait behind such a holder still lasts its whole timeout, 30 s, then
says only that the lock stayed busy; and every caller that comes next
waits its 30 s in turn.
Refuse such waits early, with one of two errors:
- `Stuck after 2 s, when the wait starts behind a holder stuck in the
kernel: one of its threads is in state D at each of 6 samples over
2 s, with no voluntary context switch in between. A healthy holder
waits in the kernel about a second at most, well inside the 2 s.
- `Busy at once, behind a holder of another process that outlasted a
whole wait and still holds: such a holder is likely to keep the
lock, and each later caller would wait 30 s for nothing. When a wait
runs out behind holders of other processes, the program that waited
remembers those processes by pid and start time, unique even if a
pid is reused, and for 20 s refuses at once a wait that finds one of
them still holding. As it remembers processes, not holds, and a
daemon is one process for all its holds, the memory lapses after
20 s: the daemon's next hold may be a new, healthy one.
For example, if xenopsd keeps the lock for minutes without being
stuck: a VF enable in xapi at 0 s waits 30 s and fails with `Busy,
and xapi remembers xenopsd until 50 s; one at 35 s fails at once,
not at 65 s; one at 55 s waits 30 s again. Five operations started
one after another fail in about 30 s, not 2.5 minutes.
The two errors read:
a holder of the GPU lock is stuck: unbind 0000:3b:00.0; holders: 1 (see the host's log)
the GPU lock stayed busy: enable VFs of 0000:3b:00.0; holders: 1 (see the host's log)
Holds that the waiting program took itself, in another of its threads
or through a tool it started while holding the lock (as xapi's VF
enable and xenopsd's unbind do), are never remembered, so a wait
behind them gets its full 30 s; `Stuck still applies. They are its
own operations, waiting for one another in turn, not a holder it
knows nothing about. And for a hold by another thread, the holding
process is the program itself: remembering it would refuse every wait
behind any of its later operations for 20 s, even short, healthy ones.
Today xapi and xenopsd queue their own threads on a mutex before
taking the lock, so this matters once a program takes it in more than
one place.
No change in behaviour until the host GPU lock is engaged on the host,
which no code in this pull request does. Also: a test of the fast
refusal. on_stuck, which must not raise, is for an external GPU tool
that reports the stuck threads; nothing here passes one.
Signed-off-by: Marcus Granado <marcus.granado@citrix.com>
A host process that takes the host GPU lock cannot tell whether xapi and xenopsd take it too. Where they do not, as on every host until the lock is turned on, its hold protects nothing: they may change a card while the process works on it. Keep a small number, the label, in the lock file's mark. Any holder reads it, and only an exclusive holder of the current lock file can change it, so whoever turns the lock on or off can record there what other processes may do, and every holder sees the same value. A new or repaired lock file starts at 0. label reads the label of an attested file, as decimal text only, else 0; set_label rewrites it under an exclusive hold, keeping the rest of the mark, and fails unless the hold is on the current, attested lock file. No change in behaviour: nothing calls them yet. Their first users are a later change that turns the lock on and off, the operator's command for the lock, and a program outside this repository. Also: tests of the label. Signed-off-by: Marcus Granado <marcus.granado@citrix.com>
A process that holds the host GPU lock cannot always let go of it by closing its descriptor: a flock(2) hold belongs to the open file, and stays while any copy of the descriptor is open, for example in a child the process started. The lock can then stay held after the work under it has ended, and every other taker waits for nothing, up to its whole timeout. Give a process outside xapi and xenopsd what it needs to take part in the lock safely: - unlock ends its hold for every copy of the descriptor, then closes it; - inode tells which lock file it holds: a repair can put a new file at the same path, so the path alone does not; - fields of the caller's in the lock file's mark carry facts that must not disagree with the lock, such as what it covers: its creator gives them when it attests the file, and any holder reads them back; - xen-api installs the xapi-gpu libraries, so that code outside this repository can link them. attest_if_alone gains ?extra, fields made by Flock.field, each name once, and ?carry (default true), which keeps an older mark's fields of the caller's; field_value reads one back, or says `Unattested. No change in behaviour: nothing calls them yet, and attest_if_alone keeps its behaviour by default. Their first user is a program outside this repository. Also: tests of the fields, unlock and inode; Unixext.flock_unlock binds flock(2)'s LOCK_UN. Signed-off-by: Marcus Granado <marcus.granado@citrix.com>
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Separate programs of a host often need to take turns on one resource like a host's GPU.
xapi enables a card's SR-IOV virtual functions with sriov-manage -e. xenopsd unbinds a card for pass-through (sriov-manage -d, nvidia-smi, the driver's unbind lock). Monitoring tools query the same driver. Nothing makes them take turns, and xen-api has no lock between processes for this: xenopsd's bind_lock keeps only its own threads apart, and nbd_client_manager.py waits on an unbounded flock that cannot name its holder.
This pull request adds such a lock and takes it at those two call sites. Lock_rw is the classic readers-writer lock (Courtois, Heymans and Parnas, 1971), with every wait bounded. Flock implements it with flock(2) on files under /run/xapi-gpu-lock, so a holder that dies releases the lock and a command it forks keeps it until the command exits. A lock file is created under no name and attested by a census of /proc, so a deleted, renamed or restored file can never let two programs both believe they hold the lock. The same census lists the holders for operators.
Nothing changes yet: xapi's switch stays off and nothing creates xenopsd's, so no lock is taken and no command gets a timeout. Once a later change engages the lock, a VF enable or an unbind waits up to 30 s for another holder, then fails with "the GPU lock stayed busy: unbind 0000:3b:00.0; holders: 1 (see the host's log)", and the log names each holder. sriov-manage is killed after 4 minutes, failing the operation with a text that names the card and what may be left. nvidia-smi --persistence-mode=0 is killed after 30 s with a warning, and the driver's unbind lock decides whether the unbind goes on.
Testing: every commit was built and its tests run (dev profile, ocamlformat, opam, unit tests), and a scratch RPM build ran %check. Each new test was shown to fail on a planted fault, and the census was timed on a XenServer 9 host's dom0 (220 runs, 31.7-59.7 ms, every holder counted).