On Tue, Dec 04, 2018 at 04:03:21PM +0200, Leon Romanovsky wrote:
> From: Moni Shoua <monis@xxxxxxxxxxxx>
>
> The verb advise_mr() is used to give advice to the kernel
> about an address range that belongs to a MR.
> Implement the verb and register it on the device. The current
> implementation supports the only known advice to date, prefetch.
>
> Signed-off-by: Moni Shoua <monis@xxxxxxxxxxxx>
> Reviewed-by: Guy Levi <guyle@xxxxxxxxxxxx>
> Signed-off-by: Leon Romanovsky <leonro@xxxxxxxxxxxx>
> drivers/infiniband/hw/mlx5/mlx5_ib.h | 17 ++++
> drivers/infiniband/hw/mlx5/mr.c | 15 ++++
> drivers/infiniband/hw/mlx5/odp.c | 122 +++++++++++++++++++++++++--
> 3 files changed, 145 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/infiniband/hw/mlx5/mlx5_ib.h b/drivers/infiniband/hw/mlx5/mlx5_ib.h
> index 95652ef8d89b..0c96d2993f41 100644
> +++ b/drivers/infiniband/hw/mlx5/mlx5_ib.h
> @@ -1088,6 +1088,12 @@ struct ib_mr *mlx5_ib_get_dma_mr(struct ib_pd *pd, int acc);
> struct ib_mr *mlx5_ib_reg_user_mr(struct ib_pd *pd, u64 start, u64 length,
> u64 virt_addr, int access_flags,
> struct ib_udata *udata);
> +int mlx5_ib_advise_mr(struct ib_pd *pd,
> + enum ib_uverbs_advise_mr_advice advice,
> + u32 flags,
> + struct ib_sge *sg_list,
> + u32 num_sge,
> + struct uverbs_attr_bundle *attrs);
> struct ib_mw *mlx5_ib_alloc_mw(struct ib_pd *pd, enum ib_mw_type type,
> struct ib_udata *udata);
> int mlx5_ib_dealloc_mw(struct ib_mw *mw);
> @@ -1185,6 +1191,10 @@ void mlx5_ib_invalidate_range(struct ib_umem_odp *umem_odp, unsigned long start,
> void mlx5_odp_init_mr_cache_entry(struct mlx5_cache_ent *ent);
> void mlx5_odp_populate_klm(struct mlx5_klm *pklm, size_t offset,
> size_t nentries, struct mlx5_ib_mr *mr, int flags);
> +
> +int mlx5_ib_advise_mr_prefetch(struct ib_pd *pd,
> + enum ib_uverbs_advise_mr_advice advice,
> + u32 flags, struct ib_sge *sg_list, u32 num_sge);
> #else /* CONFIG_INFINIBAND_ON_DEMAND_PAGING */
> static inline void mlx5_ib_internal_fill_odp_caps(struct mlx5_ib_dev *dev)
> {
> @@ -1200,6 +1210,13 @@ static inline void mlx5_odp_populate_klm(struct mlx5_klm *pklm, size_t offset,
> size_t nentries, struct mlx5_ib_mr *mr,
> int flags) {}
>
> +static int mlx5_ib_advise_mr_prefetch(struct ib_pd *pd,
> + enum ib_uverbs_advise_mr_advice advice,
> + u32 flags, struct ib_sge *sg_list,
> + u32 num_sge)
> +{
> + return -EOPNOTSUPP;
> +}
For !CONFIG_INFINIBAND_ON_DEMAND_PAGING the function pointer should be
made NULL, not assigned to a dummy function.
> @@ -1518,3 +1548,77 @@ int mlx5_ib_odp_init(void)
>
> return 0;
> }
> +
> +struct prefetch_mr_work {
> + struct work_struct work;
> + struct mlx5_ib_dev *dev;
> + u32 pf_flags;
> + struct ib_sge *sg_list;
> + u32 num_sge;
> +};
This would be better with a flex array at the end instead of doing two
allocations
> + work = kzalloc(sizeof(*work), GFP_KERNEL);
> + if (!work)
> + return -ENOMEM;
> +
> + work->sg_list = kcalloc(num_sge, sizeof(struct ib_sge), GFP_KERNEL);
And since num_sge is controlled by the user this should be kvalloc
> + if (!work->sg_list) {
> + kfree(work);
> + return -ENOMEM;
> + }
> + memcpy(work->sg_list, sg_list, num_sge * sizeof(struct ib_sge));
> +
> + work->dev = dev;
Copying a pointer without a kref? What prevents the ib_device from
becoming unregistered or freed while the work is waiting to run?
Jason
