On Thu, Jan 31, 2019 at 11:40:31AM +0200, Leon Romanovsky wrote:
> On Wed, Jan 30, 2019 at 09:37:05PM -0700, Jason Gunthorpe wrote:
> > On Wed, Jan 30, 2019 at 12:49:09PM +0200, Leon Romanovsky wrote:
> > > From: Leon Romanovsky <leonro@xxxxxxxxxxxx>
> > >
> > > Add new macros to be used in drivers while registering ops structure
> > > and IB/core while calling allocation routines, so drivers won't need
> > > to perform kzalloc/kfree in their paths.
> > >
> > > The change in allocation stage allows us to initialize common fields
> > > prior to calling to drivers (e.g. restrack).
> > >
> > > Signed-off-by: Leon Romanovsky <leonro@xxxxxxxxxxxx>
> > > drivers/infiniband/core/device.c | 2 ++
> > > include/rdma/ib_verbs.h | 13 +++++++++++++
> > > 2 files changed, 15 insertions(+)
> > >
> > > diff --git a/drivers/infiniband/core/device.c b/drivers/infiniband/core/device.c
> > > index d13491b416dc..c332398376fc 100644
> > > +++ b/drivers/infiniband/core/device.c
> > > @@ -1221,6 +1221,8 @@ void ib_set_device_ops(struct ib_device *dev, const struct ib_device_ops *ops)
> > > (ptr)->name = ops->name; \
> > > } while (0)
> > >
> > > +#define SET_OBJ_SIZE(ptr, name) SET_DEVICE_OP(ptr, size_##name)
> > > +
> > > SET_DEVICE_OP(dev_ops, add_gid);
> > > SET_DEVICE_OP(dev_ops, advise_mr);
> > > SET_DEVICE_OP(dev_ops, alloc_dm);
> > > diff --git a/include/rdma/ib_verbs.h b/include/rdma/ib_verbs.h
> > > index d54c87640f89..4423de3b3738 100644
> > > +++ b/include/rdma/ib_verbs.h
> > > @@ -2264,6 +2264,19 @@ struct ib_counters_read_attr {
> > >
> > > struct uverbs_attr_bundle;
> > >
> > > +#define INIT_RDMA_OBJ_SIZE(ib_struct, drv_struct, member) \
> > > + .size_##ib_struct = \
> > > + (sizeof(struct drv_struct) + \
> > > + BUILD_BUG_ON_ZERO(offsetof(struct drv_struct, member)) + \
> > > + BUILD_BUG_ON_ZERO( \
> > > + !__same_type(((struct drv_struct *)NULL)->member, \
> > > + struct ib_struct)))
> > > +
> > > +#define rdma_zalloc_drv_obj(ib_dev, ib_type) \
> > > + kzalloc(ib_dev->ops.size_##ib_type, GFP_KERNEL)
> >
> > Since we know the type, this should cast to the ib struct:
> >
> > ((struct ib_type *)kzalloc(...))
>
> No, it is useless to cast (void*), both checkpatch, C standard and C FAQ
> are not agree with you. I prefer to follow C standard here and not to
> cast.
No, you are mixing the advice of not open coding
pd = (struct ib_pd *)kzalloc()
in a caller with an API design that should be type safe.
The macro is an API, as such it should, as much as possible, take
fixed types as inputs and outputs, to make it type-safe.
You wouldn't return void * from a function if you know that the type
is really ib_pd.
This is the same reason that container_of casts its output rather than
just returning void *
Jason
