Repository navigation
Conversation
* device: remove the unused parsec_device_get_copy The function has no declaration in any header and no caller anywhere in the tree, so it has never run. Being non-static it drew no unused-function warning, which let it rot unnoticed while the equivalent LRU eviction logic in parsec_device_data_reserve_space kept evolving. Signed-off-by: George Bosilca <gbosilca@nvidia.com> * device: drop a local that only a disabled verbose was reading The prefetch dedup path bound the existing device copy to a local that nothing but a PARSEC_DEBUG_VERBOSE consumed, so every build without verbose debugging warned about an unused variable. Name the copy directly in the verbose instead. Signed-off-by: George Bosilca <gbosilca@nvidia.com> * device: drop two long-disabled blocks and the comments they left behind Both #if 0 blocks predate GPU copies being propagated to successors: one swapped a read-only data_out back to the CPU copy in pop, the other lied to the engine about the CPU version of a written copy in the epilog. The pop block was also the only thing that ever overwrote data_out for a read-only flow, so the two comments justifying the read-only skip in kernel_epilog and kernel_cleanout by pointing at it were describing code that does not run. Say what the skip really rests on instead. Signed-off-by: George Bosilca <gbosilca@nvidia.com> --------- Signed-off-by: George Bosilca <gbosilca@nvidia.com>
__parsec_taskpool_wait() and __parsec_taskpool_test() asserted that the calling thread was the master. That contradicts nested taskpools: a task is allowed to submit a taskpool and wait for it, in which case a worker drives the loop. dtd_test_hierarchy describes itself as testing hierarchical execution and aborts on that assert whenever asserts are live. Replace the assert with a progress_comms flag. Any thread executes tasks from the loop, but only the master owes the context its communication progress, which is what the assert was really protecting. Signed-off-by: George Bosilca <gbosilca@nvidia.com>
The destructor used the list links to decide whether the array was still registered with its parsec_info_t. Removing an item from a parsec_list does not return it to a singleton state, so those links cannot answer that question, and a second pass would walk a list it no longer belongs to. Use oa->infos as the marker instead. The constructor path sets it and the destructor already clears it on the way out, so it says exactly whether we are still attached and makes a repeated destructor pass a no-op. Signed-off-by: George Bosilca <gbosilca@nvidia.com>
complete_hook_of_dtd() released a reader on the host copy of every INOUT/OUTPUT flow tagged PARSEC_PULLIN. Nothing ever took that reference: the flag arrived in 1c5f52c together with the decrement but no matching increment, and the only retain on that path, parsec_data_transfer_ownership_to_copy() in parsec_dtd_cpu_task_submit(), is released on the spot by design and says so in a comment. The reader count therefore underflowed. It went unnoticed because the only test exercising PARSEC_PULLIN, test_cuda_memset_write_read(), was given scrambled arguments for create_and_distribute_data() in the very commit that introduced the CPU path, so it aborted long before reaching the completion hook. Fix those arguments as well; the test then runs all of its subtests. Signed-off-by: George Bosilca <gbosilca@nvidia.com>
tests/CMakeLists.txt registers SKIP_RETURN_CODE 10 (-PARSEC_ERR_DEVICE)
so that a test unable to reach an accelerator is reported as skipped.
Only nvlink, stage and redistribute honoured it, so on a machine without
a GPU one device test failed and five reported a green pass having
touched no device at all.
- dtd_test_cuda_again_async had no device check and walked into
creating CUDA-only task classes, fataling on an unsatisfiable device
mask and exiting 250.
- dtd_pingpong and ptg_pingpong returned EXIT_SUCCESS with the comment
"So that useless tests don't make the CI fail", which is exactly what
the skip code exists to express.
- stress and testing_get_best_device had no check and quietly ran on
the CPU.
- dtd_test_new_tile is also registered as a CPU-only case, so it now
takes a --require-gpu flag and only skips when the caller asked for
an accelerator.
- dtd_test_cuda_task_insert counted a skipped subtest as a failure and
returned -1 where no accelerator was present.
Label cuda_batch_status as :gpu so it is filtered like every other device
test, and register cuda_rtt as runtime/cuda/rtt:gpu: it was built by
every CUDA build but never run, and already had the correct guard.
The single-process suite is now 99 tests, 0 failed, 9 skipped on a host
without a GPU, and all of these tests still pass on one with GPUs.
Signed-off-by: George Bosilca <gbosilca@nvidia.com>
…der-release device: only touch a GPU's LRU lists while holding its manager token
Walking the incarnations to find out where a task class can run is something both the chore trimming and the device engine need to do, so give it a name instead of open coding the loop twice.
Failing loudly when no source can be reached is right, but a host mirror in the middle of receiving a write-back is not unreachable: it is the one place the value is going, and it will hold it shortly. Reporting it as a source that cannot be reached turns a wait into a fatal error. Treat it the way a peer copy being filled is already treated, as a source worth coming back for.
jdf2c decides at compile time whether a flow needs a pushout, from whether the consuming task class has an accelerator body. A task class can lose its accelerator incarnations after that, either because the build does not support the device type or because a caller trimmed them, and the compiled decision then describes a program that is no longer the one being run: the producer leaves its result on the device while the consumer reads the host copy, and the consumer computes on stale data. The successor walk that already runs before the stage out is the place that knows what the successors can do, so ask it: a successor with no accelerator incarnation reads this flow in main memory wherever it is scheduled, which means the flow has to go there. That walk was entered only when a remote successor could not be served from device memory or when the accelerators could not all read each other, as those were the only two questions it answered. It now answers one that is independent of both, so the early return goes; the check for a task class with no successor iterator stays, as the walk cannot proceed without it. Widening when the walk runs also widens who is walked, and DTD cannot be. Its successor iterator is the dependency release itself: it hands a descendant its input and retains that copy whatever the action mask says, it reads the argument as a release context rather than as whatever the caller passed, it ignores a request to stop, and it expects every descendant it walks to have been inserted already, which says nothing about a task whose successors are still being submitted. Asking it a question before the task runs answers the question by changing the program. DTD does not need to be asked in any case: it is told at insertion time which flows to push out, and that is the answer this walk would be reconstructing.
…t there The receiver predicts where to place an incoming data from the affinity of the task that will consume it, which for an owner-computes algorithm is where that task will run. It never asked whether that task can run on a device at all, so a data consumed by a task class with no accelerator incarnation could still be received straight into device memory, where its consumer cannot read it. In a distributed trsm that shows up as a rank computing on an unwritten tile. A successor with no accelerator incarnation reads this data in main memory whatever its affinity says. Once one of them has asked for it, none of the others may move it onto a device either: they can all stage it in from main memory, while none of them can read it back out of an accelerator this one cannot address. preferred_device therefore grows a host pin, -1, which no later successor overrides, and the consumer honours a device placement only when the prediction is positive. Incoming temporaries are also initialized to main memory at both iterate_successors() call sites. Some paths return before the prediction runs, notably the packed fallback for mixed receive datatypes, and must not inherit a placement hint from a recycled remote_deps item.
A task that cannot reserve the device memory it needs gave back everything it had taken and asked to be rescheduled. Nothing had promised that memory would be there the next time, so it came straight back and failed again, and on the way it evicted the blocks the other starving tasks had just given back. On a device whose memory is all dirty that goes on forever: eviction never writes anything back, so a dirty block is never a candidate, and the only mechanism that turns dirty blocks into reclaimable ones is the write-back task the manager already knows how to build. It was unreachable, because it hangs off a negative return from the push stage and the reschedule was swallowed by parsec_device_progress_stream() to wait for an event that does not exist. The reservation now says which of the two happened. A task that ran out of memory is parked on the device's gpu_mem_starved list rather than rescheduled: it queued nothing and has no event to wait for, so it takes no event slot, and it stays counted against the manager so the manager cannot leave with waiters still on the list. Seeing that list non-empty, the manager starts a write-back. When the write-back completes it reports how many copies it moved, and that many tasks are woken; waking the whole list on the strength of one write-back would only put the device back to evicting what the others need. A task cannot be held for a release that is not coming, so when the manager runs out of work entirely the list is emptied and its tasks get to try again. The status the reservation returns is private to this file, travelling only as far as parsec_device_progress_stream(), which turns it back into an uneventful tour. It is not something a body can return and does not belong in parsec_hook_return_t. Reproduced with testing_dgemm -N 1940 -t 320 -g 1 and device_cuda_memory_number_of_blocks at 21, which hung and now completes.
A write-back compared the host version with the version the device copy carries to decide whether the value it just moved is still the current one. That comparison cannot tell success from failure. In both cases the host is behind: either because the transfer did not bring it up to date, or because the copy was handed to a writer while the transfer was in flight and now carries a newer value than the one that travelled. So the write-back always concluded that its work had been superseded, and never once reported success. Record the version being written back instead, and compare against that. A copy whose version has not moved has genuinely reached the host. The failure conclusion had a second cost. Creating the transfer takes the copy out of the owned list, and only the success path puts it back into a device list. Every write-back therefore dropped a dirty copy out of both lists, where no eviction can reach it and no later write-back can retry it. The device lost the memory of that copy for good, and after enough of them it could no longer make room for anybody. Put such a copy back in the owned list: it still holds the only current value, so it still owes the host a write-back. The recorded versions live in the task locals next to the flow count, so cap the number of flows a single write-back task can carry accordingly.
bosilca
force-pushed
the
topic/host-mirror-reachable
branch
from
September 27, 2026 17:46
3ab5dd5 to
b3e3edb
Compare
Owner
Author
|
Moved upstream as ICLDisco#812 so it is visible there. |
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.
A successor that can only run on the host, and the network, both need the value in main memory, and a device that is out of memory needs somewhere to put what it is holding. Five changes to that end: a task waits for a host mirror that is being written back; a flow is pushed out to the host when a successor cannot read it on a device; an incoming data stays in main memory when its successor can only read it there; dirty device memory is written back when tasks are starved for it; and a write-back reports whether it succeeded.
Stacked on
fix/gpu-foreign-copy-reader-release.