> > @@ -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.
>
This function is not a callback function.
It is called from mlx5_ib_advise_mr() for one of the prefetch advices
> > @@ -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
I agree, Will do.
>
> > + 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
>
I agree. Will do
> > + 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?
>
I agree.
I will solve by making sure that no pending work items are in queue
when device is preregistering or module is unloading
> Jason
