From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andrey Zhadchenko Date: Fri, 14 Aug 2026 14:00:04 +0200 Subject: Re: [Devel] [PATCH VZ10 5/6] drivers/md/dm-qcow2: scan lower delta in one pass over empty L1 entry In-Reply-To: <59f591e2-4292-40d9-baad-3936b0cc2184@virtuozzo.com> References: <20260810123000.19834-1-andrey.zhadchenko@virtuozzo.com> <20260810123000.19834-6-andrey.zhadchenko@virtuozzo.com> <59f591e2-4292-40d9-baad-3936b0cc2184@virtuozzo.com> Message-ID: <5bd8e60e-e13e-4d24-aabc-a31a32d3f677@virtuozzo.com> List-Id: On 8/14/26 13:09, Pavel Tikhomirov wrote: > > > On 8/10/26 14:29, Andrey Zhadchenko wrote: >> When an L1 entry is absent and there is a backing image, the seek >> code descends into the lower delta one cluster at a time. >> >> Change advance_and_spawn_lower_seek_qio() to take limit instead of >> a size. Pass bigger range if L1 entry is empty. >> >> On a 16G image (128K clusters, extended L2) with no L1 entries over a >> backing image with data at 15G, warm-cache SEEK_DATA improves from >> 40.2 ms to 0.01 ms. >> >> https://virtuozzo.atlassian.net/browse/VSTOR-139407 >> Feature: dm-qcow2: block device over QCOW2 files driver >> Signed-off-by: Andrey Zhadchenko >> --- >> drivers/md/dm-qcow2-map.c | 29 +++++++++++++++-------------- >> 1 file changed, 15 insertions(+), 14 deletions(-) >> >> diff --git a/drivers/md/dm-qcow2-map.c b/drivers/md/dm-qcow2-map.c >> index 776e89fc4eb1b..6a73c32689eca 100644 >> --- a/drivers/md/dm-qcow2-map.c >> +++ b/drivers/md/dm-qcow2-map.c >> @@ -4441,26 +4441,19 @@ static inline void seek_qio_next_clu(struct qio *qio, struct qcow2_map *map) >> seek_qio_set_sector(qio, bi_sector); >> } >> >> -static struct qio *advance_and_spawn_lower_seek_qio(struct qio *old_qio, u32 size) >> +static struct qio *advance_and_spawn_lower_seek_qio(struct qio *old_qio, loff_t end) >> { >> + struct qcow2 *lower = old_qio->qcow2->lower; >> struct qio *new_qio; >> - loff_t start; >> >> - start = to_bytes(old_qio->bi_iter.bi_sector); >> + if (end > lower->hdr.size) >> + end = lower->hdr.size; >> >> - if (start + size > old_qio->qcow2->lower->hdr.size) >> - size = old_qio->qcow2->lower->hdr.size - start; >> - >> - new_qio = alloc_seek_qio(old_qio->qcow2->lower, old_qio, start + size); >> + new_qio = alloc_seek_qio(lower, old_qio, end); >> if (!new_qio) >> return NULL; >> >> - if (old_qio->bi_iter.bi_size == size) { >> - seek_qio_next_clu(old_qio, NULL); > > Removal of this branch is a bit suspicious. Previously we advanced old_qio to next cluster > after bi_size should become zero. After this change we don't advance. AI suggests that > this can lead to infinite loop as then we will return back from new_qio to old_qio it won't > continue the search correctly. seek_qio_set_sector(old_qio, to_sector(end)) will move old qio to the next cluster (end) we want to scan in the top image. I don't see any problem here. bi_size is rather irrelevant with new code using the end. It is only needed to check that calc_front_qio_bytes() return doesn't indicate that hole/data starts mid-cluster. Can you provide a hypothetical example for such infinite loop? > >> - } else { >> - old_qio->bi_iter.bi_sector += to_sector(size); >> - old_qio->bi_iter.bi_size -= size; >> - } >> + seek_qio_set_sector(old_qio, to_sector(end)); >> >> return new_qio; >> } >> @@ -4508,8 +4501,16 @@ static int qcow2_llseek_hole_qio(struct qio *qio, int whence, loff_t *result) >> */ >> if (to_bytes(qio->bi_iter.bi_sector) < qio->qcow2->lower->hdr.size) { >> struct qio *new_qio; >> + loff_t end; >> + >> + if (!(map.level & L2_LEVEL)) >> + end = min_t(loff_t, >> + to_bytes(get_next_l2(qio)), >> + SEEK_QIO_DATA(qio)->lim); >> + else >> + end = to_bytes(qio->bi_iter.bi_sector) + size; >> >> - new_qio = advance_and_spawn_lower_seek_qio(qio, size); >> + new_qio = advance_and_spawn_lower_seek_qio(qio, end); >> if (!new_qio) { >> ret = -ENOMEM; >> break; >