M-Labs
/
artiq-zynq
10
1
Fork 19
Code Issues 13 Pull Requests Releases Wiki Activity

Support CoreMgmt over DRTIO on Zynq Devices #323

Merged
sb10q merged 27 commits from occheung/artiq-zynq:drtio-coremgmt into master 2024-11-19 18:55:03 +08:00
Conversation 1 Commits 27 Files Changed 10 +10 -16
2 changed files with 10 additions and 16 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files
Showing only changes of commit a13f5d02fa - Show all commits

2
src/runtime/src/comms.rs
View File

@ -788,7 +788,7 @@ pub fn main(timer: GlobalTimer, cfg: Config) {
mgmt::start( mgmt::start(
cfg.clone(), cfg.clone(),
restart_idle.clone(), restart_idle.clone(),
Some((&aux_mutex, &drtio_routing_table, timer)), Some(mgmt::DrtioTuple(aux_mutex.clone(), drtio_routing_table.clone(), timer)),
); );
task::spawn(async move { task::spawn(async move {

24
src/runtime/src/mgmt.rs
View File

@ -1,5 +1,5 @@
use alloc::{rc::Rc, string::String, vec::Vec}; use alloc::{rc::Rc, string::String, vec::Vec};
use core::{cell::RefCell, ops::Deref}; use core::cell::RefCell;
use futures::{future::poll_fn, task::Poll}; use futures::{future::poll_fn, task::Poll};
use libasync::{smoltcp::TcpStream, task}; use libasync::{smoltcp::TcpStream, task};
@ -842,9 +842,10 @@ macro_rules! process {
($stream: ident, $drtio_tuple:ident, $destination:expr, $func:ident $(, $param:expr)*) => {{ ($stream: ident, $drtio_tuple:ident, $destination:expr, $func:ident $(, $param:expr)*) => {{
if $destination == 0 { if $destination == 0 {
local_coremgmt::$func($stream, $($param, )*).await local_coremgmt::$func($stream, $($param, )*).await
} else if let Some((aux_mutex, routing_table, timer)) = $drtio_tuple { } else if let Some(DrtioTuple(ref aux_mutex, ref routing_table, timer)) = $drtio_tuple {
let routing_table = routing_table.borrow();
let linkno = routing_table.0[$destination as usize][0] - 1 as u8; let linkno = routing_table.0[$destination as usize][0] - 1 as u8;
remote_coremgmt::$func($stream, aux_mutex, routing_table, timer, linkno, $destination, $($param, )*).await remote_coremgmt::$func($stream, &aux_mutex, &routing_table, timer, linkno, $destination, $($param, )*).await
} else { } else {
error!("coremgmt-over-drtio not supported for panicked device, please reboot"); error!("coremgmt-over-drtio not supported for panicked device, please reboot");
write_i8($stream, Reply::Error as i8).await?; write_i8($stream, Reply::Error as i8).await?;
@ -860,12 +861,15 @@ macro_rules! process {
}} }}
} }
#[derive(Clone)]
pub struct DrtioTuple(pub Rc<Mutex<bool>>, pub Rc<RefCell<RoutingTable>>, pub GlobalTimer);
sb10q commented 2024-09-25 17:28:36 +08:00
Outdated
Review
Copy Link

I think no other software calls objects "tuples".

I think no other software calls objects "tuples".
sb10q commented 2024-11-16 13:00:32 +08:00
Outdated
Review
Copy Link

What about DrtioContext?
And name the fields, as is usually done.

What about DrtioContext? And name the fields, as is usually done.
async fn handle_connection( async fn handle_connection(
stream: &mut TcpStream, stream: &mut TcpStream,
pull_id: Rc<RefCell<u32>>, pull_id: Rc<RefCell<u32>>,
cfg: Rc<Config>, cfg: Rc<Config>,
restart_idle: Rc<Semaphore>, restart_idle: Rc<Semaphore>,
_drtio_tuple: Option<(&Rc<Mutex<bool>>, &RoutingTable, GlobalTimer)>, _drtio_tuple: Option<DrtioTuple>,
mwojcik commented 2024-08-28 13:55:36 +08:00
Outdated
Review
Copy Link

Slightly related: I like the tuple to keep the argument list shorter/more compartmentalized, should we adapt it in other places too?

Slightly related: I like the tuple to keep the argument list shorter/more compartmentalized, should we adapt it in other places too?
srenblad commented 2024-09-02 17:57:04 +08:00
Outdated
Review
Copy Link

Agreed that a a lot of argument lists are needlessly long. Especially with the shared sync primitives theres a lot of repetition. Though I'm not sure that these tuples make things much shorter, without some type aliasing. Is there a case for putting these shared sync prims in a struct and passing around an Rc to that instead? (In a similar style to how the mainline repo passes around Io)

Putting the idea out there, obviously it would be a heavy refactor...

Agreed that a a lot of argument lists are needlessly long. Especially with the shared sync primitives theres a lot of repetition. Though I'm not sure that these tuples make things much shorter, without some type aliasing. Is there a case for putting these shared sync prims in a struct and passing around an `Rc` to that instead? (In a similar style to how the mainline repo passes around `Io`) Putting the idea out there, obviously it would be a heavy refactor...
) -> Result<()> { ) -> Result<()> {
if !expect(&stream, b"ARTIQ management\n").await? { if !expect(&stream, b"ARTIQ management\n").await? {
return Err(Error::UnexpectedPattern); return Err(Error::UnexpectedPattern);
@ -957,10 +961,8 @@ async fn handle_connection(
pub fn start( pub fn start(
cfg: Rc<Config>, cfg: Rc<Config>,
restart_idle: Rc<Semaphore>, restart_idle: Rc<Semaphore>,
drtio_tuple: Option<(&Rc<Mutex<bool>>, &Rc<RefCell<RoutingTable>>, GlobalTimer)>, drtio_tuple: Option<DrtioTuple>,
) { ) {
let drtio_tuple =
drtio_tuple.map(|(aux_mutex, routing_table, timer)| (aux_mutex.clone(), routing_table.clone(), timer));
task::spawn(async move { task::spawn(async move {
let pull_id = Rc::new(RefCell::new(0u32)); let pull_id = Rc::new(RefCell::new(0u32));
loop { loop {
@ -971,14 +973,6 @@ pub fn start(
let drtio_tuple = drtio_tuple.clone(); let drtio_tuple = drtio_tuple.clone();
task::spawn(async move { task::spawn(async move {
info!("received connection"); info!("received connection");
// Avoid consuming the tuple
// Keep the borrowed value on stack
let drtio_tuple = drtio_tuple
.as_ref()
.map(|(aux_mutex, routing_table, timer)| (aux_mutex, routing_table.borrow(), *timer));
let drtio_tuple = drtio_tuple
.as_ref()
.map(|(aux_mutex, routing_table, timer)| (*aux_mutex, routing_table.deref(), *timer));
let _ = handle_connection(&mut stream, pull_id, cfg, restart_idle, drtio_tuple) let _ = handle_connection(&mut stream, pull_id, cfg, restart_idle, drtio_tuple)
.await .await
.map_err(|e| warn!("connection terminated: {:?}", e)); .map_err(|e| warn!("connection terminated: {:?}", e));