On Tue, Apr 12, 2022 at 10:24:05AM +0300, Leon Romanovsky wrote:
> +/*
> + * Send the DMA list to the HW for a normal MR using UMR.
> + * Dmabuf MR is handled in a similar way, except that the MLX5_IB_UPD_XLT_ZAP
> + * flag may be used.
> + */
> +int mlx5r_umr_update_mr_pas(struct mlx5_ib_mr *mr, unsigned int flags)
> +{
I would prefer to see zap split into its own function, it shouldn't
call rdma_for_each_block at all - the only reason it was structured
this way in the first place was because everything had to be squeezed
into a WR struct.
Then all the places calling
mlx5r_umr_update_xlt(.. MLX5_IB_UPD_XLT_ZAP)
should call this new function instead as well.
Zapping an ODP or zapping an normal mkey should just be the same
logic.
Also, looking at it, it would be futher nice if the duplication
between mlx5r_umr_update_xlt() and mlx5r_umr_update_mr_pas() could be
removed and perhaps organized into a way to make it logical to
eliminate the mlx5_odp_populate_xlt() "callback"
Which would give four entry points:
'umr fill xlt from sgl' (mlx5r_umr_update_mr_pas)
'umr fill xlt from dma_list' (populate_mtt)
'umr fill klm from xarray' (populate_klm)
'umr fill xlt/klm with zero' (zap)
I'd imagine a general construction to consolidate more stuff into
mlx5r_umr_create_xlt(), probably having it return a struct, including
setting up the initial wqe so the above are slimmer.
Maybe in a followup series though
Jason
