From be8a7acbcb9f56cf38675937249561b258ec9c55 Mon Sep 17 00:00:00 2001 From: Li Zebin Date: Sat, 29 Jul 2023 13:12:50 +0800 Subject: [PATCH 1/2] vsock: Move `iter` outside of loop in `process_rx_queue` the iter() function is used for produce a queue iterator to iterate over the descriptors. But usually, it shouldn't be in the while loop, which might brings more unnecessary overhead. So move `iter` outside of the while loop. And the process_tx_queue has the same problem, maybe we can fix it, too. Signed-off-by: Li Zebin --- crates/vsock/src/vhu_vsock_thread.rs | 125 ++++++++++++++------------- 1 file changed, 65 insertions(+), 60 deletions(-) diff --git a/crates/vsock/src/vhu_vsock_thread.rs b/crates/vsock/src/vhu_vsock_thread.rs index 0c2ab2f3c..371b2ff08 100644 --- a/crates/vsock/src/vhu_vsock_thread.rs +++ b/crates/vsock/src/vhu_vsock_thread.rs @@ -462,76 +462,81 @@ impl VhostUserVsockThread { let queue = vring_mut.get_queue_mut(); - while let Some(mut avail_desc) = queue - .iter(atomic_mem.memory()) - .map_err(|_| Error::IterateQueue)? - .next() - { - used_any = true; - let mem = atomic_mem.clone().memory(); - - let head_idx = avail_desc.head_index(); - let used_len = match VsockPacket::from_rx_virtq_chain( - mem.deref(), - &mut avail_desc, - self.tx_buffer_size, - ) { - Ok(mut pkt) => { - let recv_result = match rx_queue_type { - RxQueueType::Standard => self.thread_backend.recv_pkt(&mut pkt), - RxQueueType::RawPkts => self.thread_backend.recv_raw_pkt(&mut pkt), - }; - - if recv_result.is_ok() { - PKT_HEADER_SIZE + pkt.len() as usize - } else { - queue.iter(mem).unwrap().go_to_previous_position(); - break; + let mut iter_has_elemnt = true; + while iter_has_elemnt { + let queue_iter = queue + .iter(atomic_mem.memory()) + .map_err(|_| Error::IterateQueue)?; + + iter_has_elemnt = false; + for mut avail_desc in queue_iter { + used_any = true; + iter_has_elemnt = true; + let mem = atomic_mem.clone().memory(); + + let head_idx = avail_desc.head_index(); + let used_len = match VsockPacket::from_rx_virtq_chain( + mem.deref(), + &mut avail_desc, + self.tx_buffer_size, + ) { + Ok(mut pkt) => { + let recv_result = match rx_queue_type { + RxQueueType::Standard => self.thread_backend.recv_pkt(&mut pkt), + RxQueueType::RawPkts => self.thread_backend.recv_raw_pkt(&mut pkt), + }; + + if recv_result.is_ok() { + PKT_HEADER_SIZE + pkt.len() as usize + } else { + queue.iter(mem).unwrap().go_to_previous_position(); + break; + } } - } - Err(e) => { - warn!("vsock: RX queue error: {:?}", e); - 0 - } - }; + Err(e) => { + warn!("vsock: RX queue error: {:?}", e); + 0 + } + }; - let vring = vring.clone(); - let event_idx = self.event_idx; + let vring = vring.clone(); + let event_idx = self.event_idx; - self.pool.spawn_ok(async move { - // TODO: Understand why doing the following in the pool works - if event_idx { - if vring.add_used(head_idx, used_len as u32).is_err() { - warn!("Could not return used descriptors to ring"); - } - match vring.needs_notification() { - Err(_) => { - warn!("Could not check if queue needs to be notified"); - vring.signal_used_queue().unwrap(); + self.pool.spawn_ok(async move { + // TODO: Understand why doing the following in the pool works + if event_idx { + if vring.add_used(head_idx, used_len as u32).is_err() { + warn!("Could not return used descriptors to ring"); } - Ok(needs_notification) => { - if needs_notification { + match vring.needs_notification() { + Err(_) => { + warn!("Could not check if queue needs to be notified"); vring.signal_used_queue().unwrap(); } + Ok(needs_notification) => { + if needs_notification { + vring.signal_used_queue().unwrap(); + } + } } + } else { + if vring.add_used(head_idx, used_len as u32).is_err() { + warn!("Could not return used descriptors to ring"); + } + vring.signal_used_queue().unwrap(); } - } else { - if vring.add_used(head_idx, used_len as u32).is_err() { - warn!("Could not return used descriptors to ring"); - } - vring.signal_used_queue().unwrap(); - } - }); + }); - match rx_queue_type { - RxQueueType::Standard => { - if !self.thread_backend.pending_rx() { - break; + match rx_queue_type { + RxQueueType::Standard => { + if !self.thread_backend.pending_rx() { + break; + } } - } - RxQueueType::RawPkts => { - if !self.thread_backend.pending_raw_pkts() { - break; + RxQueueType::RawPkts => { + if !self.thread_backend.pending_raw_pkts() { + break; + } } } } From ae31c184fa05086e6d9f2d96bdf49cd6f94dccfb Mon Sep 17 00:00:00 2001 From: Li Zebin Date: Tue, 1 Aug 2023 00:05:11 +0800 Subject: [PATCH 2/2] vsock: Move `iter` outside of loop in `process_tx_queue`. the iter() function is used for produce a queue iterator to iterate over the descriptors. But usually, it shouldn't be in the while loop, which might brings more unnecessary overhead. So move iter outside of the while loop. Signed-off-by: Li Zebin --- crates/vsock/src/vhu_vsock_thread.rs | 113 ++++++++++++++------------- 1 file changed, 60 insertions(+), 53 deletions(-) diff --git a/crates/vsock/src/vhu_vsock_thread.rs b/crates/vsock/src/vhu_vsock_thread.rs index 371b2ff08..a3790468f 100644 --- a/crates/vsock/src/vhu_vsock_thread.rs +++ b/crates/vsock/src/vhu_vsock_thread.rs @@ -621,68 +621,75 @@ impl VhostUserVsockThread { None => return Err(Error::NoMemoryConfigured), }; - while let Some(mut avail_desc) = vring - .get_mut() - .get_queue_mut() - .iter(atomic_mem.memory()) - .map_err(|_| Error::IterateQueue)? - .next() - { - used_any = true; - let mem = atomic_mem.clone().memory(); - - let head_idx = avail_desc.head_index(); - let pkt = match VsockPacket::from_tx_virtq_chain( - mem.deref(), - &mut avail_desc, - self.tx_buffer_size, - ) { - Ok(pkt) => pkt, - Err(e) => { - dbg!("vsock: error reading TX packet: {:?}", e); - continue; - } - }; - - if self.thread_backend.send_pkt(&pkt).is_err() { - vring - .get_mut() - .get_queue_mut() - .iter(mem) - .unwrap() - .go_to_previous_position(); - break; - } + let mut vring_mut = vring.get_mut(); + + let queue = vring_mut.get_queue_mut(); - // TODO: Check if the protocol requires read length to be correct - let used_len = 0; + let mut iter_has_elemnt = true; + while iter_has_elemnt { + let queue_iter = queue + .iter(atomic_mem.memory()) + .map_err(|_| Error::IterateQueue)?; - let vring = vring.clone(); - let event_idx = self.event_idx; + iter_has_elemnt = false; + for mut avail_desc in queue_iter { + iter_has_elemnt = true; + used_any = true; + let mem = atomic_mem.clone().memory(); - self.pool.spawn_ok(async move { - if event_idx { - if vring.add_used(head_idx, used_len as u32).is_err() { - warn!("Could not return used descriptors to ring"); + let head_idx = avail_desc.head_index(); + let pkt = match VsockPacket::from_tx_virtq_chain( + mem.deref(), + &mut avail_desc, + self.tx_buffer_size, + ) { + Ok(pkt) => pkt, + Err(e) => { + dbg!("vsock: error reading TX packet: {:?}", e); + continue; } - match vring.needs_notification() { - Err(_) => { - warn!("Could not check if queue needs to be notified"); - vring.signal_used_queue().unwrap(); + }; + + if self.thread_backend.send_pkt(&pkt).is_err() { + vring + .get_mut() + .get_queue_mut() + .iter(mem) + .unwrap() + .go_to_previous_position(); + break; + } + + // TODO: Check if the protocol requires read length to be correct + let used_len = 0; + + let vring = vring.clone(); + let event_idx = self.event_idx; + + self.pool.spawn_ok(async move { + if event_idx { + if vring.add_used(head_idx, used_len as u32).is_err() { + warn!("Could not return used descriptors to ring"); } - Ok(needs_notification) => { - if needs_notification { + match vring.needs_notification() { + Err(_) => { + warn!("Could not check if queue needs to be notified"); vring.signal_used_queue().unwrap(); } + Ok(needs_notification) => { + if needs_notification { + vring.signal_used_queue().unwrap(); + } + } } + } else { + if vring.add_used(head_idx, used_len as u32).is_err() { + warn!("Could not return used descriptors to ring"); + } + vring.signal_used_queue().unwrap(); } - } else { - if vring.add_used(head_idx, used_len as u32).is_err() { - warn!("Could not return used descriptors to ring"); - } - vring.signal_used_queue().unwrap(); - } - }); + }); + } } Ok(used_any)