[Devel] [RFC PATCH vz9 v6 19/62] dm-ploop: simplify llseek

[Pavel Tikhomirov]

On 12/6/24 05:55, Alexander Atanasov wrote:
> From: Andrey Zhadchenko <andrey.zhadchenko at virtuozzo.com>
> 
> Do not bother with locked clusters. For llseek we can ignore all
> concurrent operations.
> Do not call ploop_bat_entries() on every cluster as this is slow.
> Just remember md and read it directly.
> 
> https://virtuozzo.atlassian.net/browse/VSTOR-91817
> Signed-off-by: Andrey Zhadchenko <andrey.zhadchenko at virtuozzo.com>
> ---
>   drivers/md/dm-ploop-map.c | 27 ++++++++-------------------
>   1 file changed, 8 insertions(+), 19 deletions(-)
> 
> diff --git a/drivers/md/dm-ploop-map.c b/drivers/md/dm-ploop-map.c
> index 7ecf79fcd2cc..ad7ca7d43dfc 100644
> --- a/drivers/md/dm-ploop-map.c
> +++ b/drivers/md/dm-ploop-map.c
> @@ -2088,32 +2088,21 @@ ALLOW_ERROR_INJECTION(ploop_prepare_reloc_index_wb, ERRNO);
>   loff_t ploop_llseek_hole(struct dm_target *ti, loff_t offset, int whence)
>   {
>   	struct ploop *ploop = ti->private;
> -	u32 clu, dst_clu;
> +	u32 clu, dst_clu, id, *bat;
> +	struct md_page *md;
>   	loff_t result;
>   
>   	clu = SEC_TO_CLU(ploop, to_sector(offset) + ploop->skip_off);
> +	id = U32_MAX;
>   
>   	while (clu < ploop->nr_bat_entries) {
> -
> -		/*
> -		 * Assume a locked cluster to have a data.
> -		 * In worst case someone will read zeroes
> -		 */
> -		if (ploop_postpone_if_cluster_locked(ploop, NULL, clu)) {
> -			if (whence & SEEK_DATA)
> -				break;
> -			if (whence & SEEK_HOLE) {
> -				clu++;
> -				continue;
> -			}
> +		if (id != ploop_bat_clu_to_page_nr(clu)) {
> +			id = ploop_bat_clu_to_page_nr(clu);
> +			md = ploop_md_page_find(ploop, id);
>   		}
>   
> -		/*
> -		 * It *may* be possible to speed this up by iterating over clusters
> -		 * in mapped metadata page, but this is a bit problematic
> -		 * since we want to check if cluster is locked
> -		 */
> -		dst_clu = ploop_bat_entries(ploop, clu, NULL, NULL);


> +		bat = md->kmpage;
> +		dst_clu = READ_ONCE(bat[ploop_bat_clu_idx_in_page(clu)]);

Before this patch we used ploop_bat_entries which takes md->md_lock 
around dst_clu reading, shouldn't we do the same here too?

>   
>   		if (whence & SEEK_DATA && dst_clu != BAT_ENTRY_NONE)
>   			break;

-- 
Best regards, Tikhomirov Pavel
Senior Software Developer, Virtuozzo.