From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andrey Zhadchenko Date: Fri, 14 Aug 2026 12:33:47 +0200 Subject: Re: [Devel] [PATCH VZ10 4/6] drivers/md/dm-qcow2: fix seek skip to the next L2 table In-Reply-To: <4b243dfd-3796-47aa-a384-221f1c065b25@virtuozzo.com> References: <20260810123000.19834-1-andrey.zhadchenko@virtuozzo.com> <20260810123000.19834-5-andrey.zhadchenko@virtuozzo.com> <4b243dfd-3796-47aa-a384-221f1c065b25@virtuozzo.com> Message-ID: List-Id: On 8/14/26 12:25, Pavel Tikhomirov wrote: > > > On 8/10/26 14:29, Andrey Zhadchenko wrote: >> get_next_l2() adds the number of remaining clusters to the current >> sector, so for a position which is not cluster aligned (e.g. the >> llseek offset itself) the result overshoots the L2 table boundary by >> the in-cluster offset. >> >> Compute the boundary from the cluster index instead of adding to the >> current position. >> >> 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 | 8 ++++---- >> 1 file changed, 4 insertions(+), 4 deletions(-) >> >> diff --git a/drivers/md/dm-qcow2-map.c b/drivers/md/dm-qcow2-map.c >> index d6d15b04215c6..776e89fc4eb1b 100644 >> --- a/drivers/md/dm-qcow2-map.c >> +++ b/drivers/md/dm-qcow2-map.c >> @@ -4405,12 +4405,12 @@ static struct qio *free_seek_qio_ret_higher(struct qio *qio) >> static inline sector_t get_next_l2(struct qio *qio) >> { >> struct qcow2 *qcow2 = qio->qcow2; >> - loff_t start, add; >> + u64 base_l2, add; >> >> - start = to_bytes(qio->bi_iter.bi_sector); >> - add = qcow2->l2_entries - (start / qcow2->clu_size) % qcow2->l2_entries; >> + base_l2 = to_bytes(qio->bi_iter.bi_sector) / qcow2->clu_size; >> + add = qcow2->l2_entries - base_l2 % qcow2->l2_entries; >> >> - return qio->bi_iter.bi_sector + (qcow2->clu_size / to_bytes(1)) * add; >> + return to_sector((base_l2 + add) * qcow2->clu_size); > > The above code seems too complex to what it actually tries to accomplish, > it's just an alignment, what about: > > static inline sector_t get_next_l2(struct qio *qio) > { > struct qcow2 *qcow2 = qio->qcow2; > u64 l2_covered = (u64)qcow2->clu_size * qcow2->l2_entries; > > return to_sector(round_up(to_bytes(qio->bi_iter.bi_sector) + 1, > l2_covered)); > } Yeah, that looks much better. But probably no sense to convert sector to bytes and then back to sector (as cluster_size for qcow2 is always a power of 2 no less then 512). I will update this part for v2 > >> } >> >> static inline sector_t get_next_clu(struct qio *qio) >