On Tue, May 26, 2020 at 02:21:25AM +0300, Kamal Heib wrote:
> On Mon, May 25, 2020 at 11:26:41AM -0300, Jason Gunthorpe wrote:
> > On Wed, May 13, 2020 at 12:50:24PM +0300, Leon Romanovsky wrote:
> > > From: Maor Gottlieb <maorg@xxxxxxxxxxxx>
> > >
> > > Current device ops implementation allows only two stages "set"/"not set"
> > > and requires caller to check if function pointer exists before
> > > calling it.
> > >
> > > In order to simplify this repetitive task, let's give an option to
> > > overwrite those pointers. This will allow us to set dummy functions
> > > for the specific function pointers.
> > >
> > > Signed-off-by: Maor Gottlieb <maorg@xxxxxxxxxxxx>
> > > Signed-off-by: Leon Romanovsky <leonro@xxxxxxxxxxxx>
> > > ---
> > > drivers/infiniband/core/device.c | 9 ++++-----
> > > 1 file changed, 4 insertions(+), 5 deletions(-)
> > >
> > > diff --git a/drivers/infiniband/core/device.c b/drivers/infiniband/core/device.c
> > > index d9f565a779df..9486e60b42cc 100644
> > > --- a/drivers/infiniband/core/device.c
> > > +++ b/drivers/infiniband/core/device.c
> > > @@ -2542,11 +2542,10 @@ EXPORT_SYMBOL(ib_get_net_dev_by_params);
> > > void ib_set_device_ops(struct ib_device *dev, const struct ib_device_ops *ops)
> > > {
> > > struct ib_device_ops *dev_ops = &dev->ops;
> > > -#define SET_DEVICE_OP(ptr, name) \
> > > - do { \
> > > - if (ops->name) \
> > > - if (!((ptr)->name)) \
> > > - (ptr)->name = ops->name; \
> > > +#define SET_DEVICE_OP(ptr, name) \
> > > + do { \
> > > + if (ops->name) \
> > > + (ptr)->name = ops->name; \
> > > } while (0)
> >
> > Did you carefully check every driver to be sure it is OK with this?
> >
> > Maybe Kamal remembers why it was like this?
> >
> > Jason
>
> The idea was to set a specific op only once by the provider when there
> is a valid function for the op, this was done to make sure that if
> the op isn't supported by the provider then it will be set to NULL.
This is not changed.
>
> I think it will be more cleaner from the provider point of view to
> see which ops are supported or not supported in the provider code. by
> overriding the ops in the core this will make things more confusing.
Actually the patch does quite opposite, set defaults by IB/core and
overwrite it by the provider later and not vice versa. The IB/core
won't overwrite defined by the provider ops. From provider point of view
no change.
Thanks
>
> Thanks,
> Kamal
