On Tue, Nov 06, 2018 at 08:40:24AM -0700, Jason Gunthorpe wrote:
> On Mon, Nov 05, 2018 at 11:20:55PM +0200, Kamal Heib wrote:
> > On Mon, Nov 05, 2018 at 03:18:02PM -0500, Dennis Dalessandro wrote:
> > > On 11/5/2018 6:35 AM, Kamal Heib wrote:
> > > > The create_ah verb receive as parameter an udata buffer, this parameter
> > > > is missing in rvt_create_ah - fixing that.
> > >
> > > Please add a bit more to the commit message here. This implies it is fixing
> > > a bug but there is no bug per se. rdmavt has no need for udata buffer. If
> > > perhaps it is needed because of the on-coming patches, you should probably
> > > say this.
> > >
> > > -Denny
> >
> > Actually, I think that this is a bug that was found while I was doing the
> > changes to support the ib_device_ops in rdmavt. Before my changes the
> > initialization of the ib_device verbs in rdmavt was done in the
> > check_driver_override() function:
> >
> > static inline int check_driver_override(struct rvt_dev_info *rdi,
> > size_t offset, void *func)
> > {
> > if (!*(void **)((void *)&rdi->ibdev + offset)) {
> > *(void **)((void *)&rdi->ibdev + offset) = func;
> > return 0;
> > }
> >
> > return 1;
> > }
> >
> > So, when initialize the create_ah verbs from check_support() using the following
> > call, there is no guaranty that create_ah() and rvt_create_ah() have the same
> > prototype..., that's why this bug was not found during compilation time.
> >
> > case CREATE_AH:
> > check_driver_override(rdi, offsetof(struct ib_device,
> > create_ah),
> > rvt_create_ah);
> > break;
>
> *BARF* This whole mess must go away as soon as possible.
>
This will go away as part of the upcoming patch that adds the support for
ib_device_ops.
> This patch, with a better commit message, should be stand alone and go
> right away. Casting function pointers to different sigantures is a
> very bad idea.
>
OK, I'll improve the commit message and send this patch.
> Jason
Thanks,
Kamal
