Skip to content

Commit 1be7df7

Browse files
committed
refactor: use vmm_sys_util::timer module instead of timefd crate
Use vmm_sys_util's timerfd implementation instead of the one in the timerfd crate. This eliminates a couple of dependencies, including an aesthetically unpleasing double-dependency on different versions of rustix. Signed-off-by: Patrick Roy <roypat@amazon.co.uk>
1 parent f01bb79 commit 1be7df7

File tree

10 files changed

+43
-69
lines changed

10 files changed

+43
-69
lines changed

Cargo.lock

Lines changed: 1 addition & 13 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Cargo.toml

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,3 +41,6 @@ strip = "none"
4141

4242
[profile.bench]
4343
strip = "debuginfo"
44+
45+
[patch.crates-io]
46+
vmm-sys-util = { git = "https://github.com/roypat/vmm-sys-util", branch = "timerfd-nonblock-0.14" }

src/firecracker/Cargo.toml

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,6 @@ serde = { version = "1.0.219", features = ["derive"] }
2929
serde_derive = "1.0.136"
3030
serde_json = "1.0.143"
3131
thiserror = "2.0.16"
32-
timerfd = "1.6.0"
3332
utils = { path = "../utils" }
3433
vmm = { path = "../vmm" }
3534
vmm-sys-util = { version = "0.14.0", features = ["with-serde"] }

src/firecracker/src/metrics.rs

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -5,9 +5,9 @@ use std::os::unix::io::AsRawFd;
55
use std::time::Duration;
66

77
use event_manager::{EventOps, Events, MutEventSubscriber};
8-
use timerfd::{ClockId, SetTimeFlags, TimerFd, TimerState};
98
use vmm::logger::{IncMetric, METRICS, error, warn};
109
use vmm_sys_util::epoll::EventSet;
10+
use vmm_sys_util::timerfd::{TimerFd, TimerFdFlag};
1111

1212
/// Metrics reporting period.
1313
pub(crate) const WRITE_METRICS_PERIOD_MS: u64 = 60000;
@@ -23,7 +23,7 @@ pub(crate) struct PeriodicMetrics {
2323
impl PeriodicMetrics {
2424
/// PeriodicMetrics constructor. Can panic on `TimerFd` creation failure.
2525
pub fn new() -> Self {
26-
let write_metrics_event_fd = TimerFd::new_custom(ClockId::Monotonic, true, true)
26+
let write_metrics_event_fd = TimerFd::new_with_flags(TimerFdFlag::NONBLOCK)
2727
.expect("Cannot create the metrics timer fd.");
2828
PeriodicMetrics {
2929
write_metrics_event_fd,
@@ -35,12 +35,10 @@ impl PeriodicMetrics {
3535
/// Start the periodic metrics engine which will flush metrics every `interval_ms` millisecs.
3636
pub(crate) fn start(&mut self, interval_ms: u64) {
3737
// Arm the log write timer.
38-
let timer_state = TimerState::Periodic {
39-
current: Duration::from_millis(interval_ms),
40-
interval: Duration::from_millis(interval_ms),
41-
};
38+
let duration = Duration::from_millis(interval_ms);
4239
self.write_metrics_event_fd
43-
.set_state(timer_state, SetTimeFlags::Default);
40+
.reset(duration, Some(duration))
41+
.expect("failed to arm metrics write timer");
4442

4543
// Write the metrics straight away to check the process startup time.
4644
self.write_metrics();
@@ -77,7 +75,9 @@ impl MutEventSubscriber for PeriodicMetrics {
7775
}
7876

7977
if source == self.write_metrics_event_fd.as_raw_fd() {
80-
self.write_metrics_event_fd.read();
78+
self.write_metrics_event_fd
79+
.wait()
80+
.expect("failed to read metrics timer");
8181
self.write_metrics();
8282
} else {
8383
error!("Spurious METRICS event!");

src/vmm/Cargo.toml

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,6 @@ serde = { version = "1.0.219", features = ["derive", "rc"] }
4747
serde_json = "1.0.143"
4848
slab = "0.4.11"
4949
thiserror = "2.0.16"
50-
timerfd = "1.5.0"
5150
userfaultfd = "0.9.0"
5251
utils = { path = "../utils" }
5352
uuid = "1.18.0"

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

Lines changed: 5 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,8 @@ use std::time::Duration;
77

88
use log::{error, info};
99
use serde::Serialize;
10-
use timerfd::{ClockId, SetTimeFlags, TimerFd, TimerState};
1110
use vmm_sys_util::eventfd::EventFd;
11+
use vmm_sys_util::timerfd::{TimerFd, TimerFdFlag};
1212

1313
use super::super::ActivateError;
1414
use super::super::device::{DeviceState, VirtioDevice};
@@ -215,7 +215,7 @@ impl Balloon {
215215
}
216216

217217
let stats_timer =
218-
TimerFd::new_custom(ClockId::Monotonic, true, true).map_err(BalloonError::Timer)?;
218+
TimerFd::new_with_flags(TimerFdFlag::NONBLOCK).map_err(BalloonError::Timer)?;
219219

220220
Ok(Balloon {
221221
avail_features,
@@ -259,7 +259,7 @@ impl Balloon {
259259
}
260260

261261
pub(crate) fn process_stats_timer_event(&mut self) -> Result<(), BalloonError> {
262-
self.stats_timer.read();
262+
self.stats_timer.wait()?;
263263
self.trigger_stats_update()
264264
}
265265

@@ -486,12 +486,8 @@ impl Balloon {
486486
}
487487

488488
pub fn update_timer_state(&mut self) {
489-
let timer_state = TimerState::Periodic {
490-
current: Duration::from_secs(u64::from(self.stats_polling_interval_s)),
491-
interval: Duration::from_secs(u64::from(self.stats_polling_interval_s)),
492-
};
493-
self.stats_timer
494-
.set_state(timer_state, SetTimeFlags::Default);
489+
let duration = Duration::from_secs(self.stats_polling_interval_s as u64);
490+
self.stats_timer.reset(duration, Some(duration));
495491
}
496492

497493
/// Obtain the number of 4K pages the device is currently holding.

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -90,7 +90,7 @@ pub enum BalloonError {
9090
/// {0}
9191
InvalidAvailIdx(#[from] InvalidAvailIdx),
9292
/// Error creating the statistics timer: {0}
93-
Timer(std::io::Error),
93+
Timer(#[from] vmm_sys_util::errno::Error),
9494
}
9595

9696
#[derive(Debug, thiserror::Error, displaydoc::Display)]

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

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@ use std::sync::Arc;
77
use std::time::Duration;
88

99
use serde::{Deserialize, Serialize};
10-
use timerfd::{SetTimeFlags, TimerState};
1110

1211
use super::*;
1312
use crate::devices::virtio::balloon::device::{BalloonStats, ConfigSpace};
@@ -157,13 +156,8 @@ impl Persist<'_> for Balloon {
157156
balloon.set_stats_desc_index(state.stats_desc_index);
158157

159158
// Restart timer if needed.
160-
let timer_state = TimerState::Periodic {
161-
current: Duration::from_secs(u64::from(state.stats_polling_interval_s)),
162-
interval: Duration::from_secs(u64::from(state.stats_polling_interval_s)),
163-
};
164-
balloon
165-
.stats_timer
166-
.set_state(timer_state, SetTimeFlags::Default);
159+
let duration = Duration::from_secs(state.stats_polling_interval_s as u64);
160+
balloon.stats_timer.reset(duration, Some(duration));
167161
}
168162

169163
Ok(balloon)

src/vmm/src/rate_limiter/mod.rs

Lines changed: 19 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ use std::os::unix::io::{AsRawFd, RawFd};
55
use std::time::{Duration, Instant};
66
use std::{fmt, io};
77

8-
use timerfd::{ClockId, SetTimeFlags, TimerFd, TimerState};
8+
use vmm_sys_util::timerfd::{TimerFd, TimerFdFlag};
99

1010
pub mod persist;
1111

@@ -14,12 +14,12 @@ pub mod persist;
1414
pub enum RateLimiterError {
1515
/// Rate limiter event handler called without a present timer
1616
SpuriousRateLimiterEvent,
17+
/// Error reading timerfd: {0}
18+
Read(#[from] vmm_sys_util::errno::Error),
1719
}
1820

1921
// Interval at which the refill timer will run when limiter is at capacity.
20-
const REFILL_TIMER_INTERVAL_MS: u64 = 100;
21-
const TIMER_REFILL_STATE: TimerState =
22-
TimerState::Oneshot(Duration::from_millis(REFILL_TIMER_INTERVAL_MS));
22+
const REFILL_TIMER_DURATION: Duration = Duration::from_millis(100);
2323

2424
const NANOSEC_IN_ONE_MILLISEC: u64 = 1_000_000;
2525

@@ -367,7 +367,7 @@ impl RateLimiter {
367367
// We'll need a timer_fd, even if our current config effectively disables rate limiting,
368368
// because `Self::update_buckets()` might re-enable it later, and we might be
369369
// seccomp-blocked from creating the timer_fd at that time.
370-
let timer_fd = TimerFd::new_custom(ClockId::Monotonic, true, true)?;
370+
let timer_fd = TimerFd::new_with_flags(TimerFdFlag::NONBLOCK)?;
371371

372372
Ok(RateLimiter {
373373
bandwidth: bytes_token_bucket,
@@ -378,9 +378,11 @@ impl RateLimiter {
378378
}
379379

380380
// Arm the timer of the rate limiter with the provided `TimerState`.
381-
fn activate_timer(&mut self, timer_state: TimerState) {
381+
fn activate_timer(&mut self, one_shot_duration: Duration) {
382382
// Register the timer; don't care about its previous state
383-
self.timer_fd.set_state(timer_state, SetTimeFlags::Default);
383+
self.timer_fd
384+
.reset(one_shot_duration, None)
385+
.expect("failed to activate ratelimiter timer");
384386
self.timer_active = true;
385387
}
386388

@@ -407,7 +409,7 @@ impl RateLimiter {
407409
// make sure there is only one running timer for this limiter.
408410
BucketReduction::Failure => {
409411
if !self.timer_active {
410-
self.activate_timer(TIMER_REFILL_STATE);
412+
self.activate_timer(REFILL_TIMER_DURATION);
411413
}
412414
false
413415
}
@@ -424,9 +426,7 @@ impl RateLimiter {
424426
// `ratio * refill_time` milliseconds.
425427
// The conversion should be safe because the ratio is positive.
426428
#[allow(clippy::cast_sign_loss, clippy::cast_possible_truncation)]
427-
self.activate_timer(TimerState::Oneshot(Duration::from_millis(
428-
(ratio * refill_time as f64) as u64,
429-
)));
429+
self.activate_timer(Duration::from_millis((ratio * refill_time as f64) as u64));
430430
true
431431
}
432432
}
@@ -469,7 +469,7 @@ impl RateLimiter {
469469
///
470470
/// If the rate limiter is disabled or is not blocked, an error is returned.
471471
pub fn event_handler(&mut self) -> Result<(), RateLimiterError> {
472-
match self.timer_fd.read() {
472+
match self.timer_fd.wait()? {
473473
0 => Err(RateLimiterError::SpuriousRateLimiterEvent),
474474
_ => {
475475
self.timer_active = false;
@@ -777,7 +777,7 @@ pub(crate) mod tests {
777777
// second wait will always result in the limiter being refilled. Otherwise
778778
// there is a chance for a race condition between limiter refilling and limiter
779779
// checking.
780-
const TEST_REFILL_TIMER_INTERVAL_MS: u64 = REFILL_TIMER_INTERVAL_MS + 10;
780+
const TEST_REFILL_TIMER_DURATION: Duration = Duration::from_millis(110);
781781

782782
impl TokenBucket {
783783
// Resets the token bucket: budget set to max capacity and last-updated set to now.
@@ -1014,11 +1014,11 @@ pub(crate) mod tests {
10141014
// since consume failed, limiter should be blocked now
10151015
assert!(l.is_blocked());
10161016
// wait half the timer period
1017-
thread::sleep(Duration::from_millis(TEST_REFILL_TIMER_INTERVAL_MS / 2));
1017+
thread::sleep(TEST_REFILL_TIMER_DURATION / 2);
10181018
// limiter should still be blocked
10191019
assert!(l.is_blocked());
10201020
// wait the other half of the timer period
1021-
thread::sleep(Duration::from_millis(TEST_REFILL_TIMER_INTERVAL_MS / 2));
1021+
thread::sleep(TEST_REFILL_TIMER_DURATION / 2);
10221022
// the timer_fd should have an event on it by now
10231023
l.event_handler().unwrap();
10241024
// limiter should now be unblocked
@@ -1047,11 +1047,11 @@ pub(crate) mod tests {
10471047
// since consume failed, limiter should be blocked now
10481048
assert!(l.is_blocked());
10491049
// wait half the timer period
1050-
thread::sleep(Duration::from_millis(TEST_REFILL_TIMER_INTERVAL_MS / 2));
1050+
thread::sleep(TEST_REFILL_TIMER_DURATION / 2);
10511051
// limiter should still be blocked
10521052
assert!(l.is_blocked());
10531053
// wait the other half of the timer period
1054-
thread::sleep(Duration::from_millis(TEST_REFILL_TIMER_INTERVAL_MS / 2));
1054+
thread::sleep(TEST_REFILL_TIMER_DURATION / 2);
10551055
// the timer_fd should have an event on it by now
10561056
l.event_handler().unwrap();
10571057
// limiter should now be unblocked
@@ -1081,11 +1081,11 @@ pub(crate) mod tests {
10811081
// since consume failed, limiter should be blocked now
10821082
assert!(l.is_blocked());
10831083
// wait half the timer period
1084-
thread::sleep(Duration::from_millis(TEST_REFILL_TIMER_INTERVAL_MS / 2));
1084+
thread::sleep(TEST_REFILL_TIMER_DURATION / 2);
10851085
// limiter should still be blocked
10861086
assert!(l.is_blocked());
10871087
// wait the other half of the timer period
1088-
thread::sleep(Duration::from_millis(TEST_REFILL_TIMER_INTERVAL_MS / 2));
1088+
thread::sleep(TEST_REFILL_TIMER_DURATION / 2);
10891089
// the timer_fd should have an event on it by now
10901090
l.event_handler().unwrap();
10911091
// limiter should now be unblocked

src/vmm/src/rate_limiter/persist.rs

Lines changed: 4 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
//! Defines the structures needed for saving/restoring a RateLimiter.
55
66
use serde::{Deserialize, Serialize};
7+
use vmm_sys_util::timerfd::{TimerFd, TimerFdFlag};
78

89
use super::*;
910
use crate::snapshot::Persist;
@@ -82,7 +83,7 @@ impl Persist<'_> for RateLimiter {
8283
} else {
8384
None
8485
},
85-
timer_fd: TimerFd::new_custom(ClockId::Monotonic, true, true)?,
86+
timer_fd: TimerFd::new_with_flags(TimerFdFlag::NONBLOCK)?,
8687
timer_active: false,
8788
};
8889

@@ -151,10 +152,7 @@ mod tests {
151152
.unwrap()
152153
.partial_eq(restored_rate_limiter.bandwidth().unwrap())
153154
);
154-
assert_eq!(
155-
restored_rate_limiter.timer_fd.get_state(),
156-
TimerState::Disarmed
157-
);
155+
assert!(!restored_rate_limiter.timer_fd.is_armed().unwrap());
158156

159157
// Check that RateLimiter restores correctly after partially consuming tokens.
160158
rate_limiter.consume(10, TokenType::Bytes);
@@ -174,10 +172,7 @@ mod tests {
174172
.unwrap()
175173
.partial_eq(restored_rate_limiter.bandwidth().unwrap())
176174
);
177-
assert_eq!(
178-
restored_rate_limiter.timer_fd.get_state(),
179-
TimerState::Disarmed
180-
);
175+
assert!(!restored_rate_limiter.timer_fd.is_armed().unwrap());
181176

182177
// Check that RateLimiter restores correctly after totally consuming tokens.
183178
rate_limiter.consume(1000, TokenType::Bytes);

0 commit comments

Comments
 (0)