On Wed, May 20, 2015 at 03:03:52PM -0400, ira.weiny wrote:
> On Wed, May 20, 2015 at 12:19:28PM -0600, Jason Gunthorpe wrote:
> > On Wed, May 20, 2015 at 02:02:27PM -0400, ira.weiny wrote:
> > > On Wed, May 20, 2015 at 11:37:47AM -0600, Hefty, Sean wrote:
> > > > > +/**
> > > > > + * rdma_max_mad_size - Return the max MAD size required by this RDMA
> > > > > Port.
> > > > > + *
> > > > > + * @device: Device
> > > > > + * @port_num: Port number
> > > > > + *
> > > > > + * Return the max MAD size required by the Port. Should return 0 if
> > > > > the
> > > > > port
> > > > > + * does not support MADs
> > > > > + */
> > > > > +static inline size_t rdma_max_mad_size(struct ib_device *device, u8
> > > > > port_num)
> > > > > +{
> > > > > + return device->port_immutable[port_num].max_mad_size;
> > > > > +}
> > > >
> > > > Should this function check the mad_cap bit and return 0 if not set? If
> > > > not,
> > > > I would remove the 'should return 0...' statement from the comment
> > > > above and
> > > > state that the caller should check the mad_cap bit first.
> > >
> > > I'll change the comment.
> >
> > If there is no mad support the port_immutable.max_mad_size should be
> > 0, force it during registration, then the comment is right.
> >
> > Force to 0 is better than 'some random value'
>
> When you say "force" do you mean have the drivers which don't support MAD
> explicitly set it to 0?
I'm not really sure it makes sense to have the drivers set this, since
the value is fixed based on the existing caps. The core could fill it
in, then we don't have to check the driver's work.
If the driver fills it then the core should do
BUG_ON(!mad_cap && max_mad_size != 0)
After calling the driver to fill the values.
The comment seems OK (change Should => Will), it is describing the
function, and the function should be the only accessor for this value,
so it also describes the value. Maybe clarify what mad size is .. Is
it just the payload?
Jason
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
