> -----Original Message-----
> From: Jason Gunthorpe <jgg@xxxxxxxx>
> Sent: Monday, November 26, 2018 11:42 AM
> To: Parav Pandit <parav@xxxxxxxxxxxx>
> Cc: Leon Romanovsky <leon@xxxxxxxxxx>; Doug Ledford
> <dledford@xxxxxxxxxx>; Leon Romanovsky <leonro@xxxxxxxxxxxx>; RDMA
> mailing list <linux-rdma@xxxxxxxxxxxxxxx>; Daniel Jurgens
> <danielj@xxxxxxxxxxxx>
> Subject: Re: [PATCH rdma-next 5/9] RDMA/core: Restrict sysfs entries view to
> init_net
>
> On Mon, Nov 26, 2018 at 04:10:06AM +0000, Parav Pandit wrote:
> >
> >
> > > From: Jason Gunthorpe <jgg@xxxxxxxx>
> > > Sent: Sunday, November 25, 2018 9:16 PM
> > > To: Parav Pandit <parav@xxxxxxxxxxxx>
> > > Cc: Leon Romanovsky <leon@xxxxxxxxxx>; Doug Ledford
> > > <dledford@xxxxxxxxxx>; Leon Romanovsky <leonro@xxxxxxxxxxxx>;
> RDMA
> > > mailing list <linux-rdma@xxxxxxxxxxxxxxx>; Daniel Jurgens
> > > <danielj@xxxxxxxxxxxx>
> > > Subject: Re: [PATCH rdma-next 5/9] RDMA/core: Restrict sysfs entries
> > > view to init_net
> > >
> > > On Sat, Nov 24, 2018 at 02:18:24PM +0000, Parav Pandit wrote:
> > >
> > > > > > static struct class ib_class = {
> > > > > > - .name = "infiniband",
> > > > > > - .dev_release = ib_device_release,
> > > > > > - .dev_uevent = ib_device_uevent,
> > > > > > + .name = "infiniband",
> > > > > > + .dev_release = ib_device_release,
> > > > > > + .dev_uevent = ib_device_uevent,
> > > > > > + .ns_type = &net_ns_type_operations,
> > > > > > + .namespace = net_namespace,
> > > > > > };
> > > > >
> > > > > and lets not reformat code just to add horizonal whitespace please.
> > > > >
> > > > I see. 4 out of the 5 fields of structure follows right alignment,
> > > > but keep old
> > > one as is?
> > > > But ok.
> > >
> > > Don't do vertical alignment then only 1 out of 5 is wrong.
> > >
> > Ok.
> >
> > > > > > +fs_initcall(ib_core_init);
> > > > >
> > > > > Really? So very strange.
> > > > >
> > > > Yes. when module is compile as in-built to kernel, init sequence
> > > > needs first
> > > initialize the net's init function first.
> > > > Otherwise ib_core_init gets called and accesses uninitialized
> > > net_ns_operations.
> > > > This is similar to 80211 net/wireless/core.c
> > >
> > > And subsyste_late_initcall() is not OK?
> > >
> > late_initcall() is after device_initcall().
> > Apparently commit a9cd1a673737 needs before device initialization.
> > So late_initcall() is late and bug fixed in commit a9cd1a673737 will surface
> again.
> > So that is why fs_initcall is chosen, after net init and before device init.
>
> This is exactly the sort of thing I'm worried about.. We have FS's that use
> RDMA, it is scary to have RDMA init at the same level as FS..
>
If there are two modules at same level, one depend on another, sequencing their init should be done by core kernel.
I changed ib_umad.ko to fs_initcall for an experiment along with init debug logs and I see below trace.
1446 [ 4.605564] calling ib_core_init+0x0/0x22d @ 1
1447 [ 4.606644] initcall ib_core_init+0x0/0x22d returned 0 after 1049 usecs
1448 [ 4.606647] calling ib_umad_init+0x0/0xe4 @ 1
1449 [ 4.606660] initcall ib_umad_init+0x0/0xe4 returned 0 after 9 usecs
I do not know how is this magic done, but seems like intuitive as later module have symbols dependency on previous one.
Same would be the case for cifs, sunrpc or nfs rdma who would depend on ib_core module to be loaded first.
> Is it ok to register a client prior to the subsys init?
>
No, this will mostly crash. But module which has symbol dependency, has to be init first as generic init call dependency resolution.
