block: qcow: Move compressed read decompression out of lock

Move decompression of compressed QCOW2 clusters out of the metadata
lock. Previously, reading a compressed cluster acquired a write lock
on metadata to perform in place decompression. Now, try_map_read
extracts the compressed layout (host offset, size) under a read lock
and returns it in the ClusterReadMapping::Compressed variant. Each
consumer (QcowSync, QcowAsync, Qcow2Backing, QcowFile) performs the
pread and decompression at the call site without holding any lock,
using the pread_alloc and decompress_cluster helpers.

Create the decoder once in QcowMetadata as Arc<dyn Decoder> and
share it via Arc::clone to QcowAsync, QcowSync, and Qcow2Backing
at construction time. This avoids per read RwLock acquisitions and
heap allocations. Add Send + Sync bounds to the Decoder trait.

This eliminates write lock contention on compressed reads, allowing
them to proceed concurrently with other read operations.

Signed-off-by: Anatol Belski <anbelski@linux.microsoft.com>
This commit is contained in:
Anatol Belski
2026-04-17 20:57:05 +02:00
committed by Rob Bradford
parent 659f7c17e5
commit 5504ad753a
6 changed files with 153 additions and 43 deletions
+24 -6
View File
@@ -11,9 +11,10 @@ use std::os::fd::{AsFd, AsRawFd, BorrowedFd, OwnedFd};
use std::sync::Arc;
use crate::error::{BlockError, BlockErrorKind, BlockResult, ErrorOp};
use crate::qcow::decoder::Decoder;
use crate::qcow::metadata::{BackingRead, ClusterReadMapping, QcowMetadata};
use crate::qcow::{BackingFile, BackingKind, Error as QcowError};
use crate::qcow_common::pread_exact;
use crate::qcow_common::{decompress_cluster, pread_alloc, pread_exact};
/// Raw backing file using pread64 on a duplicated fd.
pub(crate) struct RawBacking {
@@ -52,6 +53,8 @@ pub(crate) struct Qcow2Backing {
pub(crate) metadata: Arc<QcowMetadata>,
pub(crate) data_fd: OwnedFd,
pub(crate) backing_file: Option<Arc<dyn BackingRead>>,
pub(crate) cluster_size: u64,
pub(crate) decoder: Arc<dyn Decoder>,
}
// SAFETY: All reads go through QcowMetadata which uses RwLock
@@ -104,10 +107,22 @@ impl Qcow2Backing {
)?;
buf_offset += length as usize;
}
ClusterReadMapping::Compressed { data } => {
let len = data.len();
buf[buf_offset..buf_offset + len].copy_from_slice(&data);
buf_offset += len;
ClusterReadMapping::Compressed {
host_offset,
compressed_size,
cluster_offset,
length,
} => {
let compressed =
pread_alloc(self.data_fd.as_raw_fd(), host_offset, compressed_size)?;
let decompressed = decompress_cluster(
&compressed,
self.cluster_size as usize,
&*self.decoder,
)?;
buf[buf_offset..buf_offset + length]
.copy_from_slice(&decompressed[cluster_offset..cluster_offset + length]);
buf_offset += length;
}
ClusterReadMapping::Backing {
offset: backing_offset,
@@ -152,8 +167,11 @@ pub fn shared_backing_from(bf: BackingFile) -> BlockResult<Arc<dyn BackingRead>>
}
BackingKind::Qcow { inner, backing } => {
let data_fd = dup_fd(inner.raw_file.as_fd())?;
let metadata = Arc::new(QcowMetadata::new(*inner));
Ok(Arc::new(Qcow2Backing {
metadata: Arc::new(QcowMetadata::new(*inner)),
cluster_size: metadata.cluster_size(),
decoder: metadata.decoder(),
metadata,
data_fd,
backing_file: backing.map(|bf| shared_backing_from(*bf)).transpose()?,
}))
+1 -1
View File
@@ -19,7 +19,7 @@ pub enum Error {
pub type Result<T> = std::result::Result<T, Error>;
/// Generic trait for decoding zlib/zstd formats
pub trait Decoder {
pub trait Decoder: Send + Sync {
fn decode(&self, input: &[u8], output: &mut [u8]) -> Result<usize>;
}
+38 -21
View File
@@ -19,10 +19,11 @@
use std::cmp::min;
use std::io::{self, Seek};
use std::mem;
use std::sync::RwLock;
use std::sync::{Arc, RwLock};
use libc::{EINVAL, EIO};
use super::decoder::Decoder;
use super::qcow_raw_file::QcowRawFile;
use super::refcount::RefCount;
use super::util::{
@@ -51,13 +52,16 @@ pub enum ClusterReadMapping {
/// bounded by cluster boundary and guest request.
Allocated { offset: u64, length: u64 },
/// The cluster is compressed. The decompressed data is returned inline
/// because decompression is a CPU only operation that was done under the
/// write lock to access the raw compressed bytes from disk.
///
/// The data field contains exactly the bytes the guest requested, already
/// sliced from the decompressed cluster.
Compressed { data: Vec<u8> },
/// The cluster is compressed. The host file offset and compressed byte
/// count are extracted from the L2 entry under the read lock. The caller
/// reads the compressed data with pread on its own fd, decompresses
/// into a cluster sized buffer, then slices the requested range.
Compressed {
host_offset: u64,
compressed_size: usize,
cluster_offset: usize,
length: usize,
},
/// The cluster is not allocated in this layer but may exist in a backing
/// file. The caller should delegate to the backing file at the given
@@ -112,6 +116,7 @@ pub enum DeallocAction {
/// write lock, so contention stays low and queues scale.
pub struct QcowMetadata {
inner: RwLock<QcowState>,
decoder: Arc<dyn Decoder>,
}
/// The actual metadata state, accessible only through the RwLock.
@@ -132,6 +137,7 @@ pub(crate) struct QcowState {
impl QcowMetadata {
pub(crate) fn new(inner: QcowState) -> Self {
QcowMetadata {
decoder: Arc::from(inner.header.get_decoder()),
inner: RwLock::new(inner),
}
}
@@ -333,6 +339,11 @@ impl QcowMetadata {
pub fn cluster_size(&self) -> u64 {
self.inner.read().unwrap().raw_file.cluster_size()
}
/// Returns the shared decoder matching the image compression type.
pub fn decoder(&self) -> Arc<dyn Decoder> {
Arc::clone(&self.decoder)
}
}
impl QcowState {
@@ -373,10 +384,19 @@ impl QcowState {
let l2_index = self.l2_table_index(address) as usize;
let l2_entry = l2_table[l2_index];
// Compressed entries require disk I/O for decompression - can't do
// that under a read lock. Fall through to the write lock path.
// Compressed entries: extract layout from L2 entry under read lock.
// The caller reads and decompresses on its own fd without holding
// the metadata lock.
if l2_entry_is_compressed(l2_entry) {
return Ok(None);
let (host_offset, compressed_size) =
l2_entry_compressed_cluster_layout(l2_entry, self.header.cluster_bits);
let cluster_offset = self.raw_file.cluster_offset(address) as usize;
return Ok(Some(ClusterReadMapping::Compressed {
host_offset,
compressed_size,
cluster_offset,
length: count,
}));
}
if l2_entry_is_empty(l2_entry) {
@@ -439,17 +459,14 @@ impl QcowState {
if l2_entry_is_empty(l2_entry) {
Ok(self.unallocated_read_mapping(address, count, has_backing_file))
} else if l2_entry_is_compressed(l2_entry) {
// Under write lock we can do I/O for decompression
let decompressed = self.decompress_l2_cluster(l2_entry)?;
let start = self.raw_file.cluster_offset(address) as usize;
let end = start
.checked_add(count)
.ok_or_else(|| io::Error::from_raw_os_error(EINVAL))?;
if end > decompressed.len() {
return Err(io::Error::from_raw_os_error(EINVAL));
}
let (host_offset, compressed_size) =
l2_entry_compressed_cluster_layout(l2_entry, self.header.cluster_bits);
let cluster_offset = self.raw_file.cluster_offset(address) as usize;
Ok(ClusterReadMapping::Compressed {
data: decompressed[start..end].to_vec(),
host_offset,
compressed_size,
cluster_offset,
length: count,
})
} else if l2_entry_is_zero(l2_entry) {
// Match original QcowFile::file_read semantics where zero flagged
+21 -2
View File
@@ -57,6 +57,7 @@ use crate::qcow::qcow_raw_file::{BeUint, QcowRawFile};
pub use crate::qcow::raw_file::RawFile;
use crate::qcow::refcount::RefCount;
use crate::qcow::vec_cache::{CacheMap, Cacheable, VecCache};
use crate::qcow_common::decompress_cluster;
#[sorted]
#[derive(Debug, Error)]
@@ -322,8 +323,26 @@ impl BackingFile {
.file_mut()
.read_exact(&mut buf[pos..pos + length as usize])?;
}
ClusterReadMapping::Compressed { data } => {
buf[pos..pos + data.len()].copy_from_slice(&data);
ClusterReadMapping::Compressed {
host_offset,
compressed_size,
cluster_offset,
length,
} => {
let mut compressed = vec![0u8; compressed_size];
inner
.raw_file
.file_mut()
.seek(SeekFrom::Start(host_offset))?;
inner.raw_file.file_mut().read_exact(&mut compressed)?;
let decompressed = decompress_cluster(
&compressed,
cluster_size as usize,
&*inner.header.get_decoder(),
)?;
buf[pos..pos + length].copy_from_slice(
&decompressed[cluster_offset..cluster_offset + length],
);
}
ClusterReadMapping::Backing {
offset: backing_off,