Re: [pbs-devel] [PATCH proxmox-backup] garbage-collect: switch to tokio Mutex

[Christian Ebner <c.ebner@proxmox.com>]

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@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


_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel