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

Doug Ledford
2015-05-12T17:54:24Z

[-- Attachment #1: Type: text/plain, Size: 2041 bytes --]

On Tue, 2015-05-12 at 17:48 +0000, Weiny, Ira wrote:
> > > 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...

I know. I just couldn't handle the function declaration ;-)

> > > @@ -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.

Excellent.

--
Doug Ledford <dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
GPG KeyID: 0E572FDD

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 819 bytes --]