Skip to content

co_manager shortcuting the scheduler - #566

Closed
BrieucNicolas wants to merge 1 commit into
ICLDisco:masterfrom
BrieucNicolas:co_manager_shortcut
Closed

co_manager shortcuting the scheduler#566
BrieucNicolas wants to merge 1 commit into
ICLDisco:masterfrom
BrieucNicolas:co_manager_shortcut

Conversation

@BrieucNicolas

Copy link
Copy Markdown
Contributor

Continuation of #509 adding a shortcut when discovering tasks in the dtd. The co-manager executes ready tasks discovered during completion immediatly instead of scheduling them

@BrieucNicolas
BrieucNicolas requested a review from a team as a code owner August 9, 2023 15:57
fix for multiple gpus

Added debug output and some documentation, error tolerance for multiple gpu and no manager, refactored loop in insert_function, got rid of deadlock

@devreal devreal left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not familiar with #509 so I might be missing something in my review.

if (PARSEC_DTD_FLUSH_TC_ID == current_task->task_class->task_class_id)
{
PARSEC_DEBUG_VERBOSE(10, parsec_gpu_output_stream,"GPU[%s]: Thread %d scheduling task %s at %s:%d",
((parsec_device_module_t*)*co_manager_tls_val)->name, es->th_id,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These casts are not necessary (here and elsewhere)

Comment thread parsec/interfaces/dtd/insert_function.c
*/
while(1) {
rc = gpu_device->mutex;
rc = rc1 = gpu_device->mutex;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't like rc1 for its lack of information. I suggest using a boolean variable like was_first_co_manager (you get the point :)) that is set to true if the CAS below succeeds and rc == 1.

PARSEC_DEBUG_VERBOSE(4, parsec_gpu_output_stream,"GPU[%s]: gpu_task %p completed by co-manager %d at %s:%d", gpu_device->super.name,
gpu_task, es->th_id, __FILE__, __LINE__);
parsec_atomic_fetch_dec_int32( &(gpu_device->complete_mutex) );
parsec_list_push_back(gpu_tasks_to_free, (parsec_list_item_t*)gpu_task);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we have to accumulate task objects here? Can they not be free'd directly?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, thank you for pointing that out. i found a fix for this, it will be applied

@bosilca

bosilca commented May 18, 2026

Copy link
Copy Markdown
Contributor

We have a simpler and less intrusive approach in #762. It lacks the DTD support for executing the ready tasks returned by the completion immediately, but if we really want that capability we should reimplement it instead of reviving this PR.

@bosilca bosilca closed this May 18, 2026
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