On Sat, Jun 20, 2020 at 05:36:46PM +0300, Vladimir Sementsov-Ogievskiy wrote:
> @@ -83,6 +84,12 @@ typedef struct BdrvTrackedRequest {
> CoQueue wait_queue; /* coroutines blocked on this request */
>
> struct BdrvTrackedRequest *waiting_for;
> +
> + /*
> + * If non-zero, the request is under lock, so it's allowed to intersect
> + * (actully it must be inside) the @lock request.
s/actully/actually/
> @@ -745,15 +747,26 @@ static bool coroutine_fn
> wait_or_find_conflicts(BdrvTrackedRequest *self,
> if (tracked_request_overlaps(req, self->overlap_offset,
> self->overlap_bytes))
> {
> - /* Hitting this means there was a reentrant request, for
> - * example, a block driver issuing nested requests. This
> must
> - * never happen since it means deadlock.
> + if (self->lock == req) {
> + /* This is atomic request under range_lock */
> + assert(req->type == BDRV_TRACKED_LOCK);
> + assert(self->offset >= req->offset);
> + assert(self->bytes <= req->bytes);
These assertions do not catch requests that start within the locked
region but span beyond the end of the region. How about:
assert(self->offset + self->bytes - req->offset >= req->bytes);
> +int coroutine_fn bdrv_co_pwrite_zeroes_locked(BdrvChild *child, int64_t
> offset,
> + int bytes, BdrvRequestFlags
> flags,
> + BdrvTrackedRequest *lock)
The name is confusing because _locked() normally means that a mutex
should be held. Functions using that naming convention already exist in
block/io.c. It would be nice to distinguish between functions that need
BdrvTrackedRequest and functions that must be called with a mutex held.
How about bdrv_co_pwrite_zeroes_with_lock()?
signature.asc
Description: PGP signature
