On Thu, Jan 31, 2019 at 11:48:06AM +0200, Leon Romanovsky wrote:
> > > diff --git a/include/rdma/restrack.h b/include/rdma/restrack.h
> > > index 92d11c35221a..c81e43d043cf 100644
> > > +++ b/include/rdma/restrack.h
> > > @@ -161,4 +161,26 @@ struct xarray *rdma_dev_to_xa(struct ib_device *dev,
> > > enum rdma_restrack_type type);
> > > void rdma_rt_read_lock(struct ib_device *dev, enum rdma_restrack_type type);
> > > void rdma_rt_read_unlock(struct ib_device *dev, enum rdma_restrack_type type);
> > > +
> > > +/**
> > > + * rdma_res_to_id() - Unique ID as seen by restrack
> > > + * @res: resrouce to operate
> > > + *
> > > + * Return: unique ID
> > > + */
> > > +static inline u32 rdma_res_to_id(struct rdma_restrack_entry *res)
> > > +{
> > > + return res->id;
> > > +}
> > > +/**
> > > + * rdma_rt_set_type() - Set restrack entry type,
> > > + * prior to call rdma_restrack_add()
> > > + * @res: resrouce to operate
> > > + * @type: Actual type
> > > + */
> > > +static inline void rdma_rt_set_type(struct rdma_restrack_entry *res,
> > > + enum rdma_restrack_type type)
> > > +{
> > > + res->type = type;
> > > +}
> >
> > This seems a bit obfuscating, generally kernel code tries to avoid
> > this sort of trivial accessor style, I thought
>
> It is not really correct, kernel code distinguishes between driver code
> and core code. Obfuscation is not allowed in drivers, but fine for core
> core which needs to hide implementation from their users.
Do you ever intend to change these implementations?
> Such difference is due to need to refactor. You will need to refactor
> core code in one place, but changes in drivers in many places.
Generally I see this as being defered until a need to change actually
arises, then you switch the open coded struct access to function and
then rework the acessors
This sort of wrapper pattern, as well as the
rdma_rt_read_lock/rdma_rt_read_unlock wrappering don't seem like good
kernel design.. This isn't Java.
> If you are interested to feel the pain of obfuscated driver,
> you are invited to do any major changes in bnxt_re driver.
Well, that drivers is *full* of obfuscating layers, like this.
If there was some *reason* to have the function be more than an
assignment then maybe, but I don't see it..
Jason
