RE: [PATCH 2/5] IB/core: Formalize the creation of immutable per port data within the ib_device object

Weiny, Ira
2015-05-12T17:48:55Z

> > In preparation for this create a structure for per port immutable data
> > and place the pkey and gid table lengths within this structure.
> >
> > This type of data requires a new call back "port_immutable" parameter
> > to ib_register_device to allow each driver to create this data as appropriate.
> > This callback is added to ib_register_device rather than as a new
> > device function because the callback should only be used when devices
> > are first registered.
> >

I tried to explain my decision here...

> > @@ -273,7 +273,9 @@ out:
> > */
> > int ib_register_device(struct ib_device *device,
> > int (*port_callback)(struct ib_device *,
> > - u8, struct kobject *))
> > + u8, struct kobject *),
> > + int (*port_immutable)(struct ib_device *, u8,
> > + struct ib_port_immutable *))
>
> I'm having a hard time getting past how ugly this is. Passing callbacks as
> arguments to a registration function should be a last resort. I would rather see
> this added to the device mandatory list (and it *is* mandatory, we use it
> without checking for NULL).

Your correct I should have checked for NULL and returned an error if not supplied.

> In truth, I'd rather see both of those moved to the
> driver callback struct. They can be placed at the end, after a comment that
> says something like "These are single use entry points for initialization, keep
> them away from the rest of the entry points to help prevent growing the entry
> point list beyond any more cachelines that needed for the more commonly
> used entry points". I would find that much preferable to this.
>

I'll change to this method in the next version.

Ira