Skip to content

Conversation

@bryancall
Copy link
Contributor

@bryancall bryancall commented Sep 24, 2019

Using this change to track down #5943.

I would like for Continuations with thread affinity to have mutexes that also have thread affinity, but that blew up in the AIO code.

@bryancall bryancall self-assigned this Sep 24, 2019
@oknet
Copy link
Member

oknet commented Oct 8, 2019

To make Continuation thread affinity, just use EThread::mutex as its mutex.

@oknet
Copy link
Member

oknet commented Oct 8, 2019

It is better to rewrite UnixNetVConnection::reenable(VIO *vio) within this PR, because it will also obtain the nh->mutex across EThread.

if (nh->mutex->thread_holding == t) {
if (vio == &read.vio) {
ep.modify(EVENTIO_READ);
ep.refresh(EVENTIO_READ);
if (read.triggered) {
nh->read_ready_list.in_or_enqueue(this);
} else {
nh->read_ready_list.remove(this);
}
} else {
ep.modify(EVENTIO_WRITE);
ep.refresh(EVENTIO_WRITE);
if (write.triggered) {
nh->write_ready_list.in_or_enqueue(this);
} else {
nh->write_ready_list.remove(this);
}
}
} else {
MUTEX_TRY_LOCK(lock, nh->mutex, t);
if (!lock.is_locked()) {
if (vio == &read.vio) {
int isin = ink_atomic_swap(&read.in_enabled_list, 1);
if (!isin) {
nh->read_enable_list.push(this);
}
} else {
int isin = ink_atomic_swap(&write.in_enabled_list, 1);
if (!isin) {
nh->write_enable_list.push(this);
}
}
if (likely(nh->thread)) {
nh->thread->tail_cb->signalActivity();
} else if (nh->trigger_event) {
nh->trigger_event->ethread->tail_cb->signalActivity();
}
} else {
if (vio == &read.vio) {
ep.modify(EVENTIO_READ);
ep.refresh(EVENTIO_READ);
if (read.triggered) {
nh->read_ready_list.in_or_enqueue(this);
} else {
nh->read_ready_list.remove(this);
}
} else {
ep.modify(EVENTIO_WRITE);
ep.refresh(EVENTIO_WRITE);
if (write.triggered) {
nh->write_ready_list.in_or_enqueue(this);
} else {
nh->write_ready_list.remove(this);
}
}
}
}

zwoop
zwoop previously requested changes Jun 9, 2020
Copy link
Contributor

@zwoop zwoop left a comment

Choose a reason for hiding this comment

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

Seems this needs more work, going to -1 it for now, and move it out to 9.1.x

@bryancall
Copy link
Contributor Author

[approve ci]

1 similar comment
@bryancall
Copy link
Contributor Author

[approve ci]

@zwoop zwoop dismissed their stale review June 25, 2021 15:28

Bryan to take a look.

@bryancall
Copy link
Contributor Author

[approve ci]

@github-actions
Copy link

This pull request has been automatically marked as stale because it has not had recent activity. Marking it stale to flag it for further consideration by the community.

@github-actions github-actions bot added the Stale label Oct 14, 2021
@bryancall
Copy link
Contributor Author

[approve ci]

@github-actions github-actions bot closed this Oct 27, 2021
@zwoop zwoop removed this from the 10.0.0 milestone Nov 8, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants