Don't singleton batched tasks rings before executing them - #790
Conversation
If the task returns PARSEC_HOOK_RETURN_AGAIN, we must not break the ring and instead hand it back to the execution step. Signed-off-by: Joseph Schuchart <joseph.schuchart@stonybrook.edu>
There was a problem hiding this comment.
Pull request overview
This PR fixes a GPU batching correctness issue where a task that returns PARSEC_HOOK_RETURN_AGAIN could be inadvertently converted back into a singleton before re-execution, dropping follower tasks from a batched ring (Issue #788).
Changes:
- Preserve the batched task ring across
PARSEC_HOOK_RETURN_AGAINretries by skippingPARSEC_LIST_ITEM_SINGLETONin that case. - Add clarifying comments around the ring/singleton reset behavior in
parsec_device_kernel_exec.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if (gpu_task->last_status != PARSEC_HOOK_RETURN_AGAIN) { | ||
| /* The task is being rescheduled, we need to reset the status */ | ||
| PARSEC_LIST_ITEM_SINGLETON(&gpu_task->list_item); | ||
| } |
|
First, I don't see why a task that batches other tasks should be allowed to return AGAIN. What is the conceptual meaning behind this ? That being said if a task does return AGAIN, it should lose all the batched tasks, because nobody wants a hot task with a bunch of potentially batched tasks lingering around. I assume you have a use case for this ? Can you detail please. |
|
In TTG, all device tasks return AGAIN. That's the design we settled on, so that we can suspend the task and submit additional kernels and/or transfers. We hardly ever do submit additional work, but it's what we support today. We can splice the ring back into the pending queue (my initial approach). We have to assume that all tasks returned AGAIN, and schedule them again individually, possibly as part of another batch. In that case, having a separate return value of DONE would be helpful to avoid scheduling another event for the task even if that particular task completed. Either way, the application has to deal with the fact that some tasks coming back at it may already be complete. |
|
Because of the coroutines ? I need to think a little more about this it is different than the model we used so far. |
|
Superseded by #792, closing |
If the task returns PARSEC_HOOK_RETURN_AGAIN, we must not break the ring and instead hand it back to the execution step.
Alternative is to feed the ring back into the pending queue but that potentially increases the number of events since now every task is treated separately. It is up to the application to handle the case where the state of tasks within a ring diverge.
Fixes #788