Re: [PATCH 05/13] block/preallocate: hold back writes into a preallocation in flight

[Kevin Wolf <kwolf@redhat.com>]

Am 30.09.2026 um 18:49 hat Denis V. Lunev geschrieben:
> From: Denis V. Lunev <den@openvz.org>
> 
> The previous patch holds the mutex across all of handle_write(),
> including the preallocating write zeroes. That is correct and far too
> coarse: the fallocate takes as long as it takes, and every writer that
> extends the file waits for it.
> 
> Reserve the range instead of holding the lock over it. file_end moves
> to the end of the preallocated range before the request is issued, so
> a writer arriving while it is in flight works from a length that
> already covers the range being zeroed.
> 
> That leaves it going straight into the range, and BDRV_REQ_SERIALISING
> cannot stop it. Serialisation orders a request against the ones in
> bs->tracked_requests, and the zeroing request is not there yet: the
> filter runs above bdrv_co_pwritev_part(), which is what tracks it.
> Whatever the filter lets through in that window is unordered by
> construction, so the filter has to hold it back itself.

If I understand correctly, the problem isn't that the zeroing request
isn't there yet, but will be later, but that it operates on bs->file
while you want to serialise requests against it on bs.

I wonder if the easier solution for this wouldn't be what you removed
from file-posix in the first patch: Get the current request and widen it
so that it covers the area that gets preallocated and then mark it
serialising.

> safe_end trails file_end, standing at the lowest start of a
> preallocation in flight and reaching file_end once none is left. A
> write above it waits on a queue and re-reads the state when the
> preallocations drain. A write zeroes merged into a preallocation
> starts it below file_end, so safe_end has to move down as well as up.
> 
> Below safe_end nothing can be zeroed any more, so those writes do not
> queue, and below zero_start there is nothing to record either, so
> those take no mutex at all. The stores are qatomic_set() because that
> path reads them without it.
> 
> Signed-off-by: Denis V. Lunev <den@openvz.org>
> Cc: qemu-stable@nongnu.org
> CC: Kevin Wolf <kwolf@redhat.com>
> CC: Hanna Reitz <hreitz@redhat.com>
> CC: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
> CC: Andrey Drobyshev <andrey.drobyshev@virtuozzo.com>
> ---
>  block/preallocate.c | 95 ++++++++++++++++++++++++++++++++++++---------
>  1 file changed, 76 insertions(+), 19 deletions(-)
> 
> diff --git a/block/preallocate.c b/block/preallocate.c
> index 442cdb0176..128f1da014 100644
> --- a/block/preallocate.c
> +++ b/block/preallocate.c
> @@ -32,6 +32,7 @@
>  #include "qemu/units.h"
>  #include "block/block-io.h"
>  #include "block/block_int.h"
> +#include "qemu/atomic.h"

Please also add comments to those struct fields that are supposed to be
accessed with atomics.

I see that data_end, safe_end and zero_start are accessed inconsistently
with and without atomics, sometimes even within the same function.

Kevin