Skip to content
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

Basic working multi-driver demo using async bbq #3

Merged
merged 6 commits into from
Jul 7, 2022
Merged

Conversation

jamesmunns
Copy link
Contributor

This implements a quad-waitcell version of bbq, as well as a basic PoC with two sort-of drivers. This data structure is intended for drivers such as a serial port.

@jamesmunns jamesmunns requested a review from hawkw July 7, 2022 16:47
Copy link
Contributor

@hawkw hawkw left a comment

Choose a reason for hiding this comment

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

this is awesome!

i had some tiny unimportant suggestions, take them or leave them

source/kernel/src/bbq.rs Outdated Show resolved Hide resolved
source/kernel/src/bbq.rs Outdated Show resolved Hide resolved
source/kernel/src/bbq.rs Outdated Show resolved Hide resolved
source/kernel/src/bbq.rs Outdated Show resolved Hide resolved
source/melpomene/src/main.rs Outdated Show resolved Hide resolved
source/melpomene/src/main.rs Outdated Show resolved Hide resolved
jamesmunns and others added 4 commits July 7, 2022 19:38
@jamesmunns jamesmunns merged commit 78ef6c6 into main Jul 7, 2022
Copy link
Contributor

@hawkw hawkw left a comment

Choose a reason for hiding this comment

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

👍 on the Arc, this is definitely a lot nicer...we can probably bring back the NonNulls as an optimization to avoid the field access, if it's necessary, but this feels like a safer starting point.

some tiny suggestions on the tracing stuff, but no blockers from me

Comment on lines +194 to +211
"Got bbqueue max write grant",
);
return GrantW {
grant: wgr,
side: self.side,
storage: self.storage.clone(),
}
},
Err(_) => {
trace!("awaiting bbqueue max write grant");
// Uh oh! Couldn't get a send grant. We need to wait for the OTHER reader
// to release some bytes first.
match self.side {
Side::ASide => &self.storage.b_wait,
Side::BSide => &self.storage.a_wait,
}.release_waitcell.wait().await.unwrap();

trace!("awoke for bbqueue max write grant");
Copy link
Contributor

Choose a reason for hiding this comment

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

nit, take it or leave it: maybe worth sticking

Suggested change
"Got bbqueue max write grant",
);
return GrantW {
grant: wgr,
side: self.side,
storage: self.storage.clone(),
}
},
Err(_) => {
trace!("awaiting bbqueue max write grant");
// Uh oh! Couldn't get a send grant. We need to wait for the OTHER reader
// to release some bytes first.
match self.side {
Side::ASide => &self.storage.b_wait,
Side::BSide => &self.storage.a_wait,
}.release_waitcell.wait().await.unwrap();
trace!("awoke for bbqueue max write grant");
side = ?self.side,
"Got bbqueue max write grant",
);
return GrantW {
grant: wgr,
side: self.side,
storage: self.storage.clone(),
}
},
Err(_) => {
trace!(side = ?self.side, max, "awaiting bbqueue max write grant");
// Uh oh! Couldn't get a send grant. We need to wait for the OTHER reader
// to release some bytes first.
match self.side {
Side::ASide => &self.storage.b_wait,
Side::BSide => &self.storage.a_wait,
}.release_waitcell.wait().await.unwrap();
trace!(side = ?self.side, max, "awoke for bbqueue max write grant");

Comment on lines +221 to +240
trace!(
size = size,
"Got bbqueue exact write grant",
);
return GrantW {
grant: wgr,
side: self.side,
storage: self.storage.clone(),
}
},
Err(_) => {
trace!("awaiting bbqueue exact write grant");
// Uh oh! Couldn't get a send grant. We need to wait for the OTHER reader
// to release some bytes first.
match self.side {
Side::ASide => &self.storage.b_wait,
Side::BSide => &self.storage.a_wait,
}.release_waitcell.wait().await.unwrap();
trace!("awoke for bbqueue exact write grant");
},
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
trace!(
size = size,
"Got bbqueue exact write grant",
);
return GrantW {
grant: wgr,
side: self.side,
storage: self.storage.clone(),
}
},
Err(_) => {
trace!("awaiting bbqueue exact write grant");
// Uh oh! Couldn't get a send grant. We need to wait for the OTHER reader
// to release some bytes first.
match self.side {
Side::ASide => &self.storage.b_wait,
Side::BSide => &self.storage.a_wait,
}.release_waitcell.wait().await.unwrap();
trace!("awoke for bbqueue exact write grant");
},
trace!(
size = size,
side = ?self.side,
"Got bbqueue exact write grant",
);
return GrantW {
grant: wgr,
side: self.side,
storage: self.storage.clone(),
}
},
Err(_) => {
trace!(side = ?self.side, size, "awaiting bbqueue exact write grant");
// Uh oh! Couldn't get a send grant. We need to wait for the OTHER reader
// to release some bytes first.
match self.side {
Side::ASide => &self.storage.b_wait,
Side::BSide => &self.storage.a_wait,
}.release_waitcell.wait().await.unwrap();
trace!(side = ?self.side, size, "awoke for bbqueue exact write grant");
},

Comment on lines +250 to +267
size = rgr.len(),
"Got bbqueue read grant",
);
return GrantR {
grant: rgr,
side: self.side,
storage: self.storage.clone(),
}
},
Err(_) => {
trace!("awaiting bbqueue read grant");
// Uh oh! Couldn't get a read grant. We need to wait for the OTHER writer
// to commit some bytes first.
match self.side {
Side::ASide => &self.storage.b_wait.commit_waitcell,
Side::BSide => &self.storage.a_wait.commit_waitcell,
}.wait().await.unwrap();
trace!("awoke for bbqueue read grant");
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
size = rgr.len(),
"Got bbqueue read grant",
);
return GrantR {
grant: rgr,
side: self.side,
storage: self.storage.clone(),
}
},
Err(_) => {
trace!("awaiting bbqueue read grant");
// Uh oh! Couldn't get a read grant. We need to wait for the OTHER writer
// to commit some bytes first.
match self.side {
Side::ASide => &self.storage.b_wait.commit_waitcell,
Side::BSide => &self.storage.a_wait.commit_waitcell,
}.wait().await.unwrap();
trace!("awoke for bbqueue read grant");
size = rgr.len(),
side = ?self.side,
"Got bbqueue read grant",
);
return GrantR {
grant: rgr,
side: self.side,
storage: self.storage.clone(),
}
},
Err(_) => {
trace!(side = ?self.side, "awaiting bbqueue read grant");
// Uh oh! Couldn't get a read grant. We need to wait for the OTHER writer
// to commit some bytes first.
match self.side {
Side::ASide => &self.storage.b_wait.commit_waitcell,
Side::BSide => &self.storage.a_wait.commit_waitcell,
}.wait().await.unwrap();
trace!(side = ?self.side, "awoke for bbqueue read grant");

match self.producer.grant_exact(size) {
Ok(wgr) => {
trace!(
size = size,
Copy link
Contributor

Choose a reason for hiding this comment

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

tiny nit: this could just be

Suggested change
size = size,
size,

Ok(wgr) => {
trace!(
size = wgr.len(),
max = max,
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: could just be

Suggested change
max = max,
max,

@jamesmunns jamesmunns deleted the bidi-async branch July 17, 2022 00:01
jamesmunns added a commit that referenced this pull request Jun 4, 2023
@jamesmunns jamesmunns added platform: melpomene Specific to the Melpomene simulator platform area: drivers Related to driver implementation for hardware devices. labels Jul 31, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: drivers Related to driver implementation for hardware devices. platform: melpomene Specific to the Melpomene simulator platform
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants