On Thu, Jul 26, 2018 at 08:45:21AM -0600, Jason Gunthorpe wrote:
> On Thu, Jul 26, 2018 at 08:47:13AM +0300, Leon Romanovsky wrote:
> > On Wed, Jul 25, 2018 at 11:10:43PM +0300, Kamal Heib wrote:
> > > Removing the check for mandatory verbs because not all providers are
> > > implementing them.
> > >
> > > Signed-off-by: Kamal Heib <kamalheib1@xxxxxxxxx>
> > > drivers/infiniband/core/device.c | 45 ----------------------------------------
> > > 1 file changed, 45 deletions(-)
> > >
> > > diff --git a/drivers/infiniband/core/device.c b/drivers/infiniband/core/device.c
> > > index b8144f194777..c6fd0ec344b7 100644
> > > +++ b/drivers/infiniband/core/device.c
> > > @@ -93,46 +93,6 @@ static struct notifier_block ibdev_lsm_nb = {
> > > .notifier_call = ib_security_change,
> > > };
> > >
> > > -static int ib_device_check_mandatory(struct ib_device *device)
> > > -{
> > > -#define IB_MANDATORY_FUNC(x) { offsetof(struct ib_device, x), #x }
> > > - static const struct {
> > > - size_t offset;
> > > - char *name;
> > > - } mandatory_table[] = {
> > > - IB_MANDATORY_FUNC(query_device),
> > > - IB_MANDATORY_FUNC(query_port),
> > > - IB_MANDATORY_FUNC(query_pkey),
> > > - IB_MANDATORY_FUNC(alloc_pd),
> > > - IB_MANDATORY_FUNC(dealloc_pd),
> > > - IB_MANDATORY_FUNC(create_ah),
> > > - IB_MANDATORY_FUNC(destroy_ah),
> > > - IB_MANDATORY_FUNC(create_qp),
> > > - IB_MANDATORY_FUNC(modify_qp),
> > > - IB_MANDATORY_FUNC(destroy_qp),
> > > - IB_MANDATORY_FUNC(post_send),
> > > - IB_MANDATORY_FUNC(post_recv),
> > > - IB_MANDATORY_FUNC(create_cq),
> > > - IB_MANDATORY_FUNC(destroy_cq),
> > > - IB_MANDATORY_FUNC(poll_cq),
> > > - IB_MANDATORY_FUNC(req_notify_cq),
> > > - IB_MANDATORY_FUNC(get_dma_mr),
> > > - IB_MANDATORY_FUNC(dereg_mr),
> > > - IB_MANDATORY_FUNC(get_port_immutable)
> > > - };
> > > - int i;
> > > -
> > > - for (i = 0; i < ARRAY_SIZE(mandatory_table); ++i) {
> > > - if (!*(void **) ((void *) device + mandatory_table[i].offset)) {
> > > - pr_warn("Device %s is missing mandatory function %s\n",
> > > - device->name, mandatory_table[i].name);
> > > - return -EINVAL;
> > > - }
> > > - }
> >
> > It is not enough to remove this check, you need to add checks if
> > function exists in all places with calls to those mandatory functions.
>
> Ugh, Okay, that is something I have some partial plans for down the
> road.
>
> > It is better have some general function/macro for that, something
> > like that:
> >
> > #define is_cmd_suported(dev, name) (dev->name)
> >
> > In long run, we will do in kernel something similar to
> > verbs_context_ops in rdma-core.
>
> So, what we should do is drop off all functions here that have
> internal protections in verbs/etc. Since Kamal's patches add these
> protections to the _ah family lets just do what he originally
> proposed..
>
> Thus ib_device_check_mandatory looks for functions that the kernel
> internally assumes are not NULL, and has nothing to do with IBTA,
> specs, etc.
>
> Jason
OK, I'll resend the patch that remove {create, destroy}_ah from
ib_device_check_mandatory().
Thanks,
Kamal
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
