Skip to content

Commit 0c25f6b

Browse files
committed
refactor(queue): remove mem from pop method
`mem` parameter was not used for anything. Signed-off-by: Egor Lazarchuk <[email protected]>
1 parent 4f264ce commit 0c25f6b

File tree

12 files changed

+57
-84
lines changed

12 files changed

+57
-84
lines changed

src/vmm/benches/block_request.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ pub fn block_request_benchmark(c: &mut Criterion) {
2424
chain.set_header(request_header);
2525

2626
let mut queue = virt_queue.create_queue();
27-
let desc = queue.pop(&mem).unwrap();
27+
let desc = queue.pop().unwrap();
2828

2929
c.bench_function("request_parse", |b| {
3030
b.iter(|| {

src/vmm/benches/queue.rs

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,7 @@ pub fn queue_benchmark(c: &mut Criterion) {
6161

6262
set_dtable_one_chain(&rxq, 1);
6363
queue.next_avail = Wrapping(0);
64-
let desc = queue.pop(&mem).unwrap();
64+
let desc = queue.pop().unwrap();
6565
c.bench_function("next_descriptor_1", |b| {
6666
b.iter(|| {
6767
let mut head = Some(desc.clone());
@@ -73,7 +73,7 @@ pub fn queue_benchmark(c: &mut Criterion) {
7373

7474
set_dtable_one_chain(&rxq, 2);
7575
queue.next_avail = Wrapping(0);
76-
let desc = queue.pop(&mem).unwrap();
76+
let desc = queue.pop().unwrap();
7777
c.bench_function("next_descriptor_2", |b| {
7878
b.iter(|| {
7979
let mut head = Some(desc.clone());
@@ -85,7 +85,7 @@ pub fn queue_benchmark(c: &mut Criterion) {
8585

8686
set_dtable_one_chain(&rxq, 4);
8787
queue.next_avail = Wrapping(0);
88-
let desc = queue.pop(&mem).unwrap();
88+
let desc = queue.pop().unwrap();
8989
c.bench_function("next_descriptor_4", |b| {
9090
b.iter(|| {
9191
let mut head = Some(desc.clone());
@@ -97,7 +97,7 @@ pub fn queue_benchmark(c: &mut Criterion) {
9797

9898
set_dtable_one_chain(&rxq, 16);
9999
queue.next_avail = Wrapping(0);
100-
let desc = queue.pop(&mem).unwrap();
100+
let desc = queue.pop().unwrap();
101101
c.bench_function("next_descriptor_16", |b| {
102102
b.iter(|| {
103103
let mut head = Some(desc.clone());
@@ -113,7 +113,7 @@ pub fn queue_benchmark(c: &mut Criterion) {
113113
c.bench_function("queue_pop_1", |b| {
114114
b.iter(|| {
115115
queue.next_avail = Wrapping(0);
116-
while let Some(desc) = queue.pop(&mem) {
116+
while let Some(desc) = queue.pop() {
117117
std::hint::black_box(desc);
118118
}
119119
})
@@ -123,7 +123,7 @@ pub fn queue_benchmark(c: &mut Criterion) {
123123
c.bench_function("queue_pop_4", |b| {
124124
b.iter(|| {
125125
queue.next_avail = Wrapping(0);
126-
while let Some(desc) = queue.pop(&mem) {
126+
while let Some(desc) = queue.pop() {
127127
std::hint::black_box(desc);
128128
}
129129
})
@@ -133,7 +133,7 @@ pub fn queue_benchmark(c: &mut Criterion) {
133133
c.bench_function("queue_pop_16", |b| {
134134
b.iter(|| {
135135
queue.next_avail = Wrapping(0);
136-
while let Some(desc) = queue.pop(&mem) {
136+
while let Some(desc) = queue.pop() {
137137
std::hint::black_box(desc);
138138
}
139139
})

src/vmm/src/devices/virtio/balloon/device.rs

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -297,7 +297,7 @@ impl Balloon {
297297
// Internal loop processes descriptors and acummulates the pfns in `pfn_buffer`.
298298
// Breaks out when there is not enough space in `pfn_buffer` to completely process
299299
// the next descriptor.
300-
while let Some(head) = queue.pop(mem) {
300+
while let Some(head) = queue.pop() {
301301
let len = head.len as usize;
302302
let max_len = MAX_PAGES_IN_DESC * SIZE_OF_U32;
303303
valid_descs_found = true;
@@ -370,14 +370,12 @@ impl Balloon {
370370
}
371371

372372
pub(crate) fn process_deflate_queue(&mut self) -> Result<(), BalloonError> {
373-
// This is safe since we checked in the event handler that the device is activated.
374-
let mem = self.device_state.mem().unwrap();
375373
METRICS.deflate_count.inc();
376374

377375
let queue = &mut self.queues[DEFLATE_INDEX];
378376
let mut needs_interrupt = false;
379377

380-
while let Some(head) = queue.pop(mem) {
378+
while let Some(head) = queue.pop() {
381379
queue.add_used(head.index, 0).map_err(BalloonError::Queue)?;
382380
needs_interrupt = true;
383381
}
@@ -394,7 +392,7 @@ impl Balloon {
394392
let mem = self.device_state.mem().unwrap();
395393
METRICS.stats_updates_count.inc();
396394

397-
while let Some(head) = self.queues[STATS_INDEX].pop(mem) {
395+
while let Some(head) = self.queues[STATS_INDEX].pop() {
398396
if let Some(prev_stats_desc) = self.stats_desc_index {
399397
// We shouldn't ever have an extra buffer if the driver follows
400398
// the protocol, but return it if we find one.

src/vmm/src/devices/virtio/block/virtio/request.rs

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -473,16 +473,15 @@ mod tests {
473473
let memory = self.driver_queue.memory();
474474

475475
assert!(matches!(
476-
Request::parse(&q.pop(memory).unwrap(), memory, NUM_DISK_SECTORS),
476+
Request::parse(&q.pop().unwrap(), memory, NUM_DISK_SECTORS),
477477
Err(_e)
478478
));
479479
}
480480

481481
fn check_parse(&self, check_data: bool) {
482482
let mut q = self.driver_queue.create_queue();
483483
let memory = self.driver_queue.memory();
484-
let request =
485-
Request::parse(&q.pop(memory).unwrap(), memory, NUM_DISK_SECTORS).unwrap();
484+
let request = Request::parse(&q.pop().unwrap(), memory, NUM_DISK_SECTORS).unwrap();
486485
let expected_header = self.header();
487486

488487
assert_eq!(
@@ -949,7 +948,7 @@ mod tests {
949948
fn parse_random_requests() {
950949
let cfg = ProptestConfig::with_cases(1000);
951950
proptest!(cfg, |(mut request in random_request_parse())| {
952-
let result = Request::parse(&request.2.pop(&request.1).unwrap(), &request.1, NUM_DISK_SECTORS);
951+
let result = Request::parse(&request.2.pop().unwrap(), &request.1, NUM_DISK_SECTORS);
953952
match result {
954953
Ok(r) => prop_assert!(r == request.0.unwrap()),
955954
Err(err) => {

src/vmm/src/devices/virtio/iovec.rs

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -490,22 +490,22 @@ mod tests {
490490
fn test_access_mode() {
491491
let mem = default_mem();
492492
let (mut q, _) = read_only_chain(&mem);
493-
let head = q.pop(&mem).unwrap();
493+
let head = q.pop().unwrap();
494494
// SAFETY: This descriptor chain is only loaded into one buffer
495495
unsafe { IoVecBuffer::from_descriptor_chain(&mem, head).unwrap() };
496496

497497
let (mut q, _) = write_only_chain(&mem);
498-
let head = q.pop(&mem).unwrap();
498+
let head = q.pop().unwrap();
499499
// SAFETY: This descriptor chain is only loaded into one buffer
500500
unsafe { IoVecBuffer::from_descriptor_chain(&mem, head).unwrap_err() };
501501

502502
let (mut q, _) = read_only_chain(&mem);
503-
let head = q.pop(&mem).unwrap();
503+
let head = q.pop().unwrap();
504504
// SAFETY: This descriptor chain is only loaded into one buffer
505505
unsafe { IoVecBufferMut::from_descriptor_chain(&mem, head).unwrap_err() };
506506

507507
let (mut q, _) = write_only_chain(&mem);
508-
let head = q.pop(&mem).unwrap();
508+
let head = q.pop().unwrap();
509509
// SAFETY: This descriptor chain is only loaded into one buffer
510510
unsafe { IoVecBufferMut::from_descriptor_chain(&mem, head).unwrap() };
511511
}
@@ -514,7 +514,7 @@ mod tests {
514514
fn test_iovec_length() {
515515
let mem = default_mem();
516516
let (mut q, _) = read_only_chain(&mem);
517-
let head = q.pop(&mem).unwrap();
517+
let head = q.pop().unwrap();
518518

519519
// SAFETY: This descriptor chain is only loaded once in this test
520520
let iovec = unsafe { IoVecBuffer::from_descriptor_chain(&mem, head).unwrap() };
@@ -525,7 +525,7 @@ mod tests {
525525
fn test_iovec_mut_length() {
526526
let mem = default_mem();
527527
let (mut q, _) = write_only_chain(&mem);
528-
let head = q.pop(&mem).unwrap();
528+
let head = q.pop().unwrap();
529529

530530
// SAFETY: This descriptor chain is only loaded once in this test
531531
let iovec = unsafe { IoVecBufferMut::from_descriptor_chain(&mem, head).unwrap() };
@@ -536,7 +536,7 @@ mod tests {
536536
fn test_iovec_read_at() {
537537
let mem = default_mem();
538538
let (mut q, _) = read_only_chain(&mem);
539-
let head = q.pop(&mem).unwrap();
539+
let head = q.pop().unwrap();
540540

541541
// SAFETY: This descriptor chain is only loaded once in this test
542542
let iovec = unsafe { IoVecBuffer::from_descriptor_chain(&mem, head).unwrap() };
@@ -591,7 +591,7 @@ mod tests {
591591
let (mut q, vq) = write_only_chain(&mem);
592592

593593
// This is a descriptor chain with 4 elements 64 bytes long each.
594-
let head = q.pop(&mem).unwrap();
594+
let head = q.pop().unwrap();
595595

596596
// SAFETY: This descriptor chain is only loaded into one buffer
597597
let mut iovec = unsafe { IoVecBufferMut::from_descriptor_chain(&mem, head).unwrap() };

src/vmm/src/devices/virtio/queue.rs

Lines changed: 14 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -500,9 +500,7 @@ impl Queue {
500500
}
501501

502502
/// Pop the first available descriptor chain from the avail ring.
503-
pub fn pop<M: GuestMemory>(&mut self, mem: &M) -> Option<DescriptorChain> {
504-
debug_assert!(self.is_valid(mem));
505-
503+
pub fn pop(&mut self) -> Option<DescriptorChain> {
506504
let len = self.len();
507505
// The number of descriptor chain heads to process should always
508506
// be smaller or equal to the queue size, as the driver should
@@ -531,10 +529,10 @@ impl Queue {
531529
/// If no descriptor is available, enable notifications.
532530
pub fn pop_or_enable_notification<M: GuestMemory>(
533531
&mut self,
534-
mem: &M,
532+
_mem: &M,
535533
) -> Option<DescriptorChain> {
536534
if !self.uses_notif_suppression {
537-
return self.pop(mem);
535+
return self.pop();
538536
}
539537

540538
if self.try_enable_notification() {
@@ -1087,7 +1085,7 @@ mod verification {
10871085
#[kani::unwind(0)]
10881086
#[kani::solver(cadical)]
10891087
fn verify_pop() {
1090-
let ProofContext(mut queue, mem) = ProofContext::bounded_queue();
1088+
let ProofContext(mut queue, _) = ProofContext::bounded_queue();
10911089

10921090
// This is an assertion in pop which we use to abort firecracker in a ddos scenario
10931091
// This condition being false means that the guest is asking us to process every element
@@ -1099,7 +1097,7 @@ mod verification {
10991097

11001098
let next_avail = queue.next_avail;
11011099

1102-
if let Some(_) = queue.pop(&mem) {
1100+
if let Some(_) = queue.pop() {
11031101
// Can't get anything out of an empty queue, assert queue_len != 0
11041102
assert_ne!(queue_len, 0);
11051103
assert_eq!(queue.next_avail, next_avail + Wrapping(1));
@@ -1110,13 +1108,13 @@ mod verification {
11101108
#[kani::unwind(0)]
11111109
#[kani::solver(cadical)]
11121110
fn verify_undo_pop() {
1113-
let ProofContext(mut queue, mem) = ProofContext::bounded_queue();
1111+
let ProofContext(mut queue, _) = ProofContext::bounded_queue();
11141112

11151113
// See verify_pop for explanation
11161114
kani::assume(queue.len() <= queue.actual_size());
11171115

11181116
let queue_clone = queue.clone();
1119-
if let Some(_) = queue.pop(&mem) {
1117+
if let Some(_) = queue.pop() {
11201118
queue.undo_pop();
11211119
assert_eq!(queue, queue_clone);
11221120

@@ -1322,7 +1320,7 @@ mod tests {
13221320
assert_eq!(q.len(), 2);
13231321

13241322
// The first chain should hold exactly two descriptors.
1325-
let d = q.pop(m).unwrap().next_descriptor().unwrap();
1323+
let d = q.pop().unwrap().next_descriptor().unwrap();
13261324
assert!(!d.has_next());
13271325
assert!(d.next_descriptor().is_none());
13281326

@@ -1331,7 +1329,7 @@ mod tests {
13311329

13321330
// The next chain holds three descriptors.
13331331
let d = q
1334-
.pop(m)
1332+
.pop()
13351333
.unwrap()
13361334
.next_descriptor()
13371335
.unwrap()
@@ -1342,15 +1340,15 @@ mod tests {
13421340

13431341
// We've popped both chains, so the queue should be empty.
13441342
assert!(q.is_empty());
1345-
assert!(q.pop(m).is_none());
1343+
assert!(q.pop().is_none());
13461344

13471345
// Undoing the last pop should let us walk the last chain again.
13481346
q.undo_pop();
13491347
assert_eq!(q.len(), 1);
13501348

13511349
// Walk the last chain again (three descriptors).
13521350
let d = q
1353-
.pop(m)
1351+
.pop()
13541352
.unwrap()
13551353
.next_descriptor()
13561354
.unwrap()
@@ -1418,7 +1416,7 @@ mod tests {
14181416
assert_eq!(q.len(), 2);
14191417

14201418
// We process the first descriptor.
1421-
let d = q.pop(m).unwrap().next_descriptor();
1419+
let d = q.pop().unwrap().next_descriptor();
14221420
assert!(matches!(d, Some(x) if !x.has_next()));
14231421
// We confuse the device and set the available index as being 6.
14241422
vq.avail.idx.set(6);
@@ -1429,7 +1427,7 @@ mod tests {
14291427
// However, since the apparent length set by the driver is more than the queue size,
14301428
// we would be running the risk of going through some descriptors more than once.
14311429
// As such, we expect to panic.
1432-
q.pop(m);
1430+
q.pop();
14331431
}
14341432

14351433
#[test]
@@ -1592,7 +1590,7 @@ mod tests {
15921590
assert_eq!(q.avail_event(m), 0);
15931591

15941592
// Consume the descriptor. avail_event should be modified
1595-
assert!(q.pop(m).is_some());
1593+
assert!(q.pop().is_some());
15961594
assert!(q.try_enable_notification());
15971595
assert_eq!(q.avail_event(m), 1);
15981596
}

src/vmm/src/devices/virtio/rng/device.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -128,7 +128,7 @@ impl Entropy {
128128
let mem = self.device_state.mem().unwrap();
129129

130130
let mut used_any = false;
131-
while let Some(desc) = self.queues[RNG_QUEUE].pop(mem) {
131+
while let Some(desc) = self.queues[RNG_QUEUE].pop() {
132132
let index = desc.index;
133133
METRICS.entropy_event_count.inc();
134134

@@ -434,15 +434,15 @@ mod tests {
434434
let mut entropy_dev = th.device();
435435

436436
// This should succeed, we just added two descriptors
437-
let desc = entropy_dev.queues_mut()[RNG_QUEUE].pop(&mem).unwrap();
437+
let desc = entropy_dev.queues_mut()[RNG_QUEUE].pop().unwrap();
438438
assert!(matches!(
439439
// SAFETY: This descriptor chain is only loaded into one buffer
440440
unsafe { IoVecBufferMut::from_descriptor_chain(&mem, desc) },
441441
Err(crate::devices::virtio::iovec::IoVecError::ReadOnlyDescriptor)
442442
));
443443

444444
// This should succeed, we should have one more descriptor
445-
let desc = entropy_dev.queues_mut()[RNG_QUEUE].pop(&mem).unwrap();
445+
let desc = entropy_dev.queues_mut()[RNG_QUEUE].pop().unwrap();
446446
// SAFETY: This descriptor chain is only loaded into one buffer
447447
let mut iovec = unsafe { IoVecBufferMut::from_descriptor_chain(&mem, desc).unwrap() };
448448
entropy_dev.handle_one(&mut iovec).unwrap();

src/vmm/src/devices/virtio/vsock/csm/connection.rs

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -863,16 +863,12 @@ mod tests {
863863
let stream = TestStream::new();
864864
let mut rx_pkt = VsockPacket::from_rx_virtq_head(
865865
&vsock_test_ctx.mem,
866-
handler_ctx.device.queues[RXQ_INDEX]
867-
.pop(&vsock_test_ctx.mem)
868-
.unwrap(),
866+
handler_ctx.device.queues[RXQ_INDEX].pop().unwrap(),
869867
)
870868
.unwrap();
871869
let tx_pkt = VsockPacket::from_tx_virtq_head(
872870
&vsock_test_ctx.mem,
873-
handler_ctx.device.queues[TXQ_INDEX]
874-
.pop(&vsock_test_ctx.mem)
875-
.unwrap(),
871+
handler_ctx.device.queues[TXQ_INDEX].pop().unwrap(),
876872
)
877873
.unwrap();
878874
let conn = match conn_state {

src/vmm/src/devices/virtio/vsock/device.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -145,7 +145,7 @@ where
145145

146146
let mut have_used = false;
147147

148-
while let Some(head) = self.queues[RXQ_INDEX].pop(mem) {
148+
while let Some(head) = self.queues[RXQ_INDEX].pop() {
149149
let index = head.index;
150150
let used_len = match VsockPacket::from_rx_virtq_head(mem, head) {
151151
Ok(mut pkt) => {
@@ -198,7 +198,7 @@ where
198198

199199
let mut have_used = false;
200200

201-
while let Some(head) = self.queues[TXQ_INDEX].pop(mem) {
201+
while let Some(head) = self.queues[TXQ_INDEX].pop() {
202202
let index = head.index;
203203
let pkt = match VsockPacket::from_tx_virtq_head(mem, head) {
204204
Ok(pkt) => pkt,
@@ -237,7 +237,7 @@ where
237237
// This is safe since we checked in the caller function that the device is activated.
238238
let mem = self.device_state.mem().unwrap();
239239

240-
let head = self.queues[EVQ_INDEX].pop(mem).ok_or_else(|| {
240+
let head = self.queues[EVQ_INDEX].pop().ok_or_else(|| {
241241
METRICS.ev_queue_event_fails.inc();
242242
DeviceError::VsockError(VsockError::EmptyQueue)
243243
})?;

0 commit comments

Comments
 (0)