[pbs-devel] [PATCH proxmox-backup] garbage-collect: switch to tokio Mutex
Christian Ebner
c.ebner at proxmox.com
Mon Sep 29 18:07:22 CEST 2025
On 9/29/25 11:32 AM, Fabian Grünbichler wrote:
> this Mutex is held for the duration of the whole garbage collection run, which
> now includes async calls into S3 code. this is not a highly contested lock, so
> switching to the more expensive tokio variant shouldn't cause noticeable
> overhead, and fix potential deadlock problems that could occur because of
> holding the lock guard across await points.
>
> Signed-off-by: Fabian Grünbichler <f.gruenbichler at proxmox.com>
> ---
> also found while going through std::sync::Mutex usage with S3
>
> pbs-datastore/src/datastore.rs | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/pbs-datastore/src/datastore.rs b/pbs-datastore/src/datastore.rs
> index 7cf020fc0..68043e7a5 100644
> --- a/pbs-datastore/src/datastore.rs
> +++ b/pbs-datastore/src/datastore.rs
> @@ -136,7 +136,7 @@ pub fn ensure_datastore_is_mounted(config: &DataStoreConfig) -> Result<(), Error
> /// management interface for backup.
> pub struct DataStoreImpl {
> chunk_store: Arc<ChunkStore>,
> - gc_mutex: Mutex<()>,
> + gc_mutex: tokio::sync::Mutex<()>,
> last_gc_status: Mutex<GarbageCollectionStatus>,
> verify_new: bool,
> chunk_order: ChunkOrder,
> @@ -152,7 +152,7 @@ impl DataStoreImpl {
> pub(crate) unsafe fn new_test() -> Arc<Self> {
> Arc::new(Self {
> chunk_store: Arc::new(unsafe { ChunkStore::panic_store() }),
> - gc_mutex: Mutex::new(()),
> + gc_mutex: tokio::sync::Mutex::new(()),
> last_gc_status: Mutex::new(GarbageCollectionStatus::default()),
> verify_new: false,
> chunk_order: Default::default(),
> @@ -513,7 +513,7 @@ impl DataStore {
>
> Ok(DataStoreImpl {
> chunk_store,
> - gc_mutex: Mutex::new(()),
> + gc_mutex: tokio::sync::Mutex::new(()),
> last_gc_status: Mutex::new(gc_status),
> verify_new: config.verify_new.unwrap_or(false),
> chunk_order: tuning.chunk_order.unwrap_or_default(),
This patch breaks the PBS build for me. Also, not sure if just replacing
the type is enough, as the tokio::sync::Mutex::lock() is now async, so
must be awaited afaiu from
https://docs.rs/tokio/latest/tokio/sync/struct.Mutex.html#method.lock
More information about the pbs-devel
mailing list