-
Notifications
You must be signed in to change notification settings - Fork 224
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Partition epoch thread list #436
Conversation
Please double-check my logic around merging the per-thread state into a unified view. It seems to be correct, but I had several bugs in it prior to this version. |
Perform memory free on epoch_exit Signed-off-by: Alan Jowett <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok modulo SAL annotation nits
Signed-off-by: Alan Jowett <[email protected]>
Signed-off-by: Alan Jowett <[email protected]>
EBPF_HASH_TABLE_OPERATION_INSERT); | ||
} else if (local_result == EBPF_SUCCESS) { | ||
// Found, merge the global and per-CPU entry. | ||
new_thread_entry->entry_epoch = max(new_thread_entry->entry_epoch, thread_entry->entry_epoch); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Non blocking Suggestion: consider updating the lowest_epoch here itself? Will save another scan through the merged hashtable after this.
cpu_entry->epoch = _ebpf_current_epoch; | ||
cpu_entry->non_preemptible_epoch = _ebpf_current_epoch; | ||
if (!ebpf_list_is_empty(&cpu_entry->free_list)) { | ||
_ebpf_epoch_release_free_list(cpu_entry, _ebpf_release_epoch); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Non blocking: Wonder if _release_free_list function should re-arm the GC timer? Consider the case - where this function calls _release_free_list - but it may not be able to free all zombie entries if another thread on another CPU is actively using it. When that other CPU exits epoch, it may not find any non-empty free list on its core - so no timers are scheduled. And the zombie entries on this CPU lingers on. I believe this is a bug and must be addressed by another PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Issue #445
Signed-off-by: Alan Jowett <[email protected]>
Partition epoch thread list.
Perform memory-free on epoch_exit.
Resolves: #417
Signed-off-by: Alan Jowett [email protected]